Patchwork [3,of,3] template: unify template output to use abspath (BC)

login
register
mail settings
Submitter timeless@mozdev.org
Date April 13, 2016, 5:14 p.m.
Message ID <38d467dc57b027547d6f.1460567676@waste.org>
Download mbox | patch
Permalink /patch/14589/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

timeless@mozdev.org - April 13, 2016, 5:14 p.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1460548072 0
#      Wed Apr 13 11:47:52 2016 +0000
# Node ID 38d467dc57b027547d6fd201d2b0ef5261fbd192
# Parent  4161598737bc141a062d8fa0d2c90da823e5b6c9
template: unify template output to use abspath (BC)

Before this change, annotate used "file", which was inaccurate,
the field is actually an abspath.

{file_dels}, {file_adds}, {file_copies}, {file_mods}, {files} are
changed so their % field is {abspath} instead of {file}.

{file} is retained for backwards compatibility.
Yuya Nishihara - April 15, 2016, 4:29 p.m.
On Wed, 13 Apr 2016 12:14:36 -0500, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1460548072 0
> #      Wed Apr 13 11:47:52 2016 +0000
> # Node ID 38d467dc57b027547d6fd201d2b0ef5261fbd192
> # Parent  4161598737bc141a062d8fa0d2c90da823e5b6c9
> template: unify template output to use abspath (BC)
> 
> Before this change, annotate used "file", which was inaccurate,
> the field is actually an abspath.
> 
> {file_dels}, {file_adds}, {file_copies}, {file_mods}, {files} are
> changed so their % field is {abspath} instead of {file}.
> 
> {file} is retained for backwards compatibility.

I don't see why {file} was bad, and should be encouraged to use "abspath",
even though {file} exists for years in log templates.

So I'm -0.5 for this.

> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -422,18 +422,26 @@
>          hexfn = fm.hexfunc
>          formatrev = formathex = str
>  
> +    _abspath = lambda x: x[0].path()
>      opmap = [('user', ' ', lambda x: x[0].user(), ui.shortuser),
>               ('number', ' ', lambda x: x[0].rev(), formatrev),
>               ('changeset', ' ', lambda x: hexfn(x[0].node()), formathex),
>               ('date', ' ', lambda x: x[0].date(), util.cachefunc(datefunc)),
> -             ('file', ' ', lambda x: x[0].path(), str),
> +             ('abspath', ' ', _abspath, str),
> +             ('file', None, _abspath, str),
>               ('line_number', ':', lambda x: x[1], str),
>              ]
> -    fieldnamemap = {'number': 'rev', 'changeset': 'node'}
> +    fieldnamemap = {
> +             'changeset': 'node',
> +             'file': 'abspath',
> +             'number': 'rev',
> +            }
>  
>      if (not opts.get('user') and not opts.get('changeset')
>          and not opts.get('date') and not opts.get('file')):
>          opts['number'] = True
> +    if opts.get('file'):
> +        opts['abspath'] = True
>  
>      linenumber = opts.get('line_number') is not None
>      if linenumber and (not opts.get('changeset')) and (not opts.get('number')):
> @@ -449,7 +457,9 @@
>                 if opts.get(op)]
>      funcmap[0] = (funcmap[0][0], '') # no separator in front of first column
>      fields = ' '.join(fieldnamemap.get(op, op) for op, sep, get, fmt in opmap
> -                      if opts.get(op))
> +                      if opts.get(op) and sep is not None)
> +    datafields = [op for op, sep, get, fmt in opmap
> +                  if opts.get(op) and sep is None]
>  
>      def bad(x, y):
>          raise error.Abort("%s: %s" % (x, y))
> @@ -469,23 +479,37 @@
>                                diffopts=diffopts)
>          formats = []
>          pieces = []
> +        data = []
>  
>          for f, sep in funcmap:
>              l = [f(n) for n, dummy in lines]
>              if l:
>                  if fm:
> +                    if sep is None:
> +                        continue
>                      formats.append(['%s' for x in l])
>                  else:
> +                    if sep is None:
> +                        data.append(l)
> +                        continue
>                      sizes = [encoding.colwidth(x) for x in l]
>                      ml = max(sizes)
>                      formats.append([sep + ' ' * (ml - w) + '%s' for w in sizes])
>                  pieces.append(l)
>  
> -        for f, p, l in zip(zip(*formats), zip(*pieces), lines):
> -            fm.startitem()
> -            fm.write(fields, "".join(f), *p)
> -            fm.write('line', ": %s", l[1])
> -
> +        if not data:
> +            for f, p, l in zip(zip(*formats), zip(*pieces), lines):
> +                fm.startitem()
> +                fm.write(fields, "".join(f), *p)
> +                fm.write('line', ": %s", l[1])
> +        else:
> +            for f, p, d, l in zip(zip(*formats), zip(*pieces), zip(*data),
> +                                  lines):
> +                fm.startitem()
> +                fm.write(fields, "".join(f), *p)
> +                fm.write('line', ": %s", l[1])
> +                d = dict(zip(datafields, d))
> +                fm.data(**d)

Instead of abusing "sep", I would do:

  abspathindex = ...  # index in fields
  ...
  for f, p, l:
      ...
      if abspathindex >= 0:
          fm.data(file=p[abspathindex])
timeless - April 15, 2016, 5:02 p.m.
Yuya Nishihara wrote:
>
> Instead of abusing "sep", I would do:
>
>   abspathindex = ...  # index in fields
>   ...
>   for f, p, l:
>       ...
>       if abspathindex >= 0:
>           fm.data(file=p[abspathindex])

I couldn't come up w/ a good approach to this, I'm happy to take yours.
I also wanted it to be somewhat scalable in case it happened again,
but, I think I should let someone else cross that bridge when they
come to it.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -422,18 +422,26 @@ 
         hexfn = fm.hexfunc
         formatrev = formathex = str
 
+    _abspath = lambda x: x[0].path()
     opmap = [('user', ' ', lambda x: x[0].user(), ui.shortuser),
              ('number', ' ', lambda x: x[0].rev(), formatrev),
              ('changeset', ' ', lambda x: hexfn(x[0].node()), formathex),
              ('date', ' ', lambda x: x[0].date(), util.cachefunc(datefunc)),
-             ('file', ' ', lambda x: x[0].path(), str),
+             ('abspath', ' ', _abspath, str),
+             ('file', None, _abspath, str),
              ('line_number', ':', lambda x: x[1], str),
             ]
-    fieldnamemap = {'number': 'rev', 'changeset': 'node'}
+    fieldnamemap = {
+             'changeset': 'node',
+             'file': 'abspath',
+             'number': 'rev',
+            }
 
     if (not opts.get('user') and not opts.get('changeset')
         and not opts.get('date') and not opts.get('file')):
         opts['number'] = True
+    if opts.get('file'):
+        opts['abspath'] = True
 
     linenumber = opts.get('line_number') is not None
     if linenumber and (not opts.get('changeset')) and (not opts.get('number')):
@@ -449,7 +457,9 @@ 
                if opts.get(op)]
     funcmap[0] = (funcmap[0][0], '') # no separator in front of first column
     fields = ' '.join(fieldnamemap.get(op, op) for op, sep, get, fmt in opmap
-                      if opts.get(op))
+                      if opts.get(op) and sep is not None)
+    datafields = [op for op, sep, get, fmt in opmap
+                  if opts.get(op) and sep is None]
 
     def bad(x, y):
         raise error.Abort("%s: %s" % (x, y))
@@ -469,23 +479,37 @@ 
                               diffopts=diffopts)
         formats = []
         pieces = []
+        data = []
 
         for f, sep in funcmap:
             l = [f(n) for n, dummy in lines]
             if l:
                 if fm:
+                    if sep is None:
+                        continue
                     formats.append(['%s' for x in l])
                 else:
+                    if sep is None:
+                        data.append(l)
+                        continue
                     sizes = [encoding.colwidth(x) for x in l]
                     ml = max(sizes)
                     formats.append([sep + ' ' * (ml - w) + '%s' for w in sizes])
                 pieces.append(l)
 
-        for f, p, l in zip(zip(*formats), zip(*pieces), lines):
-            fm.startitem()
-            fm.write(fields, "".join(f), *p)
-            fm.write('line', ": %s", l[1])
-
+        if not data:
+            for f, p, l in zip(zip(*formats), zip(*pieces), lines):
+                fm.startitem()
+                fm.write(fields, "".join(f), *p)
+                fm.write('line', ": %s", l[1])
+        else:
+            for f, p, d, l in zip(zip(*formats), zip(*pieces), zip(*data),
+                                  lines):
+                fm.startitem()
+                fm.write(fields, "".join(f), *p)
+                fm.write('line', ": %s", l[1])
+                d = dict(zip(datafields, d))
+                fm.data(**d)
         if lines and not lines[-1][1].endswith('\n'):
             fm.plain('\n')
 
diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
--- a/mercurial/templatekw.py
+++ b/mercurial/templatekw.py
@@ -49,11 +49,18 @@ 
             raise AttributeError(name)
         return getattr(self.values, name)
 
-def showlist(name, values, plural=None, element=None, separator=' ', **args):
-    if not element:
-        element = name
+def showlist(name, values, plural=None, element=None, elements=None,
+             separator=' ', **args):
+    if not elements:
+        if not element:
+            elements = [name]
+        else:
+            elements = [element]
     f = _showlist(name, values, plural, separator, **args)
-    return _hybrid(f, values, lambda x: {element: x})
+    def makemap(x):
+        v = dict((k, x) for k in elements)
+        return v
+    return _hybrid(f, values, makemap)
 
 def _showlist(name, values, plural=None, separator=' ', **args):
     '''expand set of values.
@@ -308,12 +315,14 @@ 
     return _hybrid(f, extras, makemap,
                    lambda x: '%s=%s' % (x['key'], x['value']))
 
+# backwardscompat
+_abspath = ['abspath', 'file']
 @templatekeyword('file_adds')
 def showfileadds(**args):
     """List of strings. Files added by this changeset."""
     repo, ctx, revcache = args['repo'], args['ctx'], args['revcache']
     return showlist('file_add', getfiles(repo, ctx, revcache)[1],
-                    element='file', **args)
+                    elements=_abspath, **args)
 
 @templatekeyword('file_copies')
 def showfilecopies(**args):
@@ -360,21 +369,21 @@ 
     """List of strings. Files removed by this changeset."""
     repo, ctx, revcache = args['repo'], args['ctx'], args['revcache']
     return showlist('file_del', getfiles(repo, ctx, revcache)[2],
-                    element='file', **args)
+                    elements=_abspath, **args)
 
 @templatekeyword('file_mods')
 def showfilemods(**args):
     """List of strings. Files modified by this changeset."""
     repo, ctx, revcache = args['repo'], args['ctx'], args['revcache']
     return showlist('file_mod', getfiles(repo, ctx, revcache)[0],
-                    element='file', **args)
+                    elements=_abspath, **args)
 
 @templatekeyword('files')
 def showfiles(**args):
     """List of strings. All files modified, added, or removed by this
     changeset.
     """
-    return showlist('file', args['ctx'].files(), **args)
+    return showlist('file', args['ctx'].files(), elements=_abspath, **args)
 
 @templatekeyword('graphnode')
 def showgraphnode(repo, ctx, **args):
diff --git a/mercurial/templates/map-cmdline.status b/mercurial/templates/map-cmdline.status
--- a/mercurial/templates/map-cmdline.status
+++ b/mercurial/templates/map-cmdline.status
@@ -11,15 +11,15 @@ 
                     'files:\n'))}{lfile_mods}{lfile_adds}{lfile_copies_switch}{lfile_dels}'
 
 # Exclude copied files, will display those in lfile_copies_switch
-lfile_adds  = '{file_adds % "{ifcontains(file, file_copies_switch,
+lfile_adds  = '{file_adds % "{ifcontains(abspath, file_copies_switch,
                                          '',
                                          '{lfile_add}')}"}'
-lfile_add = '{label("status.added", "A {file}\n")}'
+lfile_add = '{label("status.added", "A {abspath}\n")}'
 
 lfile_copies_switch = '{file_copies_switch % "{lfile_copy_orig}{lfile_copy_dest}"}'
 lfile_copy_orig = '{label("status.added", "A {name}\n")}'
 lfile_copy_dest = '{label("status.copied", "  {source}\n")}'
 
-lfile_mods = '{file_mods % "{label('status.modified', 'M {file}\n')}"}'
+lfile_mods = '{file_mods % "{label('status.modified', 'M {abspath}\n')}"}'
 
-lfile_dels = '{file_dels % "{label('status.removed', 'R {file}\n')}"}'
+lfile_dels = '{file_dels % "{label('status.removed', 'R {abspath}\n')}"}'
diff --git a/tests/test-annotate.t b/tests/test-annotate.t
--- a/tests/test-annotate.t
+++ b/tests/test-annotate.t
@@ -64,8 +64,8 @@ 
   $ hg annotate -Tjson -cdfnul a
   [
    {
+    "abspath": "a",
     "date": [1.0, 0],
-    "file": "a",
     "line": "a\n",
     "line_number": 1,
     "node": "8435f90966e442695d2ded29fdade2bac5ad8065",
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -2789,7 +2789,7 @@ 
 
 Test new-style inline templating:
 
-  $ hg log -R latesttag -r tip --template 'modified files: {file_mods % " {file}\n"}\n'
+  $ hg log -R latesttag -r tip --template 'modified files: {file_mods % " {abspath}\n"}\n'
   modified files:  .hgtags
   
 
@@ -3005,7 +3005,7 @@ 
 stripped before parsing:
 
   $ cat <<'EOF' > escquotetmpl
-  > changeset = "\" \\" \\\" \\\\" {files % \"{file}\"}\n"
+  > changeset = "\" \\" \\\" \\\\" {files % \"{abspath}\"}\n"
   > EOF
   $ cd latesttag
   $ hg log -r 2 --style ../escquotetmpl
@@ -3061,12 +3061,12 @@ 
 Test leading backslashes:
 
   $ cd latesttag
-  $ hg log -r 2 -T '\{rev} {files % "\{file}"}\n'
-  {rev} {file}
-  $ hg log -r 2 -T '\\{rev} {files % "\\{file}"}\n'
+  $ hg log -r 2 -T '\{rev} {files % "\{abspath}"}\n'
+  {rev} {abspath}
+  $ hg log -r 2 -T '\\{rev} {files % "\\{abspath}"}\n'
   \2 \head1
-  $ hg log -r 2 -T '\\\{rev} {files % "\\\{file}"}\n'
-  \{rev} \{file}
+  $ hg log -r 2 -T '\\\{rev} {files % "\\\{abspath}"}\n'
+  \{rev} \{abspath}
   $ cd ..
 
 Test leading backslashes in "if" expression (issue4714):
@@ -3143,7 +3143,7 @@ 
   $ hg log -R a -r 2 --template '{sub("n", r"\x2d", r"no perso\x6e")}\n'
   \x2do perso\x6e
 
-  $ hg log -R a -r 8 --template '{files % "{file}\n"}'
+  $ hg log -R a -r 8 --template '{files % "{abspath}\n"}'
   fourth
   second
   third
diff --git a/tests/test-convert-filemap.t b/tests/test-convert-filemap.t
--- a/tests/test-convert-filemap.t
+++ b/tests/test-convert-filemap.t
@@ -724,7 +724,7 @@ 
   converted/a
   converted/b
   x
-  $ hg -R merge-test2 log -G -T '{shortest(node)} {desc}\n{files % "- {file}\n"}\n'
+  $ hg -R merge-test2 log -G -T '{shortest(node)} {desc}\n{files % "- {abspath}\n"}\n'
   o    6eaa merge a & b
   |\   - converted/a
   | |  - toberemoved
@@ -770,7 +770,7 @@ 
   sorting...
   converting...
   0 3
-  $ hg -R .-hg log -G -T '{shortest(node)} {desc}\n{files % "- {file}\n"}\n'
+  $ hg -R .-hg log -G -T '{shortest(node)} {desc}\n{files % "- {abspath}\n"}\n'
   o    e9ed 3
   |\
   | o  33a0 2
diff --git a/tests/test-mq-qrefresh-replace-log-message.t b/tests/test-mq-qrefresh-replace-log-message.t
--- a/tests/test-mq-qrefresh-replace-log-message.t
+++ b/tests/test-mq-qrefresh-replace-log-message.t
@@ -40,9 +40,9 @@ 
   $ cat >> .hg/hgrc <<EOF
   > [committemplate]
   > listupfiles = {file_adds %
-  >    "HG: added {file}\n"     }{file_mods %
-  >    "HG: changed {file}\n"   }{file_dels %
-  >    "HG: removed {file}\n"   }{if(files, "",
+  >    "HG: added {abspath}\n"  }{file_mods %
+  >    "HG: changed {abspath}\n"}{file_dels %
+  >    "HG: removed {abspath}\n"}{if(files, "",
   >    "HG: no files changed\n")}
   > 
   > changeset = HG: this is customized commit template