Patchwork [RFC] color: rename 'label' template function to 'color'

login
register
mail settings
Submitter Sean Farley
Date April 7, 2014, 2:32 a.m.
Message ID <aa43236b2340fadf49b0.1396837922@laptop.local>
Download mbox | patch
Permalink /patch/4242/
State Changes Requested
Headers show

Comments

Sean Farley - April 7, 2014, 2:32 a.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1396670819 18000
#      Fri Apr 04 23:06:59 2014 -0500
# Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
# Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
color: rename 'label' template function to 'color'

Previously, the name 'label' as a template function was not that
descriptive. We change the name to 'color' now to describe exactly what it
does. Future patches will augment the ability of this function to accept a
color name as an argument.

This rename is justified because 'label' is a no-op in core Mercurial and even
with the color extension enabled, it is still not used when output is
redirected (such as scripts).
Sean Farley - April 7, 2014, 2:35 a.m.
Sean Farley <sean.michael.farley@gmail.com> writes:

> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1396670819 18000
> #      Fri Apr 04 23:06:59 2014 -0500
> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
> color: rename 'label' template function to 'color'
>
> Previously, the name 'label' as a template function was not that
> descriptive. We change the name to 'color' now to describe exactly what it
> does. Future patches will augment the ability of this function to accept a
> color name as an argument.
>
> This rename is justified because 'label' is a no-op in core Mercurial and even
> with the color extension enabled, it is still not used when output is
> redirected (such as scripts).

This is written as a statement for purposes of a patch description but
is really a question asking if this is ok.
Matt Mackall - April 7, 2014, 5:23 p.m.
On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1396670819 18000
> #      Fri Apr 04 23:06:59 2014 -0500
> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
> color: rename 'label' template function to 'color'

Breaks backward compatibility. Go directly to Jail, do not pass Go, do
not collect $200.
Sean Farley - April 7, 2014, 5:27 p.m.
Matt Mackall <mpm@selenic.com> writes:

> On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1396670819 18000
>> #      Fri Apr 04 23:06:59 2014 -0500
>> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
>> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
>> color: rename 'label' template function to 'color'
>
> Breaks backward compatibility. Go directly to Jail, do not pass Go, do
> not collect $200.

Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
for changesets) be accessed through templates?
Durham Goode - April 7, 2014, 6:13 p.m.
On 4/7/14, 10:27 AM, Sean Farley wrote:
> Matt Mackall <mpm@selenic.com> writes:
>
>> On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
>>> # HG changeset patch
>>> # User Sean Farley <sean.michael.farley@gmail.com>
>>> # Date 1396670819 18000
>>> #      Fri Apr 04 23:06:59 2014 -0500
>>> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
>>> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
>>> color: rename 'label' template function to 'color'
>> Breaks backward compatibility. Go directly to Jail, do not pass Go, do
>> not collect $200.
> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
> for changesets) be accessed through templates?

I'd make label() accept color names first, since that gives us the 
desired behavior without the BC issue.  Then perhaps we can add a 
color() function that's just an alias for label().
Matt Mackall - April 7, 2014, 7:27 p.m.
On Mon, 2014-04-07 at 12:27 -0500, Sean Farley wrote:
> Matt Mackall <mpm@selenic.com> writes:
> 
> > On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
> >> # HG changeset patch
> >> # User Sean Farley <sean.michael.farley@gmail.com>
> >> # Date 1396670819 18000
> >> #      Fri Apr 04 23:06:59 2014 -0500
> >> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
> >> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
> >> color: rename 'label' template function to 'color'
> >
> > Breaks backward compatibility. Go directly to Jail, do not pass Go, do
> > not collect $200.
> 
> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
> for changesets) be accessed through templates?

'labels' is still workable, if suboptimal. 'markers' or 'marks' is still
in the running, despite what Pierre-Yves says. 'names' might be possible
as well.
Sean Farley - April 7, 2014, 7:46 p.m.
Matt Mackall <mpm@selenic.com> writes:

> On Mon, 2014-04-07 at 12:27 -0500, Sean Farley wrote:
>> Matt Mackall <mpm@selenic.com> writes:
>> 
>> > On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
>> >> # HG changeset patch
>> >> # User Sean Farley <sean.michael.farley@gmail.com>
>> >> # Date 1396670819 18000
>> >> #      Fri Apr 04 23:06:59 2014 -0500
>> >> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
>> >> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
>> >> color: rename 'label' template function to 'color'
>> >
>> > Breaks backward compatibility. Go directly to Jail, do not pass Go, do
>> > not collect $200.
>> 
>> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
>> for changesets) be accessed through templates?
>
> 'labels' is still workable, if suboptimal. 'markers' or 'marks' is still
> in the running, despite what Pierre-Yves says. 'names' might be possible
> as well.

Fair enough. I prefer 'label' as the name but will wait for the outcome
of this running.
Angel Ezquerra - April 7, 2014, 8:58 p.m.
On Mon, Apr 7, 2014 at 9:46 PM, Sean Farley
<sean.michael.farley@gmail.com> wrote:
>
> Matt Mackall <mpm@selenic.com> writes:
>
>> On Mon, 2014-04-07 at 12:27 -0500, Sean Farley wrote:
>>> Matt Mackall <mpm@selenic.com> writes:
>>>
>>> > On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
>>> >> # HG changeset patch
>>> >> # User Sean Farley <sean.michael.farley@gmail.com>
>>> >> # Date 1396670819 18000
>>> >> #      Fri Apr 04 23:06:59 2014 -0500
>>> >> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
>>> >> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
>>> >> color: rename 'label' template function to 'color'
>>> >
>>> > Breaks backward compatibility. Go directly to Jail, do not pass Go, do
>>> > not collect $200.
>>>
>>> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
>>> for changesets) be accessed through templates?
>>
>> 'labels' is still workable, if suboptimal. 'markers' or 'marks' is still
>> in the running, despite what Pierre-Yves says. 'names' might be possible
>> as well.
>
> Fair enough. I prefer 'label' as the name but will wait for the outcome
> of this running.

I don't recall having read any discussion about "arbitrary markers for
changesets". I've only found an RFC by Sean related to this. Can you
guys point me to the relevant thread(s)? Is there a plan to add
"arbitrary" markers to changesets? e.g. will it be possible to "mark"
revisions with the same marker (e.g. integrated, etc)? will they be
mutable?

Cheers,

Angel
Sean Farley - April 7, 2014, 9:02 p.m.
Angel Ezquerra <angel.ezquerra@gmail.com> writes:

> On Mon, Apr 7, 2014 at 9:46 PM, Sean Farley
> <sean.michael.farley@gmail.com> wrote:
>>
>> Matt Mackall <mpm@selenic.com> writes:
>>
>>> On Mon, 2014-04-07 at 12:27 -0500, Sean Farley wrote:
>>>> Matt Mackall <mpm@selenic.com> writes:
>>>>
>>>> > On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
>>>> >> # HG changeset patch
>>>> >> # User Sean Farley <sean.michael.farley@gmail.com>
>>>> >> # Date 1396670819 18000
>>>> >> #      Fri Apr 04 23:06:59 2014 -0500
>>>> >> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
>>>> >> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
>>>> >> color: rename 'label' template function to 'color'
>>>> >
>>>> > Breaks backward compatibility. Go directly to Jail, do not pass Go, do
>>>> > not collect $200.
>>>>
>>>> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
>>>> for changesets) be accessed through templates?
>>>
>>> 'labels' is still workable, if suboptimal. 'markers' or 'marks' is still
>>> in the running, despite what Pierre-Yves says. 'names' might be possible
>>> as well.
>>
>> Fair enough. I prefer 'label' as the name but will wait for the outcome
>> of this running.
>
> I don't recall having read any discussion about "arbitrary markers for
> changesets". I've only found an RFC by Sean related to this. Can you
> guys point me to the relevant thread(s)? Is there a plan to add
> "arbitrary" markers to changesets? e.g. will it be possible to "mark"
> revisions with the same marker (e.g. integrated, etc)? will they be
> mutable?

