[PATCH] namespaces: add logname member to namespace object

[Sean Farley]

Pierre-Yves David writes:

> On 01/14/2015 04:49 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley at gmail.com>
>> # Date 1421281900 28800
>> #      Wed Jan 14 16:31:40 2015 -0800
>> # Node ID 0508ab9d59758c22fa968d3bc2a184c51dd2c7ff
>> # Parent  f6070d3a9cb89a50ae3112f7c3d22f4ccc5c4db7
>> namespaces: add logname member to namespace object
>>
>> Previously, there was no way to change the name used for 'hg log' output. This
>> was inconvenient for extensions that want a template name longer than 12
>> characters (e.g. remotebookmarks) but a different name for 'hg log'.
>
> The feature itself make sense. I've feedback on the implementation.
>
>
>>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -914,16 +914,16 @@ class changeset_printer(object):
>>           for name, ns in self.repo.names.iteritems():
>>               # branches has special logic already handled above, so here we just
>>               # skip it
>>               if name == 'branches':
>>                   continue
>> -            # we will use the templatename as the color name since those two
>> -            # should be the same
>> +            # we will use the logname as the color name since those two should
>> +            # be the same
>
> We probably want different color label too. (eg, if we use bookmarks for 
> both local and remote (using the /) to disambigious. we still want 
> different label. don't we?

I actually just realized that when working on remotebookmarks. You are
right. I could add both variables: logname and colorname.

>>               for name in ns.names(self.repo, changenode):
>>                   # i18n: column positioning for "hg log"
>> -                tname = _(("%s:" % ns.templatename).ljust(13) + "%s\n") % name
>> -                self.ui.write("%s" % tname, label='log.%s' % ns.templatename)
>> +                lname = _(("%s:" % ns.logname).ljust(13) + "%s\n") % name
>> +                self.ui.write("%s" % lname, label='log.%s' % ns.logname)
>>           if self.ui.debugflag:
>>               # i18n: column positioning for "hg log"
>>               self.ui.write(_("phase:       %s\n") % _(ctx.phasestr()),
>>                             label='log.phase')
>>           for parent in parents:
>> diff --git a/mercurial/namespaces.py b/mercurial/namespaces.py
>> --- a/mercurial/namespaces.py
>> +++ b/mercurial/namespaces.py
>> @@ -25,23 +25,23 @@ class namespaces(object):
>>           # shorten the class name for less indentation
>>           ns = namespace
>>
>>           # we need current mercurial named objects (bookmarks, tags, and
>>           # branches) to be initialized somewhere, so that place is here
>> -        n = ns("bookmarks", "bookmark",
>> +        n = ns("bookmarks", "bookmark", "bookmark",
>>                  lambda repo: repo._bookmarks.keys(),
>>                  lambda repo, name: tolist(repo._bookmarks.get(name)),
>>                  lambda repo, name: repo.nodebookmarks(name))
>
> 1) We can probably start using named argument so that the initialisation 
> get a bit more readable

Very true.

> 2) We probably want to have default value too. if in most case the 
> template name and the logname are the same, lets reuse template name by 
> default.

Yeah, I was thinking that as well.

>>           self.addnamespace(n)
>>
>> -        n = ns("tags", "tag",
>> +        n = ns("tags", "tag", "tag",
>>                  lambda repo: [t for t, n in repo.tagslist()],
>>                  lambda repo, name: tolist(repo._tagscache.tags.get(name)),
>>                  lambda repo, name: repo.nodetags(name))
>>           self.addnamespace(n)
>>
>> -        n = ns("branches", "branch",
>> +        n = ns("branches", "branch", "branch",
>>                  lambda repo: repo.branchmap().keys(),
>>                  lambda repo, name: tolist(repo.branchtip(name, True)),
>>                  lambda repo, node: [repo[node].branch()])
>>           self.addnamespace(n)
>>
>> @@ -118,22 +118,25 @@ class namespace(object):
>>         'namemap': function that takes a name and returns a list of nodes
>>         'nodemap': function that takes a node and returns a list of names
>>
>>       """
>>
>> -    def __init__(self, name, templatename, listnames, namemap, nodemap):
>> +    def __init__(self, name, templatename, logname, listnames, namemap,
>> +                 nodemap):
>>           """create a namespace
>>
>>           name: the namespace to be registered (in plural form)
>>           listnames: function to list all names
>>           templatename: the name to use for templating
>> +        logname: the name to use for log output
>>           namemap: function that inputs a node, output name(s)
>>           nodemap: function that inputs a name, output node(s)
>>
>>           """
>>           self.name = name
>>           self.templatename = templatename
>> +        self.logname = logname
>>           self.listnames = listnames
>>           self.namemap = namemap
>>           self.nodemap = nodemap
>>
>>       def names(self, repo, node):
>> diff --git a/tests/test-log.t b/tests/test-log.t
>> --- a/tests/test-log.t
>> +++ b/tests/test-log.t
>> @@ -1565,11 +1565,11 @@ Check that adding an arbitrary name show
>>     > """A small extension to test adding arbitrary names to a repo"""
>>     > from mercurial.namespaces import namespace
>>     >
>>     > def reposetup(ui, repo):
>>     >     foo = {'foo': repo[0].node()}
>> -  >     ns = namespace("bars", "bar",
>> +  >     ns = namespace("bars", "bar", "bar",
>>     >                    lambda r: foo.keys(),
>>     >                    lambda r, name: foo.get(name),
>>     >                    lambda r, node: [name for name, n
>>     >                                     in foo.iteritems()
>>     >                                     if n == node])
>
> Can we get a test that actuall tests the new feature ;-)

Heh, fair enough.