[PATCH] namespaces: add logname member to namespace object

[Pierre-Yves David]

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?


>               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

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.


>           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 ;-)


-- 
Pierre-Yves David