[PATCH V3] revset: make id() to not abort on an unknown 40-byte hexadecimal string

[Matt Mackall]

On Thu, 2015-04-16 at 19:24 +0300, Alexander Drozdov wrote:
> On Thu, 16 Apr 2015 22:07:27 +0900, Yuya Nishihara wrote:
> > On Thu, 16 Apr 2015 07:49:15 +0300, Alexander Drozdov wrote:
> >> # HG changeset patch
> >> # User Alexander Drozdov <al.drozdov at gmail.com>
> >> # Date 1429159436 -10800
> >> #      Thu Apr 16 07:43:56 2015 +0300
> >> # Node ID a4bd33c69fc54357e5002bb320f7abd762010e37
> >> # Parent  c560d8c687916cb70a6d54c2c9ddcb5c9e457be2
> >> revset: make id() to not abort on an unknown 40-byte hexadecimal string
> >>
> >> Instead, just return the empty set in the case as for less-then-40-byte
> >> strings.
> >>
> >> diff --git a/mercurial/revset.py b/mercurial/revset.py
> >> --- a/mercurial/revset.py
> >> +++ b/mercurial/revset.py
> >> @@ -1294,7 +1294,13 @@ def node_(repo, subset, x):
> >>       # i18n: "id" is a keyword
> >>       n = getstring(l[0], _("id requires a string"))
> >>       if len(n) == 40:
> >> -        rn = repo[n].rev()
> >> +        try:
> >> +            rn = repo.changelog.rev(node.bin(n))
> >> +        except (TypeError):
> >> +            # i18n: "id" is a keyword
> >> +            raise error.ParseError(_("id expects a hexadecimal string"))
> >
> > If we raise ParseError here, short non-hexadecimal hash should also be handled
> > the same way. But node.bin() can't be applied to odd-length hex. My suggestion
> > was to make separate patches so that the first one, no abort on unknown hash,
> > would likely be accepted.
> 
> I think extra checks for strings shorter than 40 characters here may cause a
> performance penalty as partialmatch() also do the checks. Instead, I think it is
> better to raise (actually to just re-raise) an exception from partialmatch() to
> just catch it here. C and pure python partialmatch() code should be changed.
> That change also requires to review all the code calling partialmatch(). And I
> thing that change is what should be done later by a separate patch, isn't it?

Looks like we're still iterating here, but I'll probably accept patches
for this after the freeze.

-- 
Mathematics is the supreme nostalgia of our time.