Patchwork identify: add template support

login
register
mail settings
Submitter Matt Harbison
Date June 25, 2017, 5:23 a.m.
Message ID <1e917448c8d5c36f19c4.1498368221@Envy>
Download mbox | patch
Permalink /patch/21693/
State Accepted
Headers show

Comments

Matt Harbison - June 25, 2017, 5:23 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1498360161 14400
#      Sat Jun 24 23:09:21 2017 -0400
# Node ID 1e917448c8d5c36f19c41bc83793df3e92477ede
# Parent  8299eb9b08c79699f496717d626842b72fa5ca4f
identify: add template support

This is based on a patch proposed last year by Mathias De Maré[1], with a few
changes.

  - Tags and bookmarks are now formatted lists, for more flexible queries.
  - The templater is populated whether or not [-nibtB] is specified.  (Plain
    output is unchanged.)  This seems more consistent with other templated
    commands.
  - The 'id' property is a string, instead of a list.
  - The parents of 'wdir()' have their own list of attributes.

I left 'id' as a string because it seems very useful for generating version
info.  It's also a bit strange because the value and meaning changes depending
on whether or not --debug is passed (short vs full hash), whether the revision
is a merge or not (one hash or two, separated by a '+'), the working directory
or not (node vs p1node), and local or not (remote defaults to tip, and never has
'+').  The equivalent string built with {rev} seems much less useful, and I
couldn't think of a reasonable name, so I left it out.

The discussion seemed to be pointing towards having a list of nodes, with more
than one entry for a merge.  It seems simpler to give the nodes a name, and use
{node} for the actual commit probed, especially now that there is a virtual node
for 'wdir()'.

Yuya mentioned using fm.nested() in that thread, so I did for the parent nodes.
I'm not sure if the plan is to fill in all of the context attributes in these
items, or if these nested items should simply be made {p1node} and {p1rev}.

I used ':' as the tag separator for consistency with {tags} in the log
templater.  Likewise, bookmarks are separated by a space for consistency with
the corresponding log template.

[1] https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-August/087039.html
Yuya Nishihara - June 25, 2017, 12:19 p.m.
On Sun, 25 Jun 2017 01:23:41 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1498360161 14400
> #      Sat Jun 24 23:09:21 2017 -0400
> # Node ID 1e917448c8d5c36f19c41bc83793df3e92477ede
> # Parent  8299eb9b08c79699f496717d626842b72fa5ca4f
> identify: add template support

Great. Queued this, thanks.

> I left 'id' as a string because it seems very useful for generating version
> info.  It's also a bit strange because the value and meaning changes depending
> on whether or not --debug is passed (short vs full hash), whether the revision
> is a merge or not (one hash or two, separated by a '+'), the working directory
> or not (node vs p1node), and local or not (remote defaults to tip, and never has
> '+').  The equivalent string built with {rev} seems much less useful, and I
> couldn't think of a reasonable name, so I left it out.

I think your choice makes some sense. identify acts quite differently from the
other commands. It wouldn't be possible to make it perfectly compatible with
log-like templates.

> Yuya mentioned using fm.nested() in that thread, so I did for the parent nodes.
> I'm not sure if the plan is to fill in all of the context attributes in these
> items, or if these nested items should simply be made {p1node} and {p1rev}.

[...]

> +            for i, p in enumerate(parents):
> +                fn = fm.nested('p%d' % (i + 1))
> +                fn.startitem()
> +                fn.data(rev=p.rev())
> +                fn.data(node=p.hex())
> +                fn.end()

I meant this could be a {parents} list, which is

  fn = fm.nested('parents')
  for ...
      fn.startitem()
      ...

Each item could have fn.context(ctx=p) so that all template keywrods are
available.
Matt Harbison - June 25, 2017, 8:20 p.m.
On Sun, 25 Jun 2017 08:19:29 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sun, 25 Jun 2017 01:23:41 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1498360161 14400
>> #      Sat Jun 24 23:09:21 2017 -0400
>> # Node ID 1e917448c8d5c36f19c41bc83793df3e92477ede
>> # Parent  8299eb9b08c79699f496717d626842b72fa5ca4f
>> identify: add template support
>
...

>
>> Yuya mentioned using fm.nested() in that thread, so I did for the  
>> parent nodes.
>> I'm not sure if the plan is to fill in all of the context attributes in  
>> these
>> items, or if these nested items should simply be made {p1node} and  
>> {p1rev}.
>
> [...]
>
>> +            for i, p in enumerate(parents):
>> +                fn = fm.nested('p%d' % (i + 1))
>> +                fn.startitem()
>> +                fn.data(rev=p.rev())
>> +                fn.data(node=p.hex())
>> +                fn.end()
>
> I meant this could be a {parents} list, which is
>
>   fn = fm.nested('parents')
>   for ...
>       fn.startitem()
>       ...
>
> Each item could have fn.context(ctx=p) so that all template keywrods are
> available.

I saw that, and tried it, but I didn't think it worked.  I was expecting  
lines in the json output I guess.  But after digging in a bit more, maybe  
it's just a bug in nested formatters?  `hg branches` works fine for both  
functions and keywords.  But (on default in the hg repo):

$ ../hg id -T "{p1 % '{latesttag}\n'}"

$ ../hg id -T "{p1 % '{latesttag()}\n'}"
** unknown exception encountered, please report by visiting
[...]
   File "c:\Users\Matt\projects\hg\mercurial\templatekw.py", line 486, in  
showlatesttags
     repo, ctx = args['repo'], args['ctx']
KeyError: 'repo'
Yuya Nishihara - June 26, 2017, 12:41 a.m.
On Sun, 25 Jun 2017 16:20:46 -0400, Matt Harbison wrote:
> On Sun, 25 Jun 2017 08:19:29 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Sun, 25 Jun 2017 01:23:41 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1498360161 14400
> >> #      Sat Jun 24 23:09:21 2017 -0400
> >> # Node ID 1e917448c8d5c36f19c41bc83793df3e92477ede
> >> # Parent  8299eb9b08c79699f496717d626842b72fa5ca4f
> >> identify: add template support
> >
> ...
> 
> >
> >> Yuya mentioned using fm.nested() in that thread, so I did for the  
> >> parent nodes.
> >> I'm not sure if the plan is to fill in all of the context attributes in  
> >> these
> >> items, or if these nested items should simply be made {p1node} and  
> >> {p1rev}.
> >
> > [...]
> >
> >> +            for i, p in enumerate(parents):
> >> +                fn = fm.nested('p%d' % (i + 1))
> >> +                fn.startitem()
> >> +                fn.data(rev=p.rev())
> >> +                fn.data(node=p.hex())
> >> +                fn.end()
> >
> > I meant this could be a {parents} list, which is
> >
> >   fn = fm.nested('parents')
> >   for ...
> >       fn.startitem()
> >       ...
> >
> > Each item could have fn.context(ctx=p) so that all template keywrods are
> > available.
> 
> I saw that, and tried it, but I didn't think it worked.  I was expecting  
> lines in the json output I guess.  But after digging in a bit more, maybe  
> it's just a bug in nested formatters?

Yeah, it's the bug of formatter. I'll send a fix shortly.
Matt Harbison - June 26, 2017, 12:47 a.m.
On Sun, 25 Jun 2017 20:41:21 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sun, 25 Jun 2017 16:20:46 -0400, Matt Harbison wrote:
>> On Sun, 25 Jun 2017 08:19:29 -0400, Yuya Nishihara <yuya@tcha.org>  
>> wrote:
>>
>> > On Sun, 25 Jun 2017 01:23:41 -0400, Matt Harbison wrote:
>> >> # HG changeset patch
>> >> # User Matt Harbison <matt_harbison@yahoo.com>
>> >> # Date 1498360161 14400
>> >> #      Sat Jun 24 23:09:21 2017 -0400
>> >> # Node ID 1e917448c8d5c36f19c41bc83793df3e92477ede
>> >> # Parent  8299eb9b08c79699f496717d626842b72fa5ca4f
>> >> identify: add template support
>> >
>> ...
>>
>> >
>> >> Yuya mentioned using fm.nested() in that thread, so I did for the
>> >> parent nodes.
>> >> I'm not sure if the plan is to fill in all of the context attributes  
>> in
>> >> these
>> >> items, or if these nested items should simply be made {p1node} and
>> >> {p1rev}.
>> >
>> > [...]
>> >
>> >> +            for i, p in enumerate(parents):
>> >> +                fn = fm.nested('p%d' % (i + 1))
>> >> +                fn.startitem()
>> >> +                fn.data(rev=p.rev())
>> >> +                fn.data(node=p.hex())
>> >> +                fn.end()
>> >
>> > I meant this could be a {parents} list, which is
>> >
>> >   fn = fm.nested('parents')
>> >   for ...
>> >       fn.startitem()
>> >       ...
>> >
>> > Each item could have fn.context(ctx=p) so that all template keywrods  
>> are
>> > available.
>>
>> I saw that, and tried it, but I didn't think it worked.  I was expecting
>> lines in the json output I guess.  But after digging in a bit more,  
>> maybe
>> it's just a bug in nested formatters?
>
> Yeah, it's the bug of formatter. I'll send a fix shortly.

Actually, it seems to work if fm.context(ctx=ctx) is set too.  But now  
there's an off by one issue:

$ ../hg log -r . -T "{latesttag % '{changes}'}
913
$ ../hg id -T "{p1 % '{latesttag() % \'{changes}\'}'}"
914

(Unless the bug is fn is only looking at fm's ctx?)
Yuya Nishihara - June 26, 2017, 12:56 a.m.
On Sun, 25 Jun 2017 20:47:45 -0400, Matt Harbison wrote:
> On Sun, 25 Jun 2017 20:41:21 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Sun, 25 Jun 2017 16:20:46 -0400, Matt Harbison wrote:
> >> On Sun, 25 Jun 2017 08:19:29 -0400, Yuya Nishihara <yuya@tcha.org>  
> >> wrote:
> >>
> >> > On Sun, 25 Jun 2017 01:23:41 -0400, Matt Harbison wrote:
> >> >> # HG changeset patch
> >> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> >> # Date 1498360161 14400
> >> >> #      Sat Jun 24 23:09:21 2017 -0400
> >> >> # Node ID 1e917448c8d5c36f19c41bc83793df3e92477ede
> >> >> # Parent  8299eb9b08c79699f496717d626842b72fa5ca4f
> >> >> identify: add template support
> >> >
> >> ...
> >>
> >> >
> >> >> Yuya mentioned using fm.nested() in that thread, so I did for the
> >> >> parent nodes.
> >> >> I'm not sure if the plan is to fill in all of the context attributes  
> >> in
> >> >> these
> >> >> items, or if these nested items should simply be made {p1node} and
> >> >> {p1rev}.
> >> >
> >> > [...]
> >> >
> >> >> +            for i, p in enumerate(parents):
> >> >> +                fn = fm.nested('p%d' % (i + 1))
> >> >> +                fn.startitem()
> >> >> +                fn.data(rev=p.rev())
> >> >> +                fn.data(node=p.hex())
> >> >> +                fn.end()
> >> >
> >> > I meant this could be a {parents} list, which is
> >> >
> >> >   fn = fm.nested('parents')
> >> >   for ...
> >> >       fn.startitem()
> >> >       ...
> >> >
> >> > Each item could have fn.context(ctx=p) so that all template keywrods  
> >> are
> >> > available.
> >>
> >> I saw that, and tried it, but I didn't think it worked.  I was expecting
> >> lines in the json output I guess.  But after digging in a bit more,  
> >> maybe
> >> it's just a bug in nested formatters?
> >
> > Yeah, it's the bug of formatter. I'll send a fix shortly.
> 
> Actually, it seems to work if fm.context(ctx=ctx) is set too.  But now  
> there's an off by one issue:
> 
> $ ../hg log -r . -T "{latesttag % '{changes}'}
> 913
> $ ../hg id -T "{p1 % '{latesttag() % \'{changes}\'}'}"
> 914
> 
> (Unless the bug is fn is only looking at fm's ctx?)

That depends on how the templater processes nested items. Outer template dict
is just _copied_ and updated by inner items.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2665,7 +2665,7 @@ 
     ('b', 'branch', None, _('show branch')),
     ('t', 'tags', None, _('show tags')),
     ('B', 'bookmarks', None, _('show bookmarks')),
-    ] + remoteopts,
+    ] + remoteopts + formatteropts,
     _('[-nibtB] [-r REV] [SOURCE]'),
     optionalrepo=True)
 def identify(ui, repo, source=None, rev=None,
@@ -2724,6 +2724,9 @@ 
         repo = peer.local()
         revs, checkout = hg.addbranchrevs(repo, peer, branches, None)
 
+    fm = ui.formatter('identify', opts)
+    fm.startitem()
+
     if not repo:
         if num or branch or tags:
             raise error.Abort(
@@ -2734,8 +2737,10 @@ 
             rev = "tip"
 
         remoterev = peer.lookup(rev)
+        hexrev = hexfunc(remoterev)
         if default or id:
-            output = [hexfunc(remoterev)]
+            output = [hexrev]
+        fm.data(id=hexrev)
 
         def getbms():
             bms = []
@@ -2747,13 +2752,17 @@ 
 
             return sorted(bms)
 
+        bms = getbms()
         if bookmarks:
-            output.extend(getbms())
+            output.extend(bms)
         elif default and not ui.quiet:
             # multiple bookmarks for a single parent separated by '/'
-            bm = '/'.join(getbms())
+            bm = '/'.join(bms)
             if bm:
                 output.append(bm)
+
+        fm.data(node=hex(remoterev))
+        fm.data(bookmarks=fm.formatlist(bms, name='bookmark'))
     else:
         ctx = scmutil.revsingle(repo, rev, None)
 
@@ -2765,20 +2774,32 @@ 
                 taglist.extend(p.tags())
 
             changed = ""
-            if default or id or num:
-                if (any(repo.status())
-                    or any(ctx.sub(s).dirty() for s in ctx.substate)):
-                    changed = '+'
+            if (any(repo.status())
+                or any(ctx.sub(s).dirty() for s in ctx.substate)):
+                changed = '+'
+            fm.data(changed=changed)
+
+            hexoutput = [hexfunc(p.node()) for p in parents]
             if default or id:
-                output = ["%s%s" %
-                  ('+'.join([hexfunc(p.node()) for p in parents]), changed)]
+                output = ["%s%s" % ('+'.join(hexoutput), changed)]
+            fm.data(id="%s%s" % ('+'.join(hexoutput), changed))
+
             if num:
-                output.append("%s%s" %
-                  ('+'.join([pycompat.bytestr(p.rev()) for p in parents]),
-                                                                    changed))
+                numoutput = [pycompat.bytestr(p.rev()) for p in parents]
+                output.append("%s%s" % ('+'.join(numoutput), changed))
+
+            for i, p in enumerate(parents):
+                fn = fm.nested('p%d' % (i + 1))
+                fn.startitem()
+                fn.data(rev=p.rev())
+                fn.data(node=p.hex())
+                fn.end()
         else:
+            hexoutput = hexfunc(ctx.node())
             if default or id:
-                output = [hexfunc(ctx.node())]
+                output = [hexoutput]
+            fm.data(id=hexoutput)
+
             if num:
                 output.append(pycompat.bytestr(ctx.rev()))
             taglist = ctx.tags()
@@ -2807,7 +2828,13 @@ 
             if bookmarks:
                 output.extend(ctx.bookmarks())
 
-    ui.write("%s\n" % ' '.join(output))
+        fm.data(node=ctx.hex())
+        fm.data(branch=ctx.branch())
+        fm.data(tags=fm.formatlist(taglist, name='tag', sep=':'))
+        fm.data(bookmarks=fm.formatlist(ctx.bookmarks(), name='bookmark'))
+
+    fm.plain("%s\n" % ' '.join(output))
+    fm.end()
 
 @command('import|patch',
     [('p', 'strip', 1,
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -295,7 +295,7 @@ 
   grep: print0, all, text, follow, ignore-case, files-with-matches, line-number, rev, user, date, template, include, exclude
   heads: rev, topo, active, closed, style, template
   help: extension, command, keyword, system
-  identify: rev, num, id, branch, tags, bookmarks, ssh, remotecmd, insecure
+  identify: rev, num, id, branch, tags, bookmarks, ssh, remotecmd, insecure, template
   import: strip, base, edit, force, no-commit, bypass, partial, exact, prefix, import-branch, message, logfile, date, user, similarity
   incoming: force, newest-first, bundle, rev, bookmarks, branch, patch, git, limit, no-merges, stat, graph, style, template, ssh, remotecmd, insecure, subrepos
   locate: rev, print0, fullpath, include, exclude
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -95,7 +95,7 @@ 
 test generic hooks
 
   $ hg id
-  pre-identify hook: HG_ARGS=id HG_HOOKNAME=pre-identify HG_HOOKTYPE=pre-identify HG_OPTS={'bookmarks': None, 'branch': None, 'id': None, 'insecure': None, 'num': None, 'remotecmd': '', 'rev': '', 'ssh': '', 'tags': None} HG_PATS=[]
+  pre-identify hook: HG_ARGS=id HG_HOOKNAME=pre-identify HG_HOOKTYPE=pre-identify HG_OPTS={'bookmarks': None, 'branch': None, 'id': None, 'insecure': None, 'num': None, 'remotecmd': '', 'rev': '', 'ssh': '', 'tags': None, 'template': ''} HG_PATS=[]
   abort: pre-identify hook exited with status 1
   [255]
   $ hg cat b
diff --git a/tests/test-identify.t b/tests/test-identify.t
--- a/tests/test-identify.t
+++ b/tests/test-identify.t
@@ -43,12 +43,36 @@ 
   cb9a9f314b8b
   $ hg id -n -t -b -i
   cb9a9f314b8b 0 default tip
+  $ hg id -Tjson
+  [
+   {
+    "bookmarks": [],
+    "branch": "default",
+    "changed": "",
+    "id": "cb9a9f314b8b",
+    "node": "ffffffffffffffffffffffffffffffffffffffff",
+    "p1": [{"node": "cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b", "rev": 0}],
+    "tags": ["tip"]
+   }
+  ]
 
 with modifications
 
   $ echo b > a
   $ hg id -n -t -b -i
   cb9a9f314b8b+ 0+ default tip
+  $ hg id -Tjson
+  [
+   {
+    "bookmarks": [],
+    "branch": "default",
+    "changed": "+",
+    "id": "cb9a9f314b8b+",
+    "node": "ffffffffffffffffffffffffffffffffffffffff",
+    "p1": [{"node": "cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b", "rev": 0}],
+    "tags": ["tip"]
+   }
+  ]
 
 other local repo
 
diff --git a/tests/test-merge-default.t b/tests/test-merge-default.t
--- a/tests/test-merge-default.t
+++ b/tests/test-merge-default.t
@@ -49,6 +49,19 @@ 
   $ hg merge 2
   0 files updated, 1 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
+  $ hg id -Tjson
+  [
+   {
+    "bookmarks": [],
+    "branch": "default",
+    "changed": "+",
+    "id": "f25cbe84d8b3+2d95304fed5d+",
+    "node": "ffffffffffffffffffffffffffffffffffffffff",
+    "p1": [{"node": "f25cbe84d8b320e298e7703f18a25a3959518c23", "rev": 4}],
+    "p2": [{"node": "2d95304fed5d89bc9d70b2a0d02f0d567469c3ab", "rev": 2}],
+    "tags": ["tip"]
+   }
+  ]
   $ hg commit -mm1
 
 Should succeed - 2 heads:
@@ -65,6 +78,17 @@ 
   (branch merge, don't forget to commit)
   $ hg commit -mm2
 
+  $ hg id -r 1 -Tjson
+  [
+   {
+    "bookmarks": [],
+    "branch": "default",
+    "id": "1846eede8b68",
+    "node": "1846eede8b6886d8cc8a88c96a687b7fe8f3b9d1",
+    "tags": []
+   }
+  ]
+
 Should fail because at tip:
 
   $ hg merge