Patchwork D1293: rebase: use fm.formatlist() and fm.formatdict() to support user template

login
register
mail settings
Submitter phabricator
Date Nov. 2, 2017, 8:23 p.m.
Message ID <differential-rev-PHID-DREV-dblcersee2sdxwp5svb6-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25358/
State Superseded
Headers show

Comments

phabricator - Nov. 2, 2017, 8:23 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Thanks to Yuya for suggesting this in https://phab.mercurial-scm.org/D1173.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1293

AFFECTED FILES
  hgext/rebase.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 4, 2017, 7:50 a.m.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  Can you add a test that should be fixed by this patch?
  Perhaps that will show we'll probably want to set the name of dict item pair
  other than the default 'key'/'value'.

INLINE COMMENTS

> rebase.py:1550
> +        fd = fm.formatdict
> +        nodechanges = fd({hf(oldn): fl([hf(n) for n in newn], 'succs')
> +                          for oldn, newn in replacements.iteritems()})

The name argument is for each item, not for the list. Perhaps,
it should be name='node' (or name ='succ').

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1293

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Nov. 8, 2017, 5:03 p.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D1293#21761, @yuja wrote:
  
  > Can you add a test that should be fixed by this patch?
  >  Perhaps that will show we'll probably want to set the name of dict item pair
  >  other than the default 'key'/'value'.
  
  
  Sorry but I am unable to understand the test part. Can you point me to an example?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1293

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Nov. 9, 2017, 12:12 p.m.
yuja added a comment.


  Something like this:
  
  -T '{nodechanges % ": {value % ...}"}'

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1293

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Nov. 21, 2017, 1:41 a.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D1293#22323, @yuja wrote:
  
  > Something like this:
  >
  > -T '{nodechanges % ": {value % ...}"}'
  
  
  I have added a test. I am not sure whether that was what you wanted.
  
  (Fun fact: I was trying various permutations of `{nodechanges % ":{value % ...}"}` and gave up last night. When I quoted your comment to say I am unable to come up with a case, I saw you had `{key}` in your comment too which was invisible. )

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1293

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Nov. 21, 2017, 12:38 p.m.
yuja added a comment.


  > I am not sure whether that was what you wanted.
  
  It's getting closer, thanks. What I meant is `{key}` and `{value}` wouldn't be good
  keyword names because the they are actually an old node and a list of new nodes
  respectively.
  
  I think a template should be expressed something like this:
  
    `{oldnode}: {newnodes % '{node} '}`

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1293

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Nov. 22, 2017, 1:18 p.m.
yuja added a comment.


  Queued the series, many thanks.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1293

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -21,7 +21,6 @@ 
 
 from mercurial.i18n import _
 from mercurial.node import (
-    hex,
     nullid,
     nullrev,
     short,
@@ -1545,8 +1544,11 @@ 
                 replacements[oldnode] = succs
     scmutil.cleanupnodes(repo, replacements, 'rebase', moves)
     if fm:
-        nodechanges = {hex(oldn): [hex(n) for n in newn]
-                       for oldn, newn in replacements.iteritems()}
+        hf = fm.hexfunc
+        fl = fm.formatlist
+        fd = fm.formatdict
+        nodechanges = fd({hf(oldn): fl([hf(n) for n in newn], 'succs')
+                          for oldn, newn in replacements.iteritems()})
         fm.data(nodechanges=nodechanges)
 
 def pullrebase(orig, ui, repo, *args, **opts):