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
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
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
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.
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