Patchwork [02,of,10] revset: add support for "wdir()^n"

# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1495395105 -19800
#      Mon May 22 01:01:45 2017 +0530
# Node ID 41b4ade12b0f9b227824ed62a515970c1ebbeb94
# Parent  4f81eb36137f66f8ce2a271404255fa6e0d84634
revset: add support for "wdir()^n"

This patch catches the WdirUnsupported exception raised, and adds support for
wdir^n which will give us the nth parent of the working directory.

On Mon, May 22, 2017 at 1:52 PM, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1495395105 -19800
> #      Mon May 22 01:01:45 2017 +0530
> # Node ID 41b4ade12b0f9b227824ed62a515970c1ebbeb94
> # Parent  4f81eb36137f66f8ce2a271404255fa6e0d84634
> revset: add support for "wdir()^n"
>
> This patch catches the WdirUnsupported exception raised, and adds support for
> wdir^n which will give us the nth parent of the working directory.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1502,11 +1502,19 @@
>          if n == 0:
>              ps.add(r)
>          elif n == 1:
> -            ps.add(cl.parentrevs(r)[0])
> +            try:
> +                ps.add(cl.parentrevs(r)[0])
> +            except error.WdirUnsupported:
> +                ps.add(repo[r].parents()[0].rev())
>          elif n == 2:
> -            parents = cl.parentrevs(r)
> -            if parents[1] != node.nullrev:
> -                ps.add(parents[1])
> +            try:
> +                parents = cl.parentrevs(r)
> +                if parents[1] != node.nullrev:
> +                    ps.add(parents[1])
> +            except error.WdirUnsupported:
> +                parents = repo[r].parents()
> +                if len(parents) == 2:
> +                    ps.add(parents[1].rev())

We could avoid relying on the exception by rewriting it like this:

                if current == node.wdirrev:
                    parents = [cl.rev(n) for n in repo.dirstate.parents()]
                else:
                    parents = cl.parentrevs(current)
                for parent in parents[:cut]:
                    if parent != node.nullrev:
                        heapq.heappush(h, -parent)

I've been trained not to rely on exceptions for normal flow control.
What's the advantage of using the exception here?

>      return subset & ps
>
>  @predicate('present(set)', safe=True)
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -1227,6 +1227,12 @@
>    0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>    $ hg debugrevspec 'wdir()^'
>    7
> +  $ hg debugrevspec 'wdir()^1'
> +  7
> +  $ hg debugrevspec 'wdir()^2'
> +  $ hg debugrevspec 'wdir()^3'
> +  hg: parse error: ^ expects a number 0, 1, or 2
> +  [255]
>  For tests consistency
>    $ hg up 9
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

On Wed, May 24, 2017 at 1:09 PM, Pulkit Goyal <7895pulkit@gmail.com> wrote:
>> We could avoid relying on the exception by rewriting it like this:
>>
>>                 if current == node.wdirrev:
>>                     parents = [cl.rev(n) for n in repo.dirstate.parents()]
>>                 else:
>>                     parents = cl.parentrevs(current)
>>                 for parent in parents[:cut]:
>>                     if parent != node.nullrev:
>>                         heapq.heappush(h, -parent)
>>
>> I've been trained not to rely on exceptions for normal flow control.
>> What's the advantage of using the exception here?
>>
> Yeah I agree there are cleaner way to do this, the best will be to write a
> function which will take care of this. But since this is inside a for loop
> and chances of wdir() to occur is {0,1}, it's preferred to use try-except
> for better runtime than conditions.
>
>>>> def a():
> ...     a = 'abc'
> ...     for i in xrange(1, 10000):
> ...             if i == a:
> ...                     continue
> ...             else:
> ...                     continue
> ...
>
>>>> def b():
> ...     for i in xrange(1, 10000):
> ...             try:
> ...                     continue
> ...             except TypeError:
> ...                     continue
> ...
>
>>>> timeit.timeit(a, number = 1000)
> 0.5894050598144531
>>>> timeit.timeit(b, number = 1000)
> 0.3453850746154785

That's what I was wondering, but I didn't bother checking if the cost
myself. Thanks for doing that for me!

I guess I expected that the try/except would incur similar overhead or
more than the condition, but it clearly doesn't. Thanks again, I'll
approve it (so it will become public soon).

>
> I send a series addressing the same issue using if conditions in Feb.[1]
> where Yuya suggested to me to introduce an error and catch that so that
> non-wdir() cases are not hurt.
>
> [1]:
> https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-February/093497.html