It was mainly discussed on IRC but the idea is to provide a way for
extensions to add a label to a changeset without polluting tags or
bookmarks.
Matt Mackall - April 7, 2014, 9:02 p.m.
On Mon, 2014-04-07 at 22:58 +0200, Angel Ezquerra wrote:
> On Mon, Apr 7, 2014 at 9:46 PM, Sean Farley
> <sean.michael.farley@gmail.com> wrote:
> >
> > Matt Mackall <mpm@selenic.com> writes:
> >
> >> On Mon, 2014-04-07 at 12:27 -0500, Sean Farley wrote:
> >>> Matt Mackall <mpm@selenic.com> writes:
> >>>
> >>> > On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
> >>> >> # HG changeset patch
> >>> >> # User Sean Farley <sean.michael.farley@gmail.com>
> >>> >> # Date 1396670819 18000
> >>> >> #      Fri Apr 04 23:06:59 2014 -0500
> >>> >> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
> >>> >> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
> >>> >> color: rename 'label' template function to 'color'
> >>> >
> >>> > Breaks backward compatibility. Go directly to Jail, do not pass Go, do
> >>> > not collect $200.
> >>>
> >>> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
> >>> for changesets) be accessed through templates?
> >>
> >> 'labels' is still workable, if suboptimal. 'markers' or 'marks' is still
> >> in the running, despite what Pierre-Yves says. 'names' might be possible
> >> as well.
> >
> > Fair enough. I prefer 'label' as the name but will wait for the outcome
> > of this running.
> 
> I don't recall having read any discussion about "arbitrary markers for
> changesets". I've only found an RFC by Sean related to this. Can you
> guys point me to the relevant thread(s)? Is there a plan to add
> "arbitrary" markers to changesets? e.g. will it be possible to "mark"
> revisions with the same marker (e.g. integrated, etc)? will they be
> mutable?

No, this is just a catchall name for existing naming features:
bookmarks/tags/branches.
Angel Ezquerra - April 7, 2014, 9:14 p.m.
On Mon, Apr 7, 2014 at 11:02 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Mon, 2014-04-07 at 22:58 +0200, Angel Ezquerra wrote:
>> On Mon, Apr 7, 2014 at 9:46 PM, Sean Farley
>> <sean.michael.farley@gmail.com> wrote:
>> >
>> > Matt Mackall <mpm@selenic.com> writes:
>> >
>> >> On Mon, 2014-04-07 at 12:27 -0500, Sean Farley wrote:
>> >>> Matt Mackall <mpm@selenic.com> writes:
>> >>>
>> >>> > On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
>> >>> >> # HG changeset patch
>> >>> >> # User Sean Farley <sean.michael.farley@gmail.com>
>> >>> >> # Date 1396670819 18000
>> >>> >> #      Fri Apr 04 23:06:59 2014 -0500
>> >>> >> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
>> >>> >> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
>> >>> >> color: rename 'label' template function to 'color'
>> >>> >
>> >>> > Breaks backward compatibility. Go directly to Jail, do not pass Go, do
>> >>> > not collect $200.
>> >>>
>> >>> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
>> >>> for changesets) be accessed through templates?
>> >>
>> >> 'labels' is still workable, if suboptimal. 'markers' or 'marks' is still
>> >> in the running, despite what Pierre-Yves says. 'names' might be possible
>> >> as well.
>> >
>> > Fair enough. I prefer 'label' as the name but will wait for the outcome
>> > of this running.
>>
>> I don't recall having read any discussion about "arbitrary markers for
>> changesets". I've only found an RFC by Sean related to this. Can you
>> guys point me to the relevant thread(s)? Is there a plan to add
>> "arbitrary" markers to changesets? e.g. will it be possible to "mark"
>> revisions with the same marker (e.g. integrated, etc)? will they be
>> mutable?
>
> No, this is just a catchall name for existing naming features:
> bookmarks/tags/branches.

Thanks for the clarification, although I would have preferred Sean's
answer better :->

Angel
Sean Farley - April 11, 2014, 9:13 p.m.
Sean Farley <sean.michael.farley@gmail.com> writes:

> Matt Mackall <mpm@selenic.com> writes:
>
>> On Mon, 2014-04-07 at 12:27 -0500, Sean Farley wrote:
>>> Matt Mackall <mpm@selenic.com> writes:
>>> 
>>> > On Sun, 2014-04-06 at 21:32 -0500, Sean Farley wrote:
>>> >> # HG changeset patch
>>> >> # User Sean Farley <sean.michael.farley@gmail.com>
>>> >> # Date 1396670819 18000
>>> >> #      Fri Apr 04 23:06:59 2014 -0500
>>> >> # Node ID aa43236b2340fadf49b087929f0e5ed514ec0c60
>>> >> # Parent  c7ceae0faf6997ff3c262a0b641719b6fd357055
>>> >> color: rename 'label' template function to 'color'
>>> >
>>> > Breaks backward compatibility. Go directly to Jail, do not pass Go, do
>>> > not collect $200.
>>> 
>>> Ok, that's what I thought. How should 'labels' (i.e. arbitrary markers
>>> for changesets) be accessed through templates?
>>
>> 'labels' is still workable, if suboptimal. 'markers' or 'marks' is still
>> in the running, despite what Pierre-Yves says. 'names' might be possible
>> as well.
>
> Fair enough. I prefer 'label' as the name but will wait for the outcome
> of this running.

This would be a good time for anyone with strong opinions on this naming
to chime in.

Patch

diff --git a/hgext/color.py b/hgext/color.py
--- a/hgext/color.py
+++ b/hgext/color.py
@@ -379,28 +379,28 @@  class colorui(uimod.ui):
         if effects:
             return '\n'.join([render_effects(s, effects)
                               for s in msg.split('\n')])
         return msg
 
-def templatelabel(context, mapping, args):
+def templatecolor(context, mapping, args):
     if len(args) != 2:
-        # i18n: "label" is a keyword
-        raise error.ParseError(_("label expects two arguments"))
+        # i18n: "color" is a keyword
+        raise error.ParseError(_("color expects two arguments"))
 
     thing = templater._evalifliteral(args[1], context, mapping)
 
     # apparently, repo could be a string that is the favicon?
     repo = mapping.get('repo', '')
     if isinstance(repo, str):
         return thing
 
-    label = templater._evalifliteral(args[0], context, mapping)
+    color = templater._evalifliteral(args[0], context, mapping)
 
     thing = templater.stringify(thing)
-    label = templater.stringify(label)
+    color = templater.stringify(color)
 
-    return repo.ui.label(thing, label)
+    return repo.ui.label(thing, color)
 
 def uisetup(ui):
     if ui.plain():
         return
     if not isinstance(ui, colorui):
@@ -412,11 +412,11 @@  def uisetup(ui):
         if mode:
             extstyles()
             configstyles(ui_)
         return orig(ui_, opts, cmd, cmdfunc)
     extensions.wrapfunction(dispatch, '_runcommand', colorcmd)
-    templater.funcs['label'] = templatelabel
+    templater.funcs['color'] = templatecolor
 
 def extsetup(ui):
     commands.globalopts.append(
         ('', 'color', 'auto',
          # i18n: 'always', 'auto', and 'never' are keywords and should
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -347,16 +347,16 @@  def join(context, mapping, args):
             first = False
         else:
             yield joiner
         yield x
 
-def label(context, mapping, args):
+def color(context, mapping, args):
     if len(args) != 2:
-        # i18n: "label" is a keyword
-        raise error.ParseError(_("label expects two arguments"))
+        # i18n: "color" is a keyword
+        raise error.ParseError(_("color expects two arguments"))
 
-    # ignore args[0] (the label string) since this is supposed to be a a no-op
+    # ignore args[0] (the color string) since this is supposed to be a a no-op
     yield _evalifliteral(args[1], context, mapping)
 
 def revset(context, mapping, args):
     """usage: revset(query[, formatargs...])
     """
@@ -481,11 +481,11 @@  funcs = {
     "get": get,
     "if": if_,
     "ifcontains": ifcontains,
     "ifeq": ifeq,
     "join": join,
-    "label": label,
+    "color": color,
     "pad": pad,
     "revset": revset,
     "rstdoc": rstdoc,
     "shortest": shortest,
     "strip": strip,
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
@@ -1763,13 +1763,13 @@  Test recursive evaluation:
   > [color]
   > mode=ansi
   > text.{rev} = red
   > text.1 = green
   > EOF
-  $ hg log --color=always -l 1 --template '{label(branch, "text\n")}'
+  $ hg log --color=always -l 1 --template '{color(branch, "text\n")}'
   \x1b[0;31mtext\x1b[0m (esc)
-  $ hg log --color=always -l 1 --template '{label("text.{rev}", "text\n")}'
+  $ hg log --color=always -l 1 --template '{color("text.{rev}", "text\n")}'
   \x1b[0;32mtext\x1b[0m (esc)
 
 Test branches inside if statement:
 
   $ hg log -r 0 --template '{if(branches, "yes", "no")}\n'