Patchwork [2,of,3,hglib] client: raise KeyError from __getitem__ (BC)

login
register
mail settings
Submitter Gregory Szorc
Date July 6, 2016, 9:40 p.m.
Message ID <04badaecb6343ca09230.1467841213@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15760/
State Accepted
Headers show

Comments

Gregory Szorc - July 6, 2016, 9:40 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1467839662 25200
#      Wed Jul 06 14:14:22 2016 -0700
# Node ID 04badaecb6343ca09230b9a8e80ee9e3388e748d
# Parent  2e5f8d01dc59ca4ed1314a66283cc05b2c32cf3a
client: raise KeyError from __getitem__ (BC)

object.__getitem__ is supposed to raise either IndexError or KeyError
(depending on whether the thing is a sequence or a mapping). Before,
we would raise ValueError because that's what the context constructor
raises.

I choose to raise KeyError because IndexError felt a bit too limiting.
This does sacrifice some magic with for loops handling IndexError.
However, iteration of this object should be handled by a custom
__iter__, so I don't think this is a problem.

Patch

diff --git a/hglib/client.py b/hglib/client.py
--- a/hglib/client.py
+++ b/hglib/client.py
@@ -1626,17 +1626,20 @@  class hgclient(object):
                 except TypeError:
                     v[i] = 0
 
             self._version = tuple(v)
 
         return self._version
 
     def __getitem__(self, changeid):
-        return context.changectx(self, changeid)
+        try:
+            return context.changectx(self, changeid)
+        except ValueError as e:
+            raise KeyError(*e.args)
 
     def __contains__(self, changeid):
         """
         check if changeid, which can be either a local revision number or a
         changeset id, matches a changeset in the client.
         """
         try:
             context.changectx(self, changeid)
diff --git a/tests/test-context.py b/tests/test-context.py
--- a/tests/test-context.py
+++ b/tests/test-context.py
@@ -12,16 +12,18 @@  class test_context(common.basetest):
     def test_basic(self):
         self.append('a', 'a')
         self.append('b', 'b')
         rev0, node0 = self.client.commit(b('first'), addremove=True)
 
         self.append('c', 'c')
         rev1, node1 = self.client.commit(b('second'), addremove=True)
 
+        self.assertRaises(KeyError, self.client.__getitem__, 'doesnotexist')
+
         ctx = self.client[node0]
 
         self.assertEquals(ctx.description(), b('first'))
         self.assertEquals(str(ctx), node0[:12].decode('latin-1'))
         self.assertEquals(ctx.node(), node0)
         self.assertEquals(int(ctx), rev0)
         self.assertEquals(ctx.rev(), rev0)
         self.assertEquals(ctx.branch(), b('default'))