Patchwork revset: make id() to not abort on an unknown 40-byte string

# HG changeset patch
# User Alexander Drozdov <al.drozdov@gmail.com>
# Date 1428905039 -10800
#      Mon Apr 13 09:03:59 2015 +0300
# Node ID c2e3c9bce437a01584e4c32469c0f5d0fcf566d7
# Parent  52ff737c63d2b2cb41185549aa9c35bc47317032
revset: make id() to not abort on an unknown 40-byte string

Instead, just return an empty set in the case as for less-then-40-byte
strings.

On Mon, 13 Apr 2015 09:11:02 +0300, Alexander Drozdov wrote:
> # HG changeset patch
> # User Alexander Drozdov <al.drozdov@gmail.com>
> # Date 1428905039 -10800
> #      Mon Apr 13 09:03:59 2015 +0300
> # Node ID c2e3c9bce437a01584e4c32469c0f5d0fcf566d7
> # Parent  52ff737c63d2b2cb41185549aa9c35bc47317032
> revset: make id() to not abort on an unknown 40-byte string
> 
> Instead, just return an 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
> @@ -1292,13 +1292,10 @@ def node_(repo, subset, x):
>      l = getargs(x, 1, 1, _("id requires one argument"))
>      # i18n: "id" is a keyword
>      n = getstring(l[0], _("id requires a string"))
> -    if len(n) == 40:
> -        rn = repo[n].rev()
> -    else:
> -        rn = None
> -        pm = repo.changelog._partialmatch(n)
> -        if pm is not None:
> -            rn = repo.changelog.rev(pm)
> +    rn = None
> +    pm = repo.changelog._partialmatch(n)
> +    if pm is not None:
> +        rn = repo.changelog.rev(pm)

I think "len(n) == 40" is necessary to avoid slow _partialmatch().

FWIW, this patch also fixes the bug of 40-byte tag or bookmark.

  $ hg tag xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
  $ hg log -r 'id(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx)'

Regards,

Would "if len(n) == 40 and n in repo:" be better?

On Mon, Apr 13, 2015, 07:06 Yuya Nishihara <yuya@tcha.org> wrote:

> On Mon, 13 Apr 2015 09:11:02 +0300, Alexander Drozdov wrote:
> > # HG changeset patch
> > # User Alexander Drozdov <al.drozdov@gmail.com>
> > # Date 1428905039 -10800
> > #      Mon Apr 13 09:03:59 2015 +0300
> > # Node ID c2e3c9bce437a01584e4c32469c0f5d0fcf566d7
> > # Parent  52ff737c63d2b2cb41185549aa9c35bc47317032
> > revset: make id() to not abort on an unknown 40-byte string
> >
> > Instead, just return an 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
> > @@ -1292,13 +1292,10 @@ def node_(repo, subset, x):
> >      l = getargs(x, 1, 1, _("id requires one argument"))
> >      # i18n: "id" is a keyword
> >      n = getstring(l[0], _("id requires a string"))
> > -    if len(n) == 40:
> > -        rn = repo[n].rev()
> > -    else:
> > -        rn = None
> > -        pm = repo.changelog._partialmatch(n)
> > -        if pm is not None:
> > -            rn = repo.changelog.rev(pm)
> > +    rn = None
> > +    pm = repo.changelog._partialmatch(n)
> > +    if pm is not None:
> > +        rn = repo.changelog.rev(pm)
>
> I think "len(n) == 40" is necessary to avoid slow _partialmatch().
>
> FWIW, this patch also fixes the bug of 40-byte tag or bookmark.
>
>   $ hg tag xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>   $ hg log -r 'id(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx)'
>
> Regards,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

On Mon, 13 Apr 2015 14:39:07 +0000, Martin von Zweigbergk wrote:
> Would "if len(n) == 40 and n in repo:" be better?

"n in repo" just does "try repo[n] except RepoLookupError", so I'd rather do
"try rn = repo[n].rev() except ...".

It would be nice if revlog or changelog had the function to look up node
only by full or partial nodeid, but I'm not sure how it should be implemented.
I think foozy had thoughts around it.

Regards,

On Tue, 14 Apr 2015 00:33:55 +0900, Yuya Nishihara <yuya@tcha.org> wrote:
> On Mon, 13 Apr 2015 14:39:07 +0000, Martin von Zweigbergk wrote:
>> Would "if len(n) == 40 and n in repo:" be better?
>
> "n in repo" just does "try repo[n] except RepoLookupError", so I'd rather do
> "try rn = repo[n].rev() except ...".

I'll rewrite the patch, thanks!

Also, my current patch don't not work in pure Python as revlog's _partialmatch()
don't try to match 40-byte identifiers.

> It would be nice if revlog or changelog had the function to look up node
> only by full or partial nodeid, but I'm not sure how it should be implemented.
> I think foozy had thoughts around it.
>
> Regards,
>