Patchwork [1,of,3] identify: change p1/p2 to a list of parents

login
register
mail settings
Submitter Yuya Nishihara
Date June 26, 2017, 1:48 a.m.
Message ID <556d2afe74385fe2a6dd.1498441682@mimosa>
Download mbox | patch
Permalink /patch/21716/
State Accepted
Headers show

Comments

Yuya Nishihara - June 26, 2017, 1:48 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1498436335 -32400
#      Mon Jun 26 09:18:55 2017 +0900
# Node ID 556d2afe74385fe2a6dd7cdff5651080a827813f
# Parent  a49ab7f5e7e765a94a1dfab2ee3b1da695789eb6
identify: change p1/p2 to a list of parents

It makes sense because the nested data structure is a list of items.
Matt Harbison - June 26, 2017, 3:55 a.m.
On Sun, 25 Jun 2017 21:48:02 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1498436335 -32400
> #      Mon Jun 26 09:18:55 2017 +0900
> # Node ID 556d2afe74385fe2a6dd7cdff5651080a827813f
> # Parent  a49ab7f5e7e765a94a1dfab2ee3b1da695789eb6
> identify: change p1/p2 to a list of parents
>
> It makes sense because the nested data structure is a list of items.

Having them rolled into one list makes it a little more difficult to  
access in a -T pattern, but maybe that's OK.  (I know nothing about the  
json parsing aspect.)

What I'm hoping to do is build the version string with the template.  The  
closest I came is:

$ ../hg id -T "{latesttag}{if('{dirty}', '+{latesttag % \'{changes}\'}',  
'+{parents % \'{latesttag % \"{changes}\"}\'}')}-{id}{if('{dirty}',  
'{date(date, \"%Y%m%d\")}')}"

Yikes.  This won't work for a (clean) merge revision, since there's no way  
to select only p1.  (And a future subscript operator probably makes this  
more obscure.)  The right thing to do is probably for setup.py to format  
an .hg_archival.txt type file, and then the separate handling there can  
disappear.  But that doesn't really simplify the pattern.

A few years ago you suggested something like '-r "heads(. or wdir() &  
file('glob:**'))" to select the revision based on dirty or not, to  
simplify the template.  But that's blind to '!' files, and thinking beyond  
Mercurial, also doesn't see subrepo changes either.

I'm not sure if it's reasonable to roll this up into a {version()}  
template for this command, to make it easier for 3rd party usage.  My  
concern would be that pip (I think) complains about the current version  
format.  I'm not sure if we will need to change it at some point.  And  
really, it's just the {changes} bit that's complicated here.

Thoughts?
Yuya Nishihara - June 26, 2017, 1:17 p.m.
On Sun, 25 Jun 2017 23:55:26 -0400, Matt Harbison wrote:
> On Sun, 25 Jun 2017 21:48:02 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1498436335 -32400
> > #      Mon Jun 26 09:18:55 2017 +0900
> > # Node ID 556d2afe74385fe2a6dd7cdff5651080a827813f
> > # Parent  a49ab7f5e7e765a94a1dfab2ee3b1da695789eb6
> > identify: change p1/p2 to a list of parents
> >
> > It makes sense because the nested data structure is a list of items.
> 
> Having them rolled into one list makes it a little more difficult to  
> access in a -T pattern, but maybe that's OK.  (I know nothing about the  
> json parsing aspect.)
> 
> What I'm hoping to do is build the version string with the template.  The  
> closest I came is:
> 
> $ ../hg id -T "{latesttag}{if('{dirty}', '+{latesttag % \'{changes}\'}',  
> '+{parents % \'{latesttag % \"{changes}\"}\'}')}-{id}{if('{dirty}',  
> '{date(date, \"%Y%m%d\")}')}"
> 
> Yikes.  This won't work for a (clean) merge revision, since there's no way  
> to select only p1.

You could use revset(".").

  hg identify -T '{revset(".") % "{latesttag % "{changes}"}"}\n'

Well, it doesn't look nice, we might want map operator for non-list so we
can do '{p1rev % "{latesttag ...}"}' for example. But the point of this patch
is that {p1} and {p2} are not lists. {parents} is.
Matt Harbison - June 28, 2017, 1:44 a.m.
On Mon, 26 Jun 2017 09:17:59 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sun, 25 Jun 2017 23:55:26 -0400, Matt Harbison wrote:
>> On Sun, 25 Jun 2017 21:48:02 -0400, Yuya Nishihara <yuya@tcha.org>  
>> wrote:
>>
>> > # HG changeset patch
>> > # User Yuya Nishihara <yuya@tcha.org>
>> > # Date 1498436335 -32400
>> > #      Mon Jun 26 09:18:55 2017 +0900
>> > # Node ID 556d2afe74385fe2a6dd7cdff5651080a827813f
>> > # Parent  a49ab7f5e7e765a94a1dfab2ee3b1da695789eb6
>> > identify: change p1/p2 to a list of parents
>> >
>> > It makes sense because the nested data structure is a list of items.
>>
>> Having them rolled into one list makes it a little more difficult to
>> access in a -T pattern, but maybe that's OK.  (I know nothing about the
>> json parsing aspect.)
>>
>> What I'm hoping to do is build the version string with the template.   
>> The
>> closest I came is:
>>
>> $ ../hg id -T "{latesttag}{if('{dirty}', '+{latesttag % \'{changes}\'}',
>> '+{parents % \'{latesttag % \"{changes}\"}\'}')}-{id}{if('{dirty}',
>> '{date(date, \"%Y%m%d\")}')}"
>>
>> Yikes.  This won't work for a (clean) merge revision, since there's no  
>> way
>> to select only p1.
>
> You could use revset(".").
>
>   hg identify -T '{revset(".") % "{latesttag % "{changes}"}"}\n'
>
> Well, it doesn't look nice, we might want map operator for non-list so we
> can do '{p1rev % "{latesttag ...}"}' for example. But the point of this  
> patch
> is that {p1} and {p2} are not lists. {parents} is.

OK.  IDK how to implement the map operator, but this works, thanks.

Related to this example, I noticed that the RHS of 'revset(..) %' isn't  
reevaluated for fields injected with fm.data().  I'm guessing it's caching  
for performance.  But:

$ ../hg identify -r '4.2' -T '{revset("tagged() and rev(%d)::", rev) %  
"{rev}\n"}'
35618
36319

vs an injected field:

$ ../hg identify -r '4.2' -T '{revset("tagged() and rev(%d)::", rev) %  
"{tags % "{tag}"}\n"}'
4.2
4.2

That query works fine if I comment out 'fm.data(tags=...)':

$ ../hg identify -r '4.2' -T '{revset("tagged() and rev(%d)::", rev) %  
"{tags % "{tag}"}\n"}'
4.2
4.2.1

Maybe it's a small reason to automatically populate the ctx attributes in  
json, so that these don't need to be manually set in the formatter?

(I saw that github shows the descendant tags of a commit, and wanted to  
see if I could (ab)use the existing templates to get the same results  
without writing python code for it.  I realize that log would be a better  
command for this query.)
Yuya Nishihara - June 28, 2017, 2:14 p.m.
On Tue, 27 Jun 2017 21:44:35 -0400, Matt Harbison wrote:
> On Mon, 26 Jun 2017 09:17:59 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> Related to this example, I noticed that the RHS of 'revset(..) %' isn't  
> reevaluated for fields injected with fm.data().  I'm guessing it's caching  
> for performance.  But:
> 
> $ ../hg identify -r '4.2' -T '{revset("tagged() and rev(%d)::", rev) %  
> "{rev}\n"}'
> 35618
> 36319
> 
> vs an injected field:
> 
> $ ../hg identify -r '4.2' -T '{revset("tagged() and rev(%d)::", rev) %  
> "{tags % "{tag}"}\n"}'
> 4.2
> 4.2

Good catch. Can you file a bug report? I don't think I can work on this
issue right now.

> That query works fine if I comment out 'fm.data(tags=...)':
> 
> $ ../hg identify -r '4.2' -T '{revset("tagged() and rev(%d)::", rev) %  
> "{tags % "{tag}"}\n"}'
> 4.2
> 4.2.1
> 
> Maybe it's a small reason to automatically populate the ctx attributes in  
> json, so that these don't need to be manually set in the formatter?

Perhaps. More complete fix would be to make templater drop overrides of
ctx-deriving keywords, but that isn't easy because keyword functions live
in the same dict.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2790,12 +2790,12 @@  def identify(ui, repo, source=None, rev=
                 numoutput = ["%d" % p.rev() for p in parents]
                 output.append("%s%s" % ('+'.join(numoutput), dirty))
 
-            for i, p in enumerate(parents):
-                fn = fm.nested('p%d' % (i + 1))
+            fn = fm.nested('parents')
+            for p in parents:
                 fn.startitem()
                 fn.data(rev=p.rev())
                 fn.data(node=p.hex())
-                fn.end()
+            fn.end()
         else:
             hexoutput = hexfunc(ctx.node())
             if default or id:
diff --git a/tests/test-identify.t b/tests/test-identify.t
--- a/tests/test-identify.t
+++ b/tests/test-identify.t
@@ -51,7 +51,7 @@  with options
     "dirty": "",
     "id": "cb9a9f314b8b",
     "node": "ffffffffffffffffffffffffffffffffffffffff",
-    "p1": [{"node": "cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b", "rev": 0}],
+    "parents": [{"node": "cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b", "rev": 0}],
     "tags": ["tip"]
    }
   ]
@@ -69,7 +69,7 @@  with modifications
     "dirty": "+",
     "id": "cb9a9f314b8b+",
     "node": "ffffffffffffffffffffffffffffffffffffffff",
-    "p1": [{"node": "cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b", "rev": 0}],
+    "parents": [{"node": "cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b", "rev": 0}],
     "tags": ["tip"]
    }
   ]
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
@@ -57,8 +57,7 @@  Should succeed:
     "dirty": "+",
     "id": "f25cbe84d8b3+2d95304fed5d+",
     "node": "ffffffffffffffffffffffffffffffffffffffff",
-    "p1": [{"node": "f25cbe84d8b320e298e7703f18a25a3959518c23", "rev": 4}],
-    "p2": [{"node": "2d95304fed5d89bc9d70b2a0d02f0d567469c3ab", "rev": 2}],
+    "parents": [{"node": "f25cbe84d8b320e298e7703f18a25a3959518c23", "rev": 4}, {"node": "2d95304fed5d89bc9d70b2a0d02f0d567469c3ab", "rev": 2}],
     "tags": ["tip"]
    }
   ]