Patchwork [5,of,5,V2] templater: port pad() to take keyword arguments

login
register
mail settings
Submitter Yuya Nishihara
Date April 8, 2017, 1:06 p.m.
Message ID <4519938b18da898cb116.1491656800@mimosa>
Download mbox | patch
Permalink /patch/20025/
State Accepted
Headers show

Comments

Yuya Nishihara - April 8, 2017, 1:06 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1491225832 -32400
#      Mon Apr 03 22:23:52 2017 +0900
# Node ID 4519938b18da898cb116a3fea0d1ecee604266c0
# Parent  05be4b2679b557c9fc8711d9b258e3667cec0b88
templater: port pad() to take keyword arguments

This is another example where keyword arguments can be actually useful.
Ryan McElroy - April 10, 2017, 9:48 a.m.
On 4/8/17 2:06 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1491225832 -32400
> #      Mon Apr 03 22:23:52 2017 +0900
> # Node ID 4519938b18da898cb116a3fea0d1ecee604266c0
> # Parent  05be4b2679b557c9fc8711d9b258e3667cec0b88
> templater: port pad() to take keyword arguments

Overall, I really like the direction of this series. Having keyword 
arguments will allow more clear template definitions.

I have only a few nit-picks and questions inline in a couple of the 
patches. I'll mark as "under review" pending a response.

>
> This is another example where keyword arguments can be actually useful.
>
> diff --git a/mercurial/templater.py b/mercurial/templater.py
> --- a/mercurial/templater.py
> +++ b/mercurial/templater.py
> @@ -589,29 +589,30 @@ def formatnode(context, mapping, args):
>           return node
>       return templatefilters.short(node)
>   
> -@templatefunc('pad(text, width[, fillchar=\' \'[, left=False]])')
> +@templatefunc('pad(text, width[, fillchar=\' \'[, left=False]])',
> +              argspec='text width fillchar left')
>   def pad(context, mapping, args):
>       """Pad text with a
>       fill character."""
> -    if not (2 <= len(args) <= 4):
> +    if 'text' not in args or 'width' not in args:
>           # i18n: "pad" is a keyword
>           raise error.ParseError(_("pad() expects two to four arguments"))
>   
> -    width = evalinteger(context, mapping, args[1],
> +    width = evalinteger(context, mapping, args['width'],
>                           # i18n: "pad" is a keyword
>                           _("pad() expects an integer width"))
>   
> -    text = evalstring(context, mapping, args[0])
> +    text = evalstring(context, mapping, args['text'])
>   
>       left = False
>       fillchar = ' '
> -    if len(args) > 2:
> -        fillchar = evalstring(context, mapping, args[2])
> +    if 'fillchar' in args:
> +        fillchar = evalstring(context, mapping, args['fillchar'])
>           if len(color.stripeffects(fillchar)) != 1:
>               # i18n: "pad" is a keyword
>               raise error.ParseError(_("pad() expects a single fill character"))
> -    if len(args) > 3:
> -        left = evalboolean(context, mapping, args[3])
> +    if 'left' in args:
> +        left = evalboolean(context, mapping, args['left'])
>   
>       fillwidth = width - encoding.colwidth(color.stripeffects(text))
>       if fillwidth <= 0:
> 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
> @@ -146,6 +146,9 @@ Keyword arguments:
>     hg: parse error: can't use a key-value pair in this context
>     [255]
>   
> +  $ hg debugtemplate '{pad("foo", width=10, left=true)}\n'
> +         foo
> +

This is super cool!

>   Call function which takes named arguments by filter syntax:
>   
>     $ hg debugtemplate '{" "|separate}'
>
Yuya Nishihara - April 10, 2017, 12:26 p.m.
On Mon, 10 Apr 2017 10:48:16 +0100, Ryan McElroy wrote:
> On 4/8/17 2:06 PM, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1491225832 -32400
> > #      Mon Apr 03 22:23:52 2017 +0900
> > # Node ID 4519938b18da898cb116a3fea0d1ecee604266c0
> > # Parent  05be4b2679b557c9fc8711d9b258e3667cec0b88
> > templater: port pad() to take keyword arguments

> > +  $ hg debugtemplate '{pad("foo", width=10, left=true)}\n'
> > +         foo
> > +
> 
> This is super cool!

Thanks. Future patches will add {dict(key=value)|json} and {dict(key)|json}
as a shorthand for {dict(key=key)|json}.

Patch

diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -589,29 +589,30 @@  def formatnode(context, mapping, args):
         return node
     return templatefilters.short(node)
 
-@templatefunc('pad(text, width[, fillchar=\' \'[, left=False]])')
+@templatefunc('pad(text, width[, fillchar=\' \'[, left=False]])',
+              argspec='text width fillchar left')
 def pad(context, mapping, args):
     """Pad text with a
     fill character."""
-    if not (2 <= len(args) <= 4):
+    if 'text' not in args or 'width' not in args:
         # i18n: "pad" is a keyword
         raise error.ParseError(_("pad() expects two to four arguments"))
 
-    width = evalinteger(context, mapping, args[1],
+    width = evalinteger(context, mapping, args['width'],
                         # i18n: "pad" is a keyword
                         _("pad() expects an integer width"))
 
-    text = evalstring(context, mapping, args[0])
+    text = evalstring(context, mapping, args['text'])
 
     left = False
     fillchar = ' '
-    if len(args) > 2:
-        fillchar = evalstring(context, mapping, args[2])
+    if 'fillchar' in args:
+        fillchar = evalstring(context, mapping, args['fillchar'])
         if len(color.stripeffects(fillchar)) != 1:
             # i18n: "pad" is a keyword
             raise error.ParseError(_("pad() expects a single fill character"))
-    if len(args) > 3:
-        left = evalboolean(context, mapping, args[3])
+    if 'left' in args:
+        left = evalboolean(context, mapping, args['left'])
 
     fillwidth = width - encoding.colwidth(color.stripeffects(text))
     if fillwidth <= 0:
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
@@ -146,6 +146,9 @@  Keyword arguments:
   hg: parse error: can't use a key-value pair in this context
   [255]
 
+  $ hg debugtemplate '{pad("foo", width=10, left=true)}\n'
+         foo
+
 Call function which takes named arguments by filter syntax:
 
   $ hg debugtemplate '{" "|separate}'