Patchwork [2,of,2] templater: relax unquotestring() to fall back to bare string

login
register
mail settings
Submitter Yuya Nishihara
Date March 26, 2016, 4:07 p.m.
Message ID <4124288600c9eb8c229b.1459008452@mimosa>
Download mbox | patch
Permalink /patch/14084/
State Accepted
Commit bf35644b9f3a4608d18b7d7655d8831902235d93
Headers show

Comments

Yuya Nishihara - March 26, 2016, 4:07 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1458983532 -32400
#      Sat Mar 26 18:12:12 2016 +0900
# Node ID 4124288600c9eb8c229bcf98eb6902caf7fbbcad
# Parent  1c2157434cd147c37a08494ef54d4aa3d5999728
templater: relax unquotestring() to fall back to bare string

This is convenient for our use case where quotes are optional except in
a map file.
Sean Farley - March 27, 2016, 1:41 a.m.
Yuya Nishihara <yuya@tcha.org> writes:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1458983532 -32400
> #      Sat Mar 26 18:12:12 2016 +0900
> # Node ID 4124288600c9eb8c229bcf98eb6902caf7fbbcad
> # Parent  1c2157434cd147c37a08494ef54d4aa3d5999728
> templater: relax unquotestring() to fall back to bare string
>
> This is convenient for our use case where quotes are optional except in
> a map file.

Nice work with this series. I like them.
Pierre-Yves David - March 27, 2016, 4:51 a.m.
On 03/26/2016 09:07 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1458983532 -32400
> #      Sat Mar 26 18:12:12 2016 +0900
> # Node ID 4124288600c9eb8c229bcf98eb6902caf7fbbcad
> # Parent  1c2157434cd147c37a08494ef54d4aa3d5999728
> templater: relax unquotestring() to fall back to bare string
>
> This is convenient for our use case where quotes are optional except in
> a map file.

I've pushed patch 1 (which is a great clean up),

However, I'm not clear about what is the effect of patch 2. Can you give 
an example of what was previously invalid and now is ?
Yuya Nishihara - March 27, 2016, 8:52 a.m.
On Sat, 26 Mar 2016 21:51:08 -0700, Pierre-Yves David wrote:
> On 03/26/2016 09:07 AM, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1458983532 -32400
> > #      Sat Mar 26 18:12:12 2016 +0900
> > # Node ID 4124288600c9eb8c229bcf98eb6902caf7fbbcad
> > # Parent  1c2157434cd147c37a08494ef54d4aa3d5999728
> > templater: relax unquotestring() to fall back to bare string
> >
> > This is convenient for our use case where quotes are optional except in
> > a map file.
> 
> I've pushed patch 1 (which is a great clean up),
> 
> However, I'm not clear about what is the effect of patch 2. Can you give 
> an example of what was previously invalid and now is ?

There were nothing wrong (except for the SyntaxError type and a few nits).
Previously we had to write try-catch:

  try:
      s = templater.unquotestring(t)
  except SyntaxError:
      s = t

Since it appears to be common for templater users, this patch makes
unquotestring() do that by default.

I don't know why unpaired quote is allowed, but it is for years. If I could
change the history, I would make it an error.

  [ui]
  logtemplate = "somehow unpaired quote works

BTW, I found any characters are taken as quotes if surrounded by the same
character. It is obviously a bug and should be fixed later.

  [ui]
  logtemplate = nthis is quoted by "n"\n

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1542,11 +1542,7 @@  def gettemplate(ui, tmpl, style):
     if not tmpl and not style: # template are stronger than style
         tmpl = ui.config('ui', 'logtemplate')
         if tmpl:
-            try:
-                tmpl = templater.unquotestring(tmpl)
-            except SyntaxError:
-                pass
-            return tmpl, None
+            return templater.unquotestring(tmpl), None
         else:
             style = util.expandpath(ui.config('ui', 'style', ''))
 
diff --git a/mercurial/formatter.py b/mercurial/formatter.py
--- a/mercurial/formatter.py
+++ b/mercurial/formatter.py
@@ -171,11 +171,7 @@  def lookuptemplate(ui, topic, tmpl):
     # perhaps it's a reference to [templates]
     t = ui.config('templates', tmpl)
     if t:
-        try:
-            tmpl = templater.unquotestring(t)
-        except SyntaxError:
-            tmpl = t
-        return tmpl, None
+        return templater.unquotestring(t), None
 
     if tmpl == 'list':
         ui.write(_("available styles: %s\n") % templater.stylelist())
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -867,9 +867,9 @@  def _flatten(thing):
                     yield j
 
 def unquotestring(s):
-    '''unwrap quotes'''
+    '''unwrap quotes if any; otherwise returns unmodified string'''
     if len(s) < 2 or s[0] != s[-1]:
-        raise SyntaxError(_('unmatched quotes'))
+        return s
     return s[1:-1]
 
 class engine(object):
@@ -980,10 +980,10 @@  class templater(object):
             if not val:
                 raise error.ParseError(_('missing value'), conf.source('', key))
             if val[0] in "'\"":
-                try:
-                    self.cache[key] = unquotestring(val)
-                except SyntaxError as inst:
-                    raise error.ParseError(inst.args[0], conf.source('', key))
+                if val[0] != val[-1]:
+                    raise error.ParseError(_('unmatched quotes'),
+                                           conf.source('', key))
+                self.cache[key] = unquotestring(val)
             else:
                 val = 'default', val
                 if ':' in val[1]: