[PATCH 1 of 2] localrepo: document that __contains__() may raise LookupError

Yuya Nishihara yuya at tcha.org
Sat May 27 09:20:39 EDT 2017


On Fri, 26 May 2017 22:58:21 -0700, Martin von Zweigbergk wrote:
> On Thu, May 25, 2017 at 7:56 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya at tcha.org>
> > # Date 1495721882 -32400
> > #      Thu May 25 23:18:02 2017 +0900
> > # Node ID 2dd7893c39fb0df46cfa6fbaa901f8da95beffda
> > # Parent  2b5953a49f1407f825d65b45986d213cb5c79203
> > localrepo: document that __contains__() may raise LookupError
> >
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -573,6 +573,10 @@ class localrepository(object):
> >          return context.changectx(self, changeid)
> >
> >      def __contains__(self, changeid):
> > +        """True if the given changeid exists
> > +
> > +        error.LookupError is raised if an ambiguous node specified.
> 
> This seems surprising. Would it make sense to return True instead?
> Would it be a lot of work to fix callers?

Maybe no. The result is tri-state and it's up to callers whether an ambiguous
changeid should be considered True (=FoundSome) or False (=NotIdentified).
And we have pretty low test coverage.

% grep 'ambiguous identi' tests/*.t
tests/test-revset.t:  abort: 00changelog.i at 2: ambiguous identifier!

We could add AmbiguousLookupError to make things slightly clearer, btw.


More information about the Mercurial-devel mailing list