Patchwork namespaces: add logname member to namespace object

login
register
mail settings
Submitter Sean Farley
Date Jan. 15, 2015, 12:49 a.m.
Message ID <0508ab9d59758c22fa96.1421282965@laptop.local>
Download mbox | patch
Permalink /patch/7462/
State Superseded
Commit fef1146b84421d9f26525ae51ae5efce593e0327
Headers show

Comments

Sean Farley - Jan. 15, 2015, 12:49 a.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@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'.
Pierre-Yves David - Jan. 15, 2015, 1:53 a.m.
On 01/14/2015 04:49 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@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 ;-)
Sean Farley - Jan. 15, 2015, 2 a.m.
Pierre-Yves David writes:

> On 01/14/2015 04:49 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@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.
Augie Fackler - Jan. 16, 2015, 3:29 p.m.
On Wed, Jan 14, 2015 at 04:49:25PM -0800, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@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'.

If/when we se a new version of htis, could you give an example of what
a regular vs logname might look like? I wrote remotebranches and I
can't come up with an example offhand.

>
> 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
>              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))
>          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])
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Sean Farley - Jan. 16, 2015, 6:43 p.m.
Augie Fackler writes:

> On Wed, Jan 14, 2015 at 04:49:25PM -0800, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@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'.
>
> If/when we se a new version of htis, could you give an example of what
> a regular vs logname might look like? I wrote remotebranches and I
> can't come up with an example offhand.

It's already been queued but the example is due to 'remotebookmarks'
being one or two characters too long:

$ hg log -r .
changeset:   3:78f83396d79e
bookmark:    babar
remotebookmark:beta/babar
remotebranch:beta/default
parent:      1:7c3bad9141dc
user:        test
date:        Thu Jan 01 00:00:00 1970 +0000
summary:     add d

The templatename (singular case of the plural name 'remotebookmarks')
was used in the log output. We still want to use the name
'remotebookmark' for clarity:

$ hg log -r . -T '{remotebookmarks % "REMOTE: {remotebookmark}\n"}'
REMOTE: beta/babar

Changing the template name just to affect the log output seemed too
constraining. With this patch series, we keep the same template name
(remotebookmark) and change only the log output to, let's say for
example, 'bookmark'. If we do that there is a name clash with using the
same color for 'bookmark' as we do for 'remotebookmark'. Hence, we allow
setting both the log name and the color name.

Patch

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