Patchwork [2,of,2] template: add pad function for padding output

login
register
mail settings
Submitter Durham Goode
Date Jan. 17, 2014, 8:45 a.m.
Message ID <77cf62dd2af0b5631a28.1389948330@dev010.prn1.facebook.com>
Download mbox | patch
Permalink /patch/3364/
State Superseded
Commit aa51392da50763f3e7b9ec40acc3a97682488b66
Headers show

Comments

Durham Goode - Jan. 17, 2014, 8:45 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1389946608 28800
#      Fri Jan 17 00:16:48 2014 -0800
# Node ID 77cf62dd2af0b5631a2864744e720e61ad4cde6a
# Parent  b18359a70b640d2aeb3f4afd1c8e47d775402b8e
template: add pad function for padding output

Adds a pad template function with the following signature:

pad(text, width, right=False, fillchar=' ')

This uses the standard python ljust and rjust functions to produce a string
that is at least a certain width. This is especially useful with the
introduction of a '{shortestnode}' keyword, since the output can be variable
length.
Mads Kiilerich - Jan. 17, 2014, 1:41 p.m.
On 01/17/2014 09:45 AM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1389946608 28800
> #      Fri Jan 17 00:16:48 2014 -0800
> # Node ID 77cf62dd2af0b5631a2864744e720e61ad4cde6a
> # Parent  b18359a70b640d2aeb3f4afd1c8e47d775402b8e
> template: add pad function for padding output
>
> Adds a pad template function with the following signature:
>
> pad(text, width, right=False, fillchar=' ')

Nice. I have been missing some printf-ish functionality too.

I kind of expected the 'fill' function to do this (or something very 
similar) ... especially for short strings. Having a separate "fill or 
overflow" function might make sense. Next on the feature creep list 
would be a fill that strips instead of overflowing. ;-)

I thought it was possible to have filters with parameters. That would 
have seemed like a more obvious way of doing it to me. But apparently 
not. I wonder if it would be feasible to fix that ... but I assume that 
when/if that is fixed then it will be possible to use this as a filter.

> This uses the standard python ljust and rjust functions to produce a string
> that is at least a certain width. This is especially useful with the
> introduction of a '{shortestnode}' keyword, since the output can be variable
> length.

True, but I know how to get hashes with a fixed width. It is much harder 
with usernames and branch names and commit messages ;-)

> diff --git a/mercurial/templater.py b/mercurial/templater.py
> --- a/mercurial/templater.py
> +++ b/mercurial/templater.py
> @@ -245,6 +245,31 @@
>   
>       return templatefilters.fill(text, width, initindent, hangindent)
>   
> +def pad(context, mapping, args):
> +    """usage: pad(text, width, right=False, fillchar=' ')
> +    """
> +    if not (2 <= len(args) <= 4):
> +        raise error.ParseError(_("pad() expects two to four arguments"))
> +
> +    width = int(stringify(args[1][0](context, mapping, args[1][1])))
> +
> +    text = stringify(args[0][0](context, mapping, args[0][1]))
> +    if args[0][0] == runstring:
> +        text = stringify(runtemplate(context, mapping,
> +            compiletemplate(text, context)))
> +
> +    right = False
> +    fillchar = ' '
> +    if len(args) > 2:
> +        right = stringify(args[2][0](context, mapping, args[2][1])) == "True"

Expecting a literal "True" would surprise me as a user. Most other 
boolean inputs use util.parsebool.

But I think I would suggest having a separate 'rpad' "filter" instead.

/Mads
Augie Fackler - Jan. 17, 2014, 2:20 p.m.
On Fri, Jan 17, 2014 at 02:41:36PM +0100, Mads Kiilerich wrote:
> On 01/17/2014 09:45 AM, Durham Goode wrote:
> ># HG changeset patch
> ># User Durham Goode <durham@fb.com>
> ># Date 1389946608 28800
> >#      Fri Jan 17 00:16:48 2014 -0800
> ># Node ID 77cf62dd2af0b5631a2864744e720e61ad4cde6a
> ># Parent  b18359a70b640d2aeb3f4afd1c8e47d775402b8e
> >template: add pad function for padding output
> >
> >Adds a pad template function with the following signature:
> >
> >pad(text, width, right=False, fillchar=' ')
>
> Nice. I have been missing some printf-ish functionality too.
>
> I kind of expected the 'fill' function to do this (or something very
> similar) ... especially for short strings. Having a separate "fill or
> overflow" function might make sense. Next on the feature creep list would be
> a fill that strips instead of overflowing. ;-)
>
> I thought it was possible to have filters with parameters. That would have
> seemed like a more obvious way of doing it to me. But apparently not. I
> wonder if it would be feasible to fix that ... but I assume that when/if
> that is fixed then it will be possible to use this as a filter.
>
> >This uses the standard python ljust and rjust functions to produce a string
> >that is at least a certain width. This is especially useful with the
> >introduction of a '{shortestnode}' keyword, since the output can be variable
> >length.
>
> True, but I know how to get hashes with a fixed width. It is much harder
> with usernames and branch names and commit messages ;-)
>
> >diff --git a/mercurial/templater.py b/mercurial/templater.py
> >--- a/mercurial/templater.py
> >+++ b/mercurial/templater.py
> >@@ -245,6 +245,31 @@
> >      return templatefilters.fill(text, width, initindent, hangindent)
> >+def pad(context, mapping, args):
> >+    """usage: pad(text, width, right=False, fillchar=' ')
> >+    """
> >+    if not (2 <= len(args) <= 4):
> >+        raise error.ParseError(_("pad() expects two to four arguments"))
> >+
> >+    width = int(stringify(args[1][0](context, mapping, args[1][1])))
> >+
> >+    text = stringify(args[0][0](context, mapping, args[0][1]))
> >+    if args[0][0] == runstring:
> >+        text = stringify(runtemplate(context, mapping,
> >+            compiletemplate(text, context)))
> >+
> >+    right = False
> >+    fillchar = ' '
> >+    if len(args) > 2:
> >+        right = stringify(args[2][0](context, mapping, args[2][1])) == "True"
>
> Expecting a literal "True" would surprise me as a user. Most other boolean
> inputs use util.parsebool.

+1 on util.parsebool for consistency

>
> But I think I would suggest having a separate 'rpad' "filter" instead.
>
> /Mads
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Durham Goode - Jan. 17, 2014, 6:44 p.m.
On 1/17/14 5:41 AM, "Mads Kiilerich" <mads@kiilerich.com> wrote:

>On 01/17/2014 09:45 AM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1389946608 28800
>> #      Fri Jan 17 00:16:48 2014 -0800
>> # Node ID 77cf62dd2af0b5631a2864744e720e61ad4cde6a
>> # Parent  b18359a70b640d2aeb3f4afd1c8e47d775402b8e
>> template: add pad function for padding output
>>
>> Adds a pad template function with the following signature:
>>
>> pad(text, width, right=False, fillchar=' ')
>
>> +    if len(args) > 2:
>> +        right = stringify(args[2][0](context, mapping, args[2][1])) ==
>>"True"
>
>Expecting a literal "True" would surprise me as a user. Most other
>boolean inputs use util.parsebool.

Will fix

>
>But I think I would suggest having a separate 'rpad' "filter" instead.
>

The problem with rpad is that it's ambiguous (Does the padding go on the
right? Does the text go on the right?). I'd have to use rjustify and
ljustify, like python does, but when I am looking for functions like this
I search for the word 'pad'.  I think right justified output will be rare
enough that it's ok regulating it to an optional parameter.
Matt Mackall - Jan. 18, 2014, 3:42 a.m.
On Fri, 2014-01-17 at 00:45 -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1389946608 28800
> #      Fri Jan 17 00:16:48 2014 -0800
> # Node ID 77cf62dd2af0b5631a2864744e720e61ad4cde6a
> # Parent  b18359a70b640d2aeb3f4afd1c8e47d775402b8e
> template: add pad function for padding output
> 
> Adds a pad template function with the following signature:
> 
> pad(text, width, right=False, fillchar=' ')

I like it, but I'm not pleased with the way the third argument works.
Might want to make it the last arg.

I'm tempted to say setting a negative width to get right-aligned would
be better, if still kludgey.

> +  $ hg log --template '{pad("{shortestnode}", "20")} {author|user}\n'

Unfortunate dependence on patch 1. Please test that 

+  $ hg log --template '{pad(rev, "20")} {author|user}\n'

..or something else without recursive templating does the right thing.

Let's revisit this after Feb 1.

Patch

diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -245,6 +245,31 @@ 
 
     return templatefilters.fill(text, width, initindent, hangindent)
 
+def pad(context, mapping, args):
+    """usage: pad(text, width, right=False, fillchar=' ')
+    """
+    if not (2 <= len(args) <= 4):
+        raise error.ParseError(_("pad() expects two to four arguments"))
+
+    width = int(stringify(args[1][0](context, mapping, args[1][1])))
+
+    text = stringify(args[0][0](context, mapping, args[0][1]))
+    if args[0][0] == runstring:
+        text = stringify(runtemplate(context, mapping,
+            compiletemplate(text, context)))
+
+    right = False
+    fillchar = ' '
+    if len(args) > 2:
+        right = stringify(args[2][0](context, mapping, args[2][1])) == "True"
+    if len(args) > 3:
+        fillchar = stringify(args[3][0](context, mapping, args[3][1]))
+
+    if right:
+        return text.rjust(width, fillchar)
+    else:
+        return text.ljust(width, fillchar)
+
 def get(context, mapping, args):
     if len(args) != 2:
         # i18n: "get" is a keyword
@@ -368,6 +393,7 @@ 
     "ifeq": ifeq,
     "join": join,
     "label": label,
+    "pad": pad,
     "rstdoc": rstdoc,
     "strip": strip,
     "sub": sub,
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
@@ -1635,3 +1635,16 @@ 
   d97c
   f776
 
+Test pad function
+
+  $ hg log --template '{pad("{shortestnode}", "20")} {author|user}\n'
+  d97c                 test
+  f776                 test
+
+  $ hg log --template '{pad("{shortestnode}", "20", "True")} {author|user}\n'
+                  d97c test
+                  f776 test
+
+  $ hg log --template '{pad("{shortestnode}", "20", "False", "-")} {author|user}\n'
+  d97c---------------- test
+  f776---------------- test