Patchwork [2,of,5] log: use new namespaces api to display names

login
register
mail settings
Submitter Sean Farley
Date Jan. 8, 2015, 12:15 a.m.
Message ID <541de5f46e66ee9f5941.1420676153@laptop.local>
Download mbox | patch
Permalink /patch/7369/
State Accepted
Headers show

Comments

Sean Farley - Jan. 8, 2015, 12:15 a.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1413563197 25200
#      Fri Oct 17 09:26:37 2014 -0700
# Node ID 541de5f46e66ee9f5941d576ab13665901d5fd95
# Parent  cb77acb042a9b5fb26f7e93f5d9b2939acbb67a7
log: use new namespaces api to display names

The only caveat here is that branches must be displayed first due to backwards
compatibility. The order of namespaces is defined to be the 'update' order
which, unfortunately, is not the same as log output order.

It's worth mentioning that the log output is still translated the same as
before since we are formating our strings the same way:

# i18n: column positioning for "hg log"
_("bookmark:    %s\n") % bookmark

becomes

tname = _(("%s:" % ns.templatename).ljust(13) + "%s\n") % name

when name == 'bookmark'. The ljust(13) keeps the strings and whitespace equal.
Adding a new namespace is even easier now because the log output code doesn't
need to change. A future programmer would just need to add the string to the
corresponding .po file (which is the same as they would have had to do
previously).
Wagner Bruna - Jan. 23, 2015, 10:43 p.m.
Just noted an i18n issue with this change (pushed as

http://selenic.com/hg/rev/07309e527df7 ):

On 07-01-2015 22:15, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1413563197 25200
> #      Fri Oct 17 09:26:37 2014 -0700
> # Node ID 541de5f46e66ee9f5941d576ab13665901d5fd95
> # Parent  cb77acb042a9b5fb26f7e93f5d9b2939acbb67a7
> log: use new namespaces api to display names
>
> The only caveat here is that branches must be displayed first due to backwards
> compatibility. The order of namespaces is defined to be the 'update' order
> which, unfortunately, is not the same as log output order.
>
> It's worth mentioning that the log output is still translated the same as
> before since we are formating our strings the same way:
>
> # i18n: column positioning for "hg log"
> _("bookmark:    %s\n") % bookmark
> becomes
>
> tname = _(("%s:" % ns.templatename).ljust(13) + "%s\n") % name
>
> when name == 'bookmark'. The ljust(13) keeps the strings and whitespace equal.
> Adding a new namespace is even easier now because the log output code doesn't
> need to change. A future programmer would just need to add the string to the
> corresponding .po file (which is the same as they would have had to do
> previously).

But translatable strings are extracted from the source code too, by parsing 
those _() calls with hggettext (see "%.po" Makefile target). So we need 
_("tag:"), _("bookmark:"), etc. somewhere in the code, or translators won't 
even notice they need translating (I only noticed myself because I run stable 
tip from sources, and supposedly completed pt_BR a few days ago).

There is also a formatting issue:

$ LANGUAGE=C hg log -r 3.3-rc
changeset:   23918:db8e3f7948b1
branch:      stable
tag:         3.3-rc
parent:      23826:c90d195320c5
parent:      23917:3cbb5bf4035d
user:        Matt Mackall <mpm@selenic.com>
date:        Sat Jan 17 18:28:30 2015 -0800
summary:     merge default into stable for 3.3 feature freeze

$ LANGUAGE=pt_BR hg log -r 3.3-rc
revisão:      23918:db8e3f7948b1
ramo:         stable
tag:         3.3-rc
pai:          23826:c90d195320c5
pai:          23917:3cbb5bf4035d
usuário:      Matt Mackall <mpm@selenic.com>
data:         Sat Jan 17 18:28:30 2015 -0800
sumário:      merge default into stable for 3.3 feature freeze

Those space-padded strings got an extra space because pt_BR needed 12 chars 
for some reason (can't remember right now, but that could easily happen with 
another language). But placing an extra space after "tag:" in its translation 
("etiqueta: ") won't help here, because ljust() will keep returning a 13 char 
string... Also, longer translations (or namespace names) won't get a space 
between ':' and their values.

Regards,
Wagner
Matt Mackall - Jan. 23, 2015, 11:07 p.m.
On Fri, 2015-01-23 at 20:43 -0200, Wagner Bruna wrote:
> Just noted an i18n issue with this change (pushed as

> > It's worth mentioning that the log output is still translated the same as
> > before since we are formating our strings the same way:
> >
> > # i18n: column positioning for "hg log"
> > _("bookmark:    %s\n") % bookmark
> > becomes
> >
> > tname = _(("%s:" % ns.templatename).ljust(13) + "%s\n") % name
> >

> But translatable strings are extracted from the source code too, by parsing 
> those _() calls with hggettext (see "%.po" Makefile target). So we need 
> _("tag:"), _("bookmark:"), etc. somewhere in the code, or translators won't 
> even notice they need translating (I only noticed myself because I run stable 
> tip from sources, and supposedly completed pt_BR a few days ago).

Joy. Yeah, this is a problem.

As a stop-gap for 3.3, we should probably change this:

	n = namespace("bookmarks", templatename="bookmark", listnames=bmknames,
                      namemap=bmknamemap, nodemap=bmknodemap)

to:

	# i18n: column positioning for "hg log"
	n = namespace("bookmarks", templatename="bookmark",
                 logfield=_("bookmark:    %s\n"),
		 listnames=bmknames, namemap=bmknamemap, nodemap=bmknodemap)

and if we have other uses for the translated field name, we can extract them with split(':').

(Note the reintroduction of the comment, which ends up in the .po file.)

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -900,24 +900,30 @@  class changeset_printer(object):
 
         # i18n: column positioning for "hg log"
         self.ui.write(_("changeset:   %d:%s\n") % (rev, hexfunc(changenode)),
                       label='log.changeset changeset.%s' % ctx.phasestr())
 
+        # branches are shown first before any other names due to backwards
+        # compatibility
         branch = ctx.branch()
         # don't show the default branch name
         if branch != 'default':
             # i18n: column positioning for "hg log"
             self.ui.write(_("branch:      %s\n") % branch,
                           label='log.branch')
-        for bookmark in self.repo.nodebookmarks(changenode):
-            # i18n: column positioning for "hg log"
-            self.ui.write(_("bookmark:    %s\n") % bookmark,
-                    label='log.bookmark')
-        for tag in self.repo.nodetags(changenode):
-            # i18n: column positioning for "hg log"
-            self.ui.write(_("tag:         %s\n") % tag,
-                          label='log.tag')
+
+        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
+            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)
         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: