Re: [PATCH] revset: for x^2, do not take null as a valid p2 revision
On 10/15/2016 09:06 AM, Yuya Nishihara wrote: On Fri, 14 Oct 2016 18:42:12 +0200, Pierre-Yves David wrote: On 10/14/2016 05:35 PM, Yuya Nishihara wrote: # HG changeset patch # User Yuya Nishihara # Date 1476455580 -32400 # Fri Oct 14 23:33:00 2016 +0900 # Node ID 18ba0036dd81db9e45fd1a450c7f207f1b89f22c # Parent 5cb830801855dbb63e98b948e355bc995d295bf3 revset: for x^2, do not take null as a valid p2 revision Since we don't count null p2 revision as a parent, x^2 should never return null even if null is explicitly populated. I've pushed this as a net improvement. diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -1620,7 +1620,7 @@ def parentspec(repo, subset, x, n, order ps.add(cl.parentrevs(r)[0]) elif n == 2: parents = cl.parentrevs(r) -if len(parents) > 1: +if parents[1] != node.nullrev: Is there any chance that p1 is nullid and p2 is not? No. Also, how does this behave if r is a root (p1 and p2 is nullid). Should we instead filter the parents list ?:: parents = [p for p in cl.parentrevs(r) if p != nullrev] IMHO, that's undecided problem. It doesn't make sense to take p2=null as a valid relation because otherwise all revisions must be rooted to null (b), which doesn't agree with how we draw a graph including null (a). (a) -1 - 0 - 1 ... (b) -1 = 0 - 1 \ / -- But we could say '0^1' (or 'null:tip & 0^1') is null per the graph (a). So I fixed only 'x^2 -> null', which is clearly wrong. I remember a discussion about having the special revision excluded unless something else in the revset make a direct reference to it. But this is a discussion for after this cycle. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] revset: for x^2, do not take null as a valid p2 revision
On Fri, 14 Oct 2016 18:42:12 +0200, Pierre-Yves David wrote: > On 10/14/2016 05:35 PM, Yuya Nishihara wrote: > > # HG changeset patch > > # User Yuya Nishihara > > # Date 1476455580 -32400 > > # Fri Oct 14 23:33:00 2016 +0900 > > # Node ID 18ba0036dd81db9e45fd1a450c7f207f1b89f22c > > # Parent 5cb830801855dbb63e98b948e355bc995d295bf3 > > revset: for x^2, do not take null as a valid p2 revision > > > > Since we don't count null p2 revision as a parent, x^2 should never return > > null even if null is explicitly populated. > > > > diff --git a/mercurial/revset.py b/mercurial/revset.py > > --- a/mercurial/revset.py > > +++ b/mercurial/revset.py > > @@ -1620,7 +1620,7 @@ def parentspec(repo, subset, x, n, order > > ps.add(cl.parentrevs(r)[0]) > > elif n == 2: > > parents = cl.parentrevs(r) > > -if len(parents) > 1: > > +if parents[1] != node.nullrev: > > Is there any chance that p1 is nullid and p2 is not? No. > Also, how does this behave if r is a root (p1 and p2 is nullid). > > Should we instead filter the parents list ?:: > >parents = [p for p in cl.parentrevs(r) if p != nullrev] IMHO, that's undecided problem. It doesn't make sense to take p2=null as a valid relation because otherwise all revisions must be rooted to null (b), which doesn't agree with how we draw a graph including null (a). (a) -1 - 0 - 1 ... (b) -1 = 0 - 1 \ / -- But we could say '0^1' (or 'null:tip & 0^1') is null per the graph (a). So I fixed only 'x^2 -> null', which is clearly wrong. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] revset: for x^2, do not take null as a valid p2 revision
On 10/14/2016 05:35 PM, Yuya Nishihara wrote: # HG changeset patch # User Yuya Nishihara # Date 1476455580 -32400 # Fri Oct 14 23:33:00 2016 +0900 # Node ID 18ba0036dd81db9e45fd1a450c7f207f1b89f22c # Parent 5cb830801855dbb63e98b948e355bc995d295bf3 revset: for x^2, do not take null as a valid p2 revision Since we don't count null p2 revision as a parent, x^2 should never return null even if null is explicitly populated. diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -1620,7 +1620,7 @@ def parentspec(repo, subset, x, n, order ps.add(cl.parentrevs(r)[0]) elif n == 2: parents = cl.parentrevs(r) -if len(parents) > 1: +if parents[1] != node.nullrev: Is there any chance that p1 is nullid and p2 is not? Also, how does this behave if r is a root (p1 and p2 is nullid). Should we instead filter the parents list ?:: parents = [p for p in cl.parentrevs(r) if p != nullrev] ps.add(parents[1]) return subset & ps diff --git a/tests/test-revset.t b/tests/test-revset.t --- a/tests/test-revset.t +++ b/tests/test-revset.t @@ -2750,6 +2750,7 @@ parentrevspec 5 $ log 'merge()^2' 4 + $ log '(not merge())^2' $ log 'merge()^^' 3 $ log 'merge()^1^' ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel