Patchwork [6,of,6] cmdutil: expand filename format string by templater (BC)

login
register
mail settings
Submitter Yuya Nishihara
Date Feb. 15, 2018, 1:18 p.m.
Message ID <437212580debfa26ee26.1518700724@mimosa>
Download mbox | patch
Permalink /patch/27973/
State Accepted
Headers show

Comments

Yuya Nishihara - Feb. 15, 2018, 1:18 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1515293587 -32400
#      Sun Jan 07 11:53:07 2018 +0900
# Node ID 437212580debfa26ee266598f64139b4df5a9cfd
# Parent  e7f4a2b09c484d4313aa6624470d92aef3ad5a10
cmdutil: expand filename format string by templater (BC)

This is BC because '{}' could be a valid filename before, but I believe good
programmers wouldn't use such catastrophic output filenames.

The output of "%m" is slightly changed because "{desc}" is stripped from both
sides. I think the old behavior was a bug.

This patch also adds cmdutil.rendertemplate(ctx, tmpl, props) as a simpler
way of expanding template against single changeset.

.. bc::

   '{' in output filename passed to archive/cat/export is taken as a start
   of a template expression.
Augie Fackler - Feb. 16, 2018, 10:13 p.m.
On Thu, Feb 15, 2018 at 10:18:44PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1515293587 -32400
> #      Sun Jan 07 11:53:07 2018 +0900
> # Node ID 437212580debfa26ee266598f64139b4df5a9cfd
> # Parent  e7f4a2b09c484d4313aa6624470d92aef3ad5a10
> cmdutil: expand filename format string by templater (BC)

queued, thanks
Matt Harbison - Feb. 17, 2018, 6:13 p.m.
On Thu, 15 Feb 2018 08:18:44 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1515293587 -32400
> #      Sun Jan 07 11:53:07 2018 +0900
> # Node ID 437212580debfa26ee266598f64139b4df5a9cfd
> # Parent  e7f4a2b09c484d4313aa6624470d92aef3ad5a10
> cmdutil: expand filename format string by templater (BC)

This causes the filename to be interpreted for escape sequences, so  
"C:\temp" becomes "C:<TAB>emp".  Surprisingly, only test-graft.t caught  
it.  Similarly, "..\appdata\.." becomes "..\x07ppdata\\..".

https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/473/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio
Yuya Nishihara - Feb. 18, 2018, 1:32 a.m.
On Sat, 17 Feb 2018 13:13:57 -0500, Matt Harbison wrote:
> On Thu, 15 Feb 2018 08:18:44 -0500, Yuya Nishihara <yuya@tcha.org> wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1515293587 -32400
> > #      Sun Jan 07 11:53:07 2018 +0900
> > # Node ID 437212580debfa26ee266598f64139b4df5a9cfd
> > # Parent  e7f4a2b09c484d4313aa6624470d92aef3ad5a10
> > cmdutil: expand filename format string by templater (BC)
> 
> This causes the filename to be interpreted for escape sequences, so  
> "C:\temp" becomes "C:<TAB>emp".  Surprisingly, only test-graft.t caught  
> it.  Similarly, "..\appdata\.." becomes "..\x07ppdata\\..".

Good catch. Dropped the last patch from hg-committed, thanks.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -42,6 +42,7 @@  from . import (
     scmutil,
     smartset,
     subrepoutil,
+    templatekw,
     templater,
     util,
     vfs as vfsmod,
@@ -891,46 +892,82 @@  def getcommiteditor(edit=False, finishde
     else:
         return commiteditor
 
-def makefilename(ctx, pat,
-                  total=None, seqno=None, revwidth=None, pathname=None):
+def rendertemplate(ctx, tmpl, props=None):
+    """Expand a literal template 'tmpl' byte-string against one changeset
+
+    Each props item must be a stringify-able value or a callable returning
+    such value, i.e. no bare list nor dict should be passed.
+    """
+    repo = ctx.repo()
+    tres = formatter.templateresources(repo.ui, repo)
+    t = formatter.maketemplater(repo.ui, tmpl, defaults=templatekw.keywords,
+                                resources=tres)
+    mapping = {'ctx': ctx, 'revcache': {}}
+    if props:
+        mapping.update(props)
+    return t.render(mapping)
+
+def _buildfntemplate(pat, total=None, seqno=None, revwidth=None, pathname=None):
+    """Convert old-style filename format string to template string
+
+    >>> _buildfntemplate(b'foo-%b-%n.patch', seqno=0)
+    'foo-{reporoot|basename}-{seqno}.patch'
+    >>> _buildfntemplate(b'%R{tags % "{tag}"}%H')
+    '{rev}{tags % "{tag}"}{node}'
+    """
     expander = {
-        'H': lambda: ctx.hex(),
-        'R': lambda: '%d' % ctx.rev(),
-        'h': lambda: short(ctx.node()),
-        'm': lambda: re.sub('[^\w]', '_',
-                            ctx.description().rstrip().splitlines()[0]),
-        'r': lambda: ('%d' % ctx.rev()).zfill(revwidth or 0),
-        '%': lambda: '%',
-        'b': lambda: os.path.basename(ctx.repo().root),
+        'H': '{node}',
+        'R': '{rev}',
+        'h': '{node|short}',
+        'm': br'{sub(r"[^\w]", "_", desc|firstline)}',
+        'r': '{if(revwidth, pad(rev, revwidth, "0", left=True), rev)}',
+        '%': '%',
+        'b': '{reporoot|basename}',
         }
     if total is not None:
-        expander['N'] = lambda: '%d' % total
+        expander['N'] = '{total}'
     if seqno is not None:
-        expander['n'] = lambda: '%d' % seqno
+        expander['n'] = '{seqno}'
     if total is not None and seqno is not None:
-        expander['n'] = (lambda: ('%d' % seqno).zfill(len('%d' % total)))
+        expander['n'] = '{pad(seqno, total|stringify|count, "0", left=True)}'
     if pathname is not None:
-        expander['s'] = lambda: os.path.basename(pathname)
-        expander['d'] = lambda: os.path.dirname(pathname) or '.'
-        expander['p'] = lambda: pathname
+        expander['s'] = '{pathname|basename}'
+        expander['d'] = '{if(pathname|dirname, pathname|dirname, ".")}'
+        expander['p'] = '{pathname}'
 
     newname = []
-    patlen = len(pat)
-    i = 0
-    while i < patlen:
-        c = pat[i:i + 1]
-        if c == '%':
-            i += 1
-            c = pat[i:i + 1]
+    for typ, start, end in templater.scantemplate(pat):
+        if typ != 'string':
+            newname.append(pat[start:end])
+            continue
+        i = start
+        while i < end:
+            n = pat.find('%', i, end)
+            if n < 0:
+                newname.append(pat[i:end])
+                break
+            newname.append(pat[i:n])
+            if n + 2 > end:
+                raise error.Abort(_("incomplete format spec in output "
+                                    "filename"))
+            c = pat[n + 1:n + 2]
+            i = n + 2
             try:
-                c = expander[c]()
+                newname.append(expander[c])
             except KeyError:
                 raise error.Abort(_("invalid format spec '%%%s' in output "
                                     "filename") % c)
-        newname.append(c)
-        i += 1
     return ''.join(newname)
 
+def makefilename(ctx, pat, **props):
+    if not pat:
+        return pat
+    tmpl = _buildfntemplate(pat, **props)
+    # BUG: alias expansion shouldn't be made against template fragments
+    # rewritten from %-format strings, but we have no easy way to partially
+    # disable the expansion.
+    return rendertemplate(ctx, tmpl, pycompat.byteskwargs(props))
+
 def isstdiofilename(pat):
     """True if the given pat looks like a filename denoting stdin/stdout"""
     return not pat or pat == '-'
@@ -954,10 +991,7 @@  class _unclosablefile(object):
     def __exit__(self, exc_type, exc_value, exc_tb):
         pass
 
-def makefileobj(ctx, pat, total=None,
-                seqno=None, revwidth=None, mode='wb', modemap=None,
-                pathname=None):
-
+def makefileobj(ctx, pat, mode='wb', modemap=None, **props):
     writable = mode not in ('r', 'rb')
 
     if isstdiofilename(pat):
@@ -967,7 +1001,7 @@  def makefileobj(ctx, pat, total=None,
         else:
             fp = repo.ui.fin
         return _unclosablefile(fp)
-    fn = makefilename(ctx, pat, total, seqno, revwidth, pathname)
+    fn = makefilename(ctx, pat, **props)
     if modemap is not None:
         mode = modemap.get(fn, mode)
         if mode == 'wb':
@@ -1542,9 +1576,8 @@  def export(repo, revs, fntemplate='hg-%h
         ctx = repo[rev]
         fo = None
         if not fp and fntemplate:
-            fo = makefileobj(ctx, fntemplate,
-                             total=total, seqno=seqno, revwidth=revwidth,
-                             mode='wb', modemap=filemode)
+            fo = makefileobj(ctx, fntemplate, mode='wb', modemap=filemode,
+                             total=total, seqno=seqno, revwidth=revwidth)
             dest = fo.name
             def write(s, **kw):
                 fo.write(s)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1282,7 +1282,9 @@  def cat(ui, repo, file1, *pats, **opts):
     no revision is given, the parent of the working directory is used.
 
     Output may be to a file, in which case the name of the file is
-    given using a format string. The formatting rules as follows:
+    given using a template string. See :hg:`help templates`. In addition
+    to the common template keywords, the following formatting rules are
+    supported:
 
     :``%%``: literal "%" character
     :``%s``: basename of file being printed
@@ -1902,7 +1904,9 @@  def export(ui, repo, *changesets, **opts
        first parent only.
 
     Output may be to a file, in which case the name of the file is
-    given using a format string. The formatting rules are as follows:
+    given using a template string. See :hg:`help templates`. In addition
+    to the common template keywords, the following formatting rules are
+    supported:
 
     :``%%``: literal "%" character
     :``%H``: changeset hash (40 hexadecimal digits)
diff --git a/tests/test-doctest.py b/tests/test-doctest.py
--- a/tests/test-doctest.py
+++ b/tests/test-doctest.py
@@ -42,6 +42,7 @@  def testmod(name, optionflags=0, testtar
 
 testmod('mercurial.changegroup')
 testmod('mercurial.changelog')
+testmod('mercurial.cmdutil')
 testmod('mercurial.color')
 testmod('mercurial.config')
 testmod('mercurial.context')
diff --git a/tests/test-export.t b/tests/test-export.t
--- a/tests/test-export.t
+++ b/tests/test-export.t
@@ -184,13 +184,28 @@  Checking if only alphanumeric characters
   $ hg commit -m " !\"#$%&(,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]"'^'"_\`abcdefghijklmnopqrstuvwxyz{|}~"
   $ hg export -v -o %m.patch tip
   exporting patch:
-  ____________0123456789_______ABCDEFGHIJKLMNOPQRSTUVWXYZ______abcdefghijklmnopqrstuvwxyz____.patch
+  ___________0123456789_______ABCDEFGHIJKLMNOPQRSTUVWXYZ______abcdefghijklmnopqrstuvwxyz____.patch
+
+Template fragments in file name:
+
+  $ hg export -v -o '{node|shortest}.patch' tip
+  exporting patch:
+  197e.patch
 
 Invalid pattern in file name:
 
   $ hg export -o '%x.patch' tip
   abort: invalid format spec '%x' in output filename
   [255]
+  $ hg export -o '%' tip
+  abort: incomplete format spec in output filename
+  [255]
+  $ hg export -o '%{"foo"}' tip
+  abort: incomplete format spec in output filename
+  [255]
+  $ hg export -o '%m{' tip
+  hg: parse error at 3: unterminated template expansion
+  [255]
 
 Catch exporting unknown revisions (especially empty revsets, see issue3353)