Patchwork [8,of,8] fileset: add a 'status(...)' predicate to control evaluation context

login
register
mail settings
Submitter Pierre-Yves David
Date March 3, 2017, 1:40 p.m.
Message ID <3e95bf7ed38189f6f56d.1488548445@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/18892/
State Superseded
Headers show

Comments

Pierre-Yves David - March 3, 2017, 1:40 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1488546500 -3600
#      Fri Mar 03 14:08:20 2017 +0100
# Node ID 3e95bf7ed38189f6f56de89571fffc667280bb56
# Parent  9d6e733046b9aa7e2ded8c4207625fedcc2a8c04
# EXP-Topic filesetrev-func
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3e95bf7ed381
fileset: add a 'status(...)' predicate to control evaluation context

Same as 'revs', this predicate does not select files but switches the evaluation
context. This allow to match file according arbitrary status call. We can now
express the same query as 'hg status'.

The API (two 'revsingle' class) have been picked instead of a single 'revs'
revset for multiple reasons:

 * it is less confusing to express
 * it allow to express more query (eg: backward status, cross branch status)
Yuya Nishihara - March 5, 2017, 11:49 a.m.
On Fri, 03 Mar 2017 14:40:45 +0100, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1488546500 -3600
> #      Fri Mar 03 14:08:20 2017 +0100
> # Node ID 3e95bf7ed38189f6f56de89571fffc667280bb56
> # Parent  9d6e733046b9aa7e2ded8c4207625fedcc2a8c04
> # EXP-Topic filesetrev-func
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3e95bf7ed381
> fileset: add a 'status(...)' predicate to control evaluation context

The series looks generally good, but I don't queue as the first half was
originally written by me. Thanks for bringing my hack into production.

> --- a/mercurial/fileset.py
> +++ b/mercurial/fileset.py
> @@ -463,6 +463,25 @@ def revs(mctx, x):
>                  result.append(f)
>      return result
>  
> +@predicate('status(base, rev, pattern)')
> +def status(mctx, x):
> +    """``status(base, rev, revspec)``
> +
> +    Evaluate predicate using status change between ``base`` and
> +    ``rev``. Examples:
> +
> +    - ``status(3, 7, added())`` - matches files added from "3" to "7"
> +    """
> +    repo = mctx.ctx.repo()
> +    # i18n: "status" is a keyword
> +    b, r, x = getargs(x, 3, 3, _("status takes three arguments"))
> +    # i18n: "status" is a keyword
> +    baserevspec = getstring(b, _("first argument to revs must be a revision"))
> +    revspec = getstring(r, _("second argument to revs must be a revision"))

s/revs/status/

> +    basectx = scmutil.revsingle(repo, baserevspec)
> +    ctx = scmutil.revsingle(repo, revspec)

Perhaps it's better to reject an empty revspec string.

And I slightly prefer picking the first rev for basectx just like
revset.rangeset() would do.
Pierre-Yves David - March 5, 2017, 1:18 p.m.
On 03/05/2017 12:49 PM, Yuya Nishihara wrote:
> On Fri, 03 Mar 2017 14:40:45 +0100, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1488546500 -3600
>> #      Fri Mar 03 14:08:20 2017 +0100
>> # Node ID 3e95bf7ed38189f6f56de89571fffc667280bb56
>> # Parent  9d6e733046b9aa7e2ded8c4207625fedcc2a8c04
>> # EXP-Topic filesetrev-func
>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3e95bf7ed381
>> fileset: add a 'status(...)' predicate to control evaluation context
>
> The series looks generally good, but I don't queue as the first half was
> originally written by me. Thanks for bringing my hack into production.
>
>> --- a/mercurial/fileset.py
>> +++ b/mercurial/fileset.py
>> @@ -463,6 +463,25 @@ def revs(mctx, x):
>>                  result.append(f)
>>      return result
>>
>> +@predicate('status(base, rev, pattern)')
>> +def status(mctx, x):
>> +    """``status(base, rev, revspec)``
>> +
>> +    Evaluate predicate using status change between ``base`` and
>> +    ``rev``. Examples:
>> +
>> +    - ``status(3, 7, added())`` - matches files added from "3" to "7"
>> +    """
>> +    repo = mctx.ctx.repo()
>> +    # i18n: "status" is a keyword
>> +    b, r, x = getargs(x, 3, 3, _("status takes three arguments"))
>> +    # i18n: "status" is a keyword
>> +    baserevspec = getstring(b, _("first argument to revs must be a revision"))
>> +    revspec = getstring(r, _("second argument to revs must be a revision"))
>
> s/revs/status/

Good catch, Do you want a V2 or a followup ?

>> +    basectx = scmutil.revsingle(repo, baserevspec)
>> +    ctx = scmutil.revsingle(repo, revspec)
>
> Perhaps it's better to reject an empty revspec string.

Revsingle natively refuse empty revset, (but we should add a test for it).

> And I slightly prefer picking the first rev for basectx just like
> revset.rangeset() would do.

I do not really like the idea. It seems strange to me to have the same 
revset interpreted as a differet revs when pased as 'base' or 'rev'.

Cheers,
Yuya Nishihara - March 5, 2017, 2:08 p.m.
On Sun, 5 Mar 2017 14:18:06 +0100, Pierre-Yves David wrote:
> On 03/05/2017 12:49 PM, Yuya Nishihara wrote:
> > On Fri, 03 Mar 2017 14:40:45 +0100, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> >> # Date 1488546500 -3600
> >> #      Fri Mar 03 14:08:20 2017 +0100
> >> # Node ID 3e95bf7ed38189f6f56de89571fffc667280bb56
> >> # Parent  9d6e733046b9aa7e2ded8c4207625fedcc2a8c04
> >> # EXP-Topic filesetrev-func
> >> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> >> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3e95bf7ed381
> >> fileset: add a 'status(...)' predicate to control evaluation context
> >
> > The series looks generally good, but I don't queue as the first half was
> > originally written by me. Thanks for bringing my hack into production.
> >
> >> --- a/mercurial/fileset.py
> >> +++ b/mercurial/fileset.py
> >> @@ -463,6 +463,25 @@ def revs(mctx, x):
> >>                  result.append(f)
> >>      return result
> >>
> >> +@predicate('status(base, rev, pattern)')
> >> +def status(mctx, x):
> >> +    """``status(base, rev, revspec)``
> >> +
> >> +    Evaluate predicate using status change between ``base`` and
> >> +    ``rev``. Examples:
> >> +
> >> +    - ``status(3, 7, added())`` - matches files added from "3" to "7"
> >> +    """
> >> +    repo = mctx.ctx.repo()
> >> +    # i18n: "status" is a keyword
> >> +    b, r, x = getargs(x, 3, 3, _("status takes three arguments"))
> >> +    # i18n: "status" is a keyword
> >> +    baserevspec = getstring(b, _("first argument to revs must be a revision"))
> >> +    revspec = getstring(r, _("second argument to revs must be a revision"))
> >
> > s/revs/status/
> 
> Good catch, Do you want a V2 or a followup ?

I'm not the one who will queue this series, but perhaps this can be fixed in
flight.

> >> +    basectx = scmutil.revsingle(repo, baserevspec)
> >> +    ctx = scmutil.revsingle(repo, revspec)
> >
> > Perhaps it's better to reject an empty revspec string.
> 
> Revsingle natively refuse empty revset, (but we should add a test for it).

I meant status('', '', pattern), not an empty result.

> > And I slightly prefer picking the first rev for basectx just like
> > revset.rangeset() would do.
> 
> I do not really like the idea. It seems strange to me to have the same 
> revset interpreted as a differet revs when pased as 'base' or 'rev'.

Okay, I don't have a strong opinion about this. But I note that it's the common
behavior where revision pair is required. Try "hg status --rev : --rev :" for
example.
Pierre-Yves David - March 6, 2017, 9:25 a.m.
On 03/05/2017 03:08 PM, Yuya Nishihara wrote:
> On Sun, 5 Mar 2017 14:18:06 +0100, Pierre-Yves David wrote:
>> On 03/05/2017 12:49 PM, Yuya Nishihara wrote:
>>> On Fri, 03 Mar 2017 14:40:45 +0100, Pierre-Yves David wrote:
>>>> # HG changeset patch
>>>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>>>> # Date 1488546500 -3600
>>>> #      Fri Mar 03 14:08:20 2017 +0100
>>>> # Node ID 3e95bf7ed38189f6f56de89571fffc667280bb56
>>>> # Parent  9d6e733046b9aa7e2ded8c4207625fedcc2a8c04
>>>> # EXP-Topic filesetrev-func
>>>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3e95bf7ed381
>>>> fileset: add a 'status(...)' predicate to control evaluation context
>>>
>>> The series looks generally good, but I don't queue as the first half was
>>> originally written by me. Thanks for bringing my hack into production.
>>>
>>>> --- a/mercurial/fileset.py
>>>> +++ b/mercurial/fileset.py
>>>> @@ -463,6 +463,25 @@ def revs(mctx, x):
>>>>                  result.append(f)
>>>>      return result
>>>>
>>>> +@predicate('status(base, rev, pattern)')
>>>> +def status(mctx, x):
>>>> +    """``status(base, rev, revspec)``
>>>> +
>>>> +    Evaluate predicate using status change between ``base`` and
>>>> +    ``rev``. Examples:
>>>> +
>>>> +    - ``status(3, 7, added())`` - matches files added from "3" to "7"
>>>> +    """
>>>> +    repo = mctx.ctx.repo()
>>>> +    # i18n: "status" is a keyword
>>>> +    b, r, x = getargs(x, 3, 3, _("status takes three arguments"))
>>>> +    # i18n: "status" is a keyword
>>>> +    baserevspec = getstring(b, _("first argument to revs must be a revision"))
>>>> +    revspec = getstring(r, _("second argument to revs must be a revision"))
>>>
>>> s/revs/status/
>>
>> Good catch, Do you want a V2 or a followup ?
>
> I'm not the one who will queue this series, but perhaps this can be fixed in
> flight.
>
>>>> +    basectx = scmutil.revsingle(repo, baserevspec)
>>>> +    ctx = scmutil.revsingle(repo, revspec)
>>>
>>> Perhaps it's better to reject an empty revspec string.
>>
>> Revsingle natively refuse empty revset, (but we should add a test for it).
>
> I meant status('', '', pattern), not an empty result.

Ha yes, good catch.

>>> And I slightly prefer picking the first rev for basectx just like
>>> revset.rangeset() would do.
>>
>> I do not really like the idea. It seems strange to me to have the same
>> revset interpreted as a differet revs when pased as 'base' or 'rev'.
>
> Okay, I don't have a strong opinion about this. But I note that it's the common
> behavior where revision pair is required. Try "hg status --rev : --rev :" for
> example.

Ho, I never noticed that behavior from `hg status`, in this case, sure, 
we should align `status(X, Y, …)` with `hg statuc --rev X --rev Y.

V2 incoming.

Cheer

Patch

diff --git a/mercurial/fileset.py b/mercurial/fileset.py
--- a/mercurial/fileset.py
+++ b/mercurial/fileset.py
@@ -463,6 +463,25 @@  def revs(mctx, x):
                 result.append(f)
     return result
 
+@predicate('status(base, rev, pattern)')
+def status(mctx, x):
+    """``status(base, rev, revspec)``
+
+    Evaluate predicate using status change between ``base`` and
+    ``rev``. Examples:
+
+    - ``status(3, 7, added())`` - matches files added from "3" to "7"
+    """
+    repo = mctx.ctx.repo()
+    # i18n: "status" is a keyword
+    b, r, x = getargs(x, 3, 3, _("status takes three arguments"))
+    # i18n: "status" is a keyword
+    baserevspec = getstring(b, _("first argument to revs must be a revision"))
+    revspec = getstring(r, _("second argument to revs must be a revision"))
+    basectx = scmutil.revsingle(repo, baserevspec)
+    ctx = scmutil.revsingle(repo, revspec)
+    return getset(mctx.switch(ctx, _buildstatus(ctx, x, basectx=basectx)), x)
+
 @predicate('subrepo([pattern])')
 def subrepo(mctx, x):
     """Subrepositories whose paths match the given pattern.
@@ -538,6 +557,7 @@  class fullmatchctx(matchctx):
 # filesets using matchctx.switch()
 _switchcallers = [
     'revs',
+    'status',
 ]
 
 def _intree(funcs, tree):
diff --git a/tests/test-fileset.t b/tests/test-fileset.t
--- a/tests/test-fileset.t
+++ b/tests/test-fileset.t
@@ -521,3 +521,59 @@  overlapping set
 
   $ fileset "revs('1+2', modified())"
   b2
+
+test 'status(...)'
+=================
+
+Simple case
+-----------
+
+  $ fileset "status(3, 4, added())"
+  .hgsub
+  .hgsubstate
+
+use rev to restrict matched file
+-----------------------------------------
+
+  $ hg status --removed --rev 0 --rev 1
+  R a2
+  $ fileset "status(0, 1, removed())"
+  a2
+  $ fileset "* and status(0, 1, removed())"
+  $ fileset -r 4 "status(0, 1, removed())"
+  a2
+  $ fileset -r 4 "* and status(0, 1, removed())"
+  $ fileset "revs('4', * and status(0, 1, removed()))"
+  $ fileset "revs('0', * and status(0, 1, removed()))"
+  a2
+
+check wdir()
+------------
+
+  $ hg status --removed  --rev 4
+  R con.xml
+  $ fileset "status(4, 'wdir()', removed())"
+  con.xml
+
+  $ hg status --removed --rev 2
+  R a2
+  $ fileset "status('2', 'wdir()', removed())"
+  a2
+
+test backward status
+--------------------
+
+  $ hg status --removed --rev 0 --rev 4
+  R a2
+  $ hg status --added --rev 4 --rev 0
+  A a2
+  $ fileset "status(4, 0, added())"
+  a2
+
+test cross branch status
+--------------------
+
+  $ hg status --added --rev 1 --rev 2
+  A a2
+  $ fileset "status(1, 2, added())"
+  a2