[PATCH] revset: add function for matching extra data (issue2767)

[Henrik Stuart]

On 11-05-2012 21:55, MHarbison at attotech.com wrote:
> [Apologies if this doesn't thread correctly- I only get the digest and
> wasn't sure
> the proper way to initially respond]
>
>>
>> ----- Message from Henrik Stuart<hg at hstuart.dk>  on Fri, 11 May 2012
>> 14:23:43 +0200 -----
>>
>> To:
>>
>> mercurial-devel at selenic.com
>>
>> Subject:
>>
>> [PATCH] revset: add function for matching extra data (issue2767)
>>
>> # HG changeset patch
>> # User Henrik Stuart<hg at hstuart.dk>
>> # Date 1336738342 -7200
>> # Node ID 417f576e3552190b4a5ba9d2e2d1db0ec5b65927
>> # Parent  ddd4996740c785cc187766249977ea3ece8c17ab
>> revset: add function for matching extra data (issue2767)
>>
>> diff --git a/mercurial/revset.py b/mercurial/revset.py
>> --- a/mercurial/revset.py
>> +++ b/mercurial/revset.py
>> @@ -464,6 +464,31 @@
>>       getargs(x, 0, 0, _("draft takes no arguments"))
>>       return [r for r in subset if repo._phaserev[r] == phases.draft]
>>
>> +def extra(repo, subset, x):
>> +    """``extra(label, [value], [set])``
>> +    Changesets with the given label in the extra metadata, with the
> given
>> +    optional value, matched globally or from the optional set."""
>> +
>> +    l = getargs(x, 1, 3, _('extra takes at least 1 and at most 3
> arguments'))
>> +    label = getstring(l[0], 'first argument to extra must be a string')
>> +    value = None
>> +    s = subset
>> +
>> +    if len(l)>  1:
>> +        try:
>> +            value = getstring(l[1], '')
>> +        except error.ParseError:
>> +            # not a string, but another revspec, e.g. tip()
>> +            s = getset(repo, subset, l[1])
>> +    if len(l)>  2:
>> +        s = getset(repo, subset, l[2])
>> +
>> +    def _matchvalue(r):
>> +        v = repo[r].extra().get(label)
>> +        return value is None or v == value
>> +
>> +    return [r for r in s if _matchvalue(r)]
>> +
>>   def filelog(repo, subset, x):
>>       """``filelog(pattern)``
>>       Changesets connected to the specified filelog.
>> @@ -1145,6 +1170,7 @@
>>       "descendants": descendants,
>>       "_firstdescendants": _firstdescendants,
>>       "draft": draft,
>> +    "extra": extra,
>>       "file": hasfile,
>>       "filelog": filelog,
>>       "first": first,
>> diff --git a/tests/test-revset.t b/tests/test-revset.t
>> --- a/tests/test-revset.t
>> +++ b/tests/test-revset.t
>> @@ -32,6 +32,18 @@
>>     (branches are permanent and global, did you want a bookmark?)
>>     $ hg ci -Aqm2 -u Bob
>>
>> +  $ hg log -r "extra('branch', 'a-b-c-', 1:2)" --template '{rev}\n'
>> +  2
>> +  $ hg log -r "extra('branch', 'a-b-c-')" --template '{rev}\n'
>> +  2
>> +  $ hg log -r "extra('branch', 0:1)" --template '{rev}\n'
>> +  0
>> +  1
>> +  $ hg log -r "extra('branch')" --template '{rev}\n'
>> +  0
>> +  1
>> +  2
>> +
>>     $ hg co 1
>>     1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>     $ hg branch +a+b+c+
>>
>
> I hadn't seen the issue written up but started fiddling around trying to
> get
> the same functionality last night, and was thinking in a different
> direction.
> This isn't mutually exclusive with where I was going and does provide for
> the
> general case.  However, since graft, transplant, rebase and convert all
> leave
> different markers in 'extra', wouldn't it be better to have a revset symbol
> for each?  Something like converted([rev]), grafted([rev]), etc, which
> would
> allow grouping all revs that are a product of that operation, or specific
> rev(s) that originated from a given source?  The string that is embedded is
> a
> (not easy to remember) implementation detail, and some cases may have to be
> handled specially (e.g. I was trying to figure out why rebased revs weren't
> being marked as such in thg like it does for convert and transplant, and
> the
> extra field for rebase looked like gibberish- maybe it was encoded
> differently- I didn't have time to look into it further).  Also, it would
> let
> the user put in the local rev value or the short hex value instead of the
> whole thing (for graft and transplant anyway- the local value doesn't make
> sense for convert because that is in another repo, but maybe the short hex
> value does).

I definitely agree that graft/transplant/etc. should provide their own 
revset functions, but occasionally it's useful to be able to access 
extra directly. So it's mostly just a matter of convenience, not about 
replacing support for all of those by saying "just use extra()". And 
while the patch mentions issue2767 I'd say that the convert extension 
should also provide a converted (or equivalent) revset function - please 
feel free to take a stab at that.

-- 
Kind regards,
   Henrik Stuart