Patchwork [3,of,4] commands: add template support for identify

login
register
mail settings
Submitter Mathias De Maré
Date Aug. 8, 2016, 3:54 p.m.
Message ID <74056050d1bd9069f0df.1470671685@waste.org>
Download mbox | patch
Permalink /patch/16205/
State Changes Requested
Headers show

Comments

Mathias De Maré - Aug. 8, 2016, 3:54 p.m.
# HG changeset patch
# User Mathias De Maré <mathias.demare@gmail.com>
# Date 1470200890 -7200
#      Wed Aug 03 07:08:10 2016 +0200
# Node ID 74056050d1bd9069f0dfaed861162ee65a77032d
# Parent  2a711c2a71c1e5e562fc988cfe9011ebf3d8e6e9
commands: add template support for identify

Example output:
$ hg id -Tjson
[
 {
  "bookmarks": ["bar"],
  "branch": "stable",
  "changed": "",
  "id": ["5ebe437b3392"],
  "tags": ["tip"]
 }
]
Matt Harbison - Aug. 9, 2016, 1:08 a.m.
On Mon, 08 Aug 2016 11:54:45 -0400, Mathias De Maré  
<mathias.demare@gmail.com> wrote:

> # HG changeset patch
> # User Mathias De Maré <mathias.demare@gmail.com>
> # Date 1470200890 -7200
> #      Wed Aug 03 07:08:10 2016 +0200
> # Node ID 74056050d1bd9069f0dfaed861162ee65a77032d
> # Parent  2a711c2a71c1e5e562fc988cfe9011ebf3d8e6e9
> commands: add template support for identify
>
> Example output:
> $ hg id -Tjson
> [
>  {
>   "bookmarks": ["bar"],
>   "branch": "stable",
>   "changed": "",
>   "id": ["5ebe437b3392"],
>   "tags": ["tip"]
>  }
> ]

A subtle issue that I noticed: `hg id` with an uncommitted merge lists  
both nodes:

   $ hg id
   5af771ba5a5f+b8f9cdca8807+ tip

However, `hg id -T "{id}\n"` sticks them together without the '+' in  
between:

   $ ../hg id -T "{id}\n"
   5af771ba5a5fb8f9cdca8807

And it doesn't look like the % operator works, so you can't manually put  
"changed" into the template, even though -Tjson seems to print "id" as a  
list:

   $ ../hg id -T "{ids % {id}}\n"
   hg: parse error at 7: syntax error
   $ ../hg id -Tjson
   [
    {
     "changed": "+",
     "id": ["5af771ba5a5f", "b8f9cdca8807"],
     "tags": ["tip"]
    }
   ]


Also, not really a direct problem with this patch, though it opens the  
door to it.  I saw the {tags} keyword, and tried using {latesttags()} from  
the log templates.  This prints a stacktrace.  It's not clear to me to  
what extent these templates are expected to work together.

It would probably be nice in the future to have keywords like  
{latesttags}, {changessincelatesttag}, etc on `hg id` to be able to build  
a string like `hg version`, but simpler than the magic currently required  
when using `hg log` to do the same.  Also, I assume we can add {node} in  
the future, so that you don't need to use --debug to get the full hash?


> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4637,7 +4637,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,
> @@ -4688,6 +4688,8 @@
>      default = not (num or id or branch or tags or bookmarks)
>      output = []
>      revs = []
> +    fm = ui.formatter('identify', opts)
> +    fm.startitem()
>     if source:
>          source, branches = hg.parseurl(ui.expandpath(source))
> @@ -4706,7 +4708,9 @@
>         remoterev = peer.lookup(rev)
>          if default or id:
> -            output = [hexfunc(remoterev)]
> +            hexrev = hexfunc(remoterev)
> +            output = [hexrev]
> +            fm.data(id=hexrev)
>         def getbms():
>              bms = []
> @@ -4719,12 +4723,16 @@
>              return sorted(bms)
>         if bookmarks:
> -            output.extend(getbms())
> +            bms = getbms()
> +            output.extend(bms)
> +            fm.data(bookmarks=bms)
>          elif default and not ui.quiet:
>              # multiple bookmarks for a single parent separated by '/'
> -            bm = '/'.join(getbms())
> +            bms = getbms()
> +            bm = '/'.join(bms)
>              if bm:
>                  output.append(bm)
> +                fm.data(bookmarks=bms)
>      else:
>          ctx = scmutil.revsingle(repo, rev, None)
> @@ -4740,44 +4748,58 @@
>                  if (any(repo.status())
>                      or any(ctx.sub(s).dirty() for s in ctx.substate)):
>                      changed = '+'
> +                fm.data(changed=changed)
>              if default or id:
> -                output = ["%s%s" %
> -                  ('+'.join([hexfunc(p.node()) for p in parents]),  
> changed)]
> +                hexoutput = [hexfunc(p.node()) for p in parents]
> +                output = ["%s%s" % ('+'.join(hexoutput), changed)]
> +                fm.data(id=hexoutput)
>              if num:
> -                output.append("%s%s" %
> -                  ('+'.join([str(p.rev()) for p in parents]), changed))
> +                numoutput = [str(p.rev()) for p in parents]
> +                output.append("%s%s" % ('+'.join(numoutput), changed))
> +                fm.data(num=numoutput)
>          else:
>              if default or id:
> -                output = [hexfunc(ctx.node())]
> +                hexoutput = hexfunc(ctx.node())
> +                output = [hexoutput]
> +                fm.data(id=hexoutput)
>              if num:
> -                output.append(str(ctx.rev()))
> +                numoutput = str(ctx.rev())
> +                output.append(numoutput)
> +                fm.data(num=numoutput)
>              taglist = ctx.tags()
>         if default and not ui.quiet:
>              b = ctx.branch()
>              if b != 'default':
>                  output.append("(%s)" % b)
> +                fm.data(branch=b)
>             # multiple tags for a single parent separated by '/'
>              t = '/'.join(taglist)
>              if t:
>                  output.append(t)
> +                fm.data(tags=taglist)
>             # multiple bookmarks for a single parent separated by '/'
>              bm = '/'.join(ctx.bookmarks())
>              if bm:
>                  output.append(bm)
> +                fm.data(bookmarks=ctx.bookmarks())
>          else:
>              if branch:
>                  output.append(ctx.branch())
> +                fm.data(branch=ctx.branch())
>             if tags:
>                  output.extend(taglist)
> +                fm.data(tags=taglist)
>             if bookmarks:
>                  output.extend(ctx.bookmarks())
> -
> -    ui.write("%s\n" % ' '.join(output))
> +                fm.data(bookmarks=ctx.bookmarks())
> +
> +    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
> @@ -281,7 +281,7 @@
>    grep: print0, all, text, follow, ignore-case, files-with-matches,  
> line-number, rev, user, date, 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_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_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,6 +43,16 @@
>    cb9a9f314b8b
>    $ hg id -n -t -b -i
>    cb9a9f314b8b 0 default tip
> +  $ hg id -n -t -b -i -Tjson
> +  [
> +   {
> +    "branch": "default",
> +    "changed": "",
> +    "id": ["cb9a9f314b8b"],
> +    "num": ["0"],
> +    "tags": ["tip"]
> +   }
> +  ]
> with modifications
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Aug. 9, 2016, 1:34 p.m.
On Mon, 08 Aug 2016 21:08:39 -0400, Matt Harbison wrote:
> On Mon, 08 Aug 2016 11:54:45 -0400, Mathias De Maré  
> <mathias.demare@gmail.com> wrote:
> > # HG changeset patch
> > # User Mathias De Maré <mathias.demare@gmail.com>
> > # Date 1470200890 -7200
> > #      Wed Aug 03 07:08:10 2016 +0200
> > # Node ID 74056050d1bd9069f0dfaed861162ee65a77032d
> > # Parent  2a711c2a71c1e5e562fc988cfe9011ebf3d8e6e9
> > commands: add template support for identify
> >
> > Example output:
> > $ hg id -Tjson
> > [
> >  {
> >   "bookmarks": ["bar"],
> >   "branch": "stable",
> >   "changed": "",
> >   "id": ["5ebe437b3392"],

It's generally called as a "node" or "nodes".

> A subtle issue that I noticed: `hg id` with an uncommitted merge lists  
> both nodes:
> 
>    $ hg id
>    5af771ba5a5f+b8f9cdca8807+ tip
> 
> However, `hg id -T "{id}\n"` sticks them together without the '+' in  
> between:
> 
>    $ ../hg id -T "{id}\n"
>    5af771ba5a5fb8f9cdca8807
> 
> And it doesn't look like the % operator works, so you can't manually put  
> "changed" into the template, even though -Tjson seems to print "id" as a
> list:
> 
>    $ ../hg id -T "{ids % {id}}\n"
>    hg: parse error at 7: syntax error
>    $ ../hg id -Tjson
>    [
>     {
>      "changed": "+",
>      "id": ["5af771ba5a5f", "b8f9cdca8807"],
>      "tags": ["tip"]
>     }
>    ]

Good catch. We can't put a plain list into formatter. That's why I added
fm.formatlist().

> Also, not really a direct problem with this patch, though it opens the  
> door to it.  I saw the {tags} keyword, and tried using {latesttags()} from  
> the log templates.  This prints a stacktrace.  It's not clear to me to  
> what extent these templates are expected to work together.
> 
> It would probably be nice in the future to have keywords like  
> {latesttags}, {changessincelatesttag}, etc on `hg id` to be able to build  
> a string like `hg version`, but simpler than the magic currently required  
> when using `hg log` to do the same.  Also, I assume we can add {node} in
> the future, so that you don't need to use --debug to get the full hash?

Yeah, I'm thinking of an API to associate ctx with the current item. In order
to do that, each item should be mapped to a single revision, which means
we'll have to build [{"node": x}, ...] instead of [{"nodes": x, ...}].
Mathias De Maré - Aug. 29, 2016, 3:44 p.m.
On Tue, Aug 9, 2016 at 3:34 PM, Yuya Nishihara <yuya@tcha.org> wrote:

> On Mon, 08 Aug 2016 21:08:39 -0400, Matt Harbison wrote:
> > On Mon, 08 Aug 2016 11:54:45 -0400, Mathias De Maré
> > <mathias.demare@gmail.com> wrote:
> > > # HG changeset patch
> > > # User Mathias De Maré <mathias.demare@gmail.com>
> > > # Date 1470200890 -7200
> > > #      Wed Aug 03 07:08:10 2016 +0200
> > > # Node ID 74056050d1bd9069f0dfaed861162ee65a77032d
> > > # Parent  2a711c2a71c1e5e562fc988cfe9011ebf3d8e6e9
> > > commands: add template support for identify
> > >
> > > Example output:
> > > $ hg id -Tjson
> > > [
> > >  {
> > >   "bookmarks": ["bar"],
> > >   "branch": "stable",
> > >   "changed": "",
> > >   "id": ["5ebe437b3392"],
>
> It's generally called as a "node" or "nodes".
>
Yes, I called it 'id' in this patch because of the '--id' flag. I agree,
'node'/'nodes' would be a better option.

>
> > A subtle issue that I noticed: `hg id` with an uncommitted merge lists
> > both nodes:
> >
> >    $ hg id
> >    5af771ba5a5f+b8f9cdca8807+ tip
> >
> > However, `hg id -T "{id}\n"` sticks them together without the '+' in
> > between:
> >
> >    $ ../hg id -T "{id}\n"
> >    5af771ba5a5fb8f9cdca8807
> >
> > And it doesn't look like the % operator works, so you can't manually put
> > "changed" into the template, even though -Tjson seems to print "id" as a
> > list:
> >
> >    $ ../hg id -T "{ids % {id}}\n"
> >    hg: parse error at 7: syntax error
> >    $ ../hg id -Tjson
> >    [
> >     {
> >      "changed": "+",
> >      "id": ["5af771ba5a5f", "b8f9cdca8807"],
> >      "tags": ["tip"]
> >     }
> >    ]
>
> Good catch. We can't put a plain list into formatter. That's why I added
> fm.formatlist().
>
> > Also, not really a direct problem with this patch, though it opens the
> > door to it.  I saw the {tags} keyword, and tried using {latesttags()}
> from
> > the log templates.  This prints a stacktrace.  It's not clear to me to
> > what extent these templates are expected to work together.
> >
> > It would probably be nice in the future to have keywords like
> > {latesttags}, {changessincelatesttag}, etc on `hg id` to be able to build
> > a string like `hg version`, but simpler than the magic currently required
> > when using `hg log` to do the same.  Also, I assume we can add {node} in
> > the future, so that you don't need to use --debug to get the full hash?
>
> Yeah, I'm thinking of an API to associate ctx with the current item. In
> order
> to do that, each item should be mapped to a single revision, which means
> we'll have to build [{"node": x}, ...] instead of [{"nodes": x, ...}].
>

Do you prefer I solve the above listed issues and resubmit? Or is this
patch in general not in the right direction?

Greetings,
Mathias
Yuya Nishihara - Aug. 30, 2016, 2:10 p.m.
On Mon, 29 Aug 2016 17:44:44 +0200, Mathias De Maré wrote:
> On Tue, Aug 9, 2016 at 3:34 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Mon, 08 Aug 2016 21:08:39 -0400, Matt Harbison wrote:
> > > Also, not really a direct problem with this patch, though it opens the
> > > door to it.  I saw the {tags} keyword, and tried using {latesttags()}
> > from
> > > the log templates.  This prints a stacktrace.  It's not clear to me to
> > > what extent these templates are expected to work together.
> > >
> > > It would probably be nice in the future to have keywords like
> > > {latesttags}, {changessincelatesttag}, etc on `hg id` to be able to build
> > > a string like `hg version`, but simpler than the magic currently required
> > > when using `hg log` to do the same.  Also, I assume we can add {node} in
> > > the future, so that you don't need to use --debug to get the full hash?
> >
> > Yeah, I'm thinking of an API to associate ctx with the current item. In
> > order
> > to do that, each item should be mapped to a single revision, which means
> > we'll have to build [{"node": x}, ...] instead of [{"nodes": x, ...}].
> 
> Do you prefer I solve the above listed issues and resubmit? Or is this
> patch in general not in the right direction?

Try fm.nested() to build a list of dicts. Full templating support can be
added later.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4637,7 +4637,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,
@@ -4688,6 +4688,8 @@ 
     default = not (num or id or branch or tags or bookmarks)
     output = []
     revs = []
+    fm = ui.formatter('identify', opts)
+    fm.startitem()
 
     if source:
         source, branches = hg.parseurl(ui.expandpath(source))
@@ -4706,7 +4708,9 @@ 
 
         remoterev = peer.lookup(rev)
         if default or id:
-            output = [hexfunc(remoterev)]
+            hexrev = hexfunc(remoterev)
+            output = [hexrev]
+            fm.data(id=hexrev)
 
         def getbms():
             bms = []
@@ -4719,12 +4723,16 @@ 
             return sorted(bms)
 
         if bookmarks:
-            output.extend(getbms())
+            bms = getbms()
+            output.extend(bms)
+            fm.data(bookmarks=bms)
         elif default and not ui.quiet:
             # multiple bookmarks for a single parent separated by '/'
-            bm = '/'.join(getbms())
+            bms = getbms()
+            bm = '/'.join(bms)
             if bm:
                 output.append(bm)
+                fm.data(bookmarks=bms)
     else:
         ctx = scmutil.revsingle(repo, rev, None)
 
@@ -4740,44 +4748,58 @@ 
                 if (any(repo.status())
                     or any(ctx.sub(s).dirty() for s in ctx.substate)):
                     changed = '+'
+                fm.data(changed=changed)
             if default or id:
-                output = ["%s%s" %
-                  ('+'.join([hexfunc(p.node()) for p in parents]), changed)]
+                hexoutput = [hexfunc(p.node()) for p in parents]
+                output = ["%s%s" % ('+'.join(hexoutput), changed)]
+                fm.data(id=hexoutput)
             if num:
-                output.append("%s%s" %
-                  ('+'.join([str(p.rev()) for p in parents]), changed))
+                numoutput = [str(p.rev()) for p in parents]
+                output.append("%s%s" % ('+'.join(numoutput), changed))
+                fm.data(num=numoutput)
         else:
             if default or id:
-                output = [hexfunc(ctx.node())]
+                hexoutput = hexfunc(ctx.node())
+                output = [hexoutput]
+                fm.data(id=hexoutput)
             if num:
-                output.append(str(ctx.rev()))
+                numoutput = str(ctx.rev())
+                output.append(numoutput)
+                fm.data(num=numoutput)
             taglist = ctx.tags()
 
         if default and not ui.quiet:
             b = ctx.branch()
             if b != 'default':
                 output.append("(%s)" % b)
+                fm.data(branch=b)
 
             # multiple tags for a single parent separated by '/'
             t = '/'.join(taglist)
             if t:
                 output.append(t)
+                fm.data(tags=taglist)
 
             # multiple bookmarks for a single parent separated by '/'
             bm = '/'.join(ctx.bookmarks())
             if bm:
                 output.append(bm)
+                fm.data(bookmarks=ctx.bookmarks())
         else:
             if branch:
                 output.append(ctx.branch())
+                fm.data(branch=ctx.branch())
 
             if tags:
                 output.extend(taglist)
+                fm.data(tags=taglist)
 
             if bookmarks:
                 output.extend(ctx.bookmarks())
-
-    ui.write("%s\n" % ' '.join(output))
+                fm.data(bookmarks=ctx.bookmarks())
+
+    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
@@ -281,7 +281,7 @@ 
   grep: print0, all, text, follow, ignore-case, files-with-matches, line-number, rev, user, date, 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_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_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,6 +43,16 @@ 
   cb9a9f314b8b
   $ hg id -n -t -b -i
   cb9a9f314b8b 0 default tip
+  $ hg id -n -t -b -i -Tjson
+  [
+   {
+    "branch": "default",
+    "changed": "",
+    "id": ["cb9a9f314b8b"],
+    "num": ["0"],
+    "tags": ["tip"]
+   }
+  ]
 
 with modifications