Re: [PATCH v3 2/2] blame: use a helper to get suitable blame_date_width

2014-04-22 Thread Jiang Xin
2014-04-22 3:19 GMT+08:00 Junio C Hamano gits...@pobox.com:
 Junio C Hamano gits...@pobox.com writes:

 What I am wondering is if we can do something
 like this:
 ...

 Nah, that is a lot more stupid than just doing

 In code:

 blame_date_width = strtoul(_(4 years, 11 months ago), NULL, 10) + 
 1;

 In git.pot:

 #. This string is used to tell us the maximum display width for a
 #. relative timestamp in git blame output.  For C locale, 4 years,
 #. 11 months ago, which takes 22 places, is the longest among 
 various
 #. forms of relative timestamps, but your language may need more or
 #. fewer display columns.
 msgid 4 years, 11 months ago
 msgstr 

 In de.po:
 #. This string is used to tell us the maximum display width for a
 #. relative timestamp in git blame output.  For C locale, 4 years,
 #. 11 months ago, which takes 22 places, is the longest among 
 various
 #. forms of relative timestamps, but your language may need more or
 #. fewer display columns.
 msgid 4 years, 11 months ago
 msgstr vor 4 Jahren, und 11 Monaten

 which is essentially how your very original looked like (modulo the
 comments).  So let's not try to be clever or cute, and just have a
 good instruction in the TRANSLATORS comments.

 Sorry for flipping and flopping on this one.

I will send patch v4 tonight, and I think all l10n team leaders should
pay attention
to this thread:

 * http://thread.gmane.org/gmane.comp.version-control.git/246464

-- 
Jiang Xin
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/2] blame: use a helper to get suitable blame_date_width

2014-04-21 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 When show date in relative date format for git-blame, the max display
 width of datetime is set as the length of the string Thu Oct 19
 16:00:04 2006 -0700 (30 characters long).  But actually the max width
 for C locale is only 22 (the length of string x years, xx months ago).
 And for other locale, it maybe smaller.  E.g. For Chinese locale, only
 needs a half (16-character width).

 Add a helper function date_relative_maxwidth() to date.c, which returns
 the suitable display width for the relative date field in different
 locale.

 Suggested-by: Junio C Hamano gits...@pobox.com
 Signed-off-by: Jiang Xin worldhello@gmail.com
 ---
  builtin/blame.c | 64 
 -
  1 file changed, 63 insertions(+), 1 deletion(-)

That does not seem to have the helper in date.c immediately next to
the definition of show_date_relative(), as the above log message
claims, does it?

I was hoping that you would respond with a set of counter-arugments
to shoot the suggestion down.  Here are a few obvious ones:

 - By inlining the implementation of show_date_relative() in the new
   helper function, we would end up having to maintain two copies of
   every format strings.

 - Even if we don't inline the logic and duplicate the format
   strings, and call show_date_relative() with some predetermined
   offsets instead, e.g.

static int date_relative_maxwidth(void)
{
struct timebuf = STRBUF_INIT;
struct timeval now;
static int maxwidth = 0;

gettimeofday(now, NULL);

/* in the future */
show_date_relative(now-tv_sec + 100, 0, now, timebuf);
maxwidth = maxwidth  timebuf.len ? timebuf.len : maxwidth;
strbuf_reset(timebuf);

/* seconds ago */
show_date_relative(now-tv_sec - 89, 0, now, timebuf);
maxwidth = maxwidth  timebuf.len ? timebuf.len : maxwidth;
strbuf_reset(timebuf);

...
}

   we still end up hardcoding the logic in the code.

 - There is no guarantee that these predetermined offsets would
   produce the longest possible timestamp for the target language
   with these gettext strings.  In a language where the noun
   second takes a lot longer shape for singular than the plural
   (e.g. 1 SECOND vs 2 SEC, perhaps), checking for 89 seconds
   ago may not produce the longest string for %lu seconds ago.
   The approach, we compute everything for translators, sounds
   nice in theory, but may not work well in practice and the worst
   part is that there is no way for translators to work it around,
   unlike your original patch.

So after sleeping on the idea overnight, I think I like the
simplicity of your original patch better.  It just needs to be
explained well for translators to understand.

Sorry for making you go off exploring in a strange direction.

When msgmerge is run to update XX.po with a new version of git.pot,
does it destroy comments an earlier translator placed in XX.po, or
are they kept intact?  What I am wondering is if we can do something
like this:

In code:

blame_date_width = strtoul(_(22 (C)), NULL, 10) + 1; /* add the null 
*/

In git.pot:

#. This string encodes the preferred maximum display width for a
#. relative timestamp in git blame output.  For C locale, 4 years,
#. 11 months ago, which takes 22 places, is the longest among various
#. forms of relative timestamps, but your languate may need more or
#. fewer display columns.  If zh_CN locale needs 14 display columns to
#. hold any relative timestamp in the reasonably near past, for
#. example, translate this string as 14 (zh_CN).
msgid 22 (C)
msgstr 

In de.po:
#. This string encodes the preferred maximum display width for a
#. relative timestamp in git blame output.  For C locale, 4 years,
#. 11 months ago, which takes 22 places, is the longest among various
#. forms of relative timestamps, but your languate may need more or
#. fewer display columns.  If zh_CN locale needs 14 display columns to
#. hold any relative timestamp in the reasonably near past, for
#. example, translate this string as 14 (zh_CN).
#.
#. In de locale, vor %lu Jahren, und %lu Monaten is the
#. longest and fits within 28 display columns.
msgid 22 (C)
msgstr 28 (de)

where the instructions for tranlators to various languages come from
git.pot and the translator to a specific language can describe which
variant in the language described in XX.po is the longest.  For that
to work well, the last two lines in the comment an earlier de
translator added need to be preserved across msgmerge, though.

--
To unsubscribe from this list: send the line unsubscribe git in
the body 

Re: [PATCH v3 2/2] blame: use a helper to get suitable blame_date_width

2014-04-21 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 What I am wondering is if we can do something
 like this:
 ...

Nah, that is a lot more stupid than just doing

 In code:

 blame_date_width = strtoul(_(4 years, 11 months ago), NULL, 10) + 1;

 In git.pot:

 #. This string is used to tell us the maximum display width for a
 #. relative timestamp in git blame output.  For C locale, 4 years,
 #. 11 months ago, which takes 22 places, is the longest among various
 #. forms of relative timestamps, but your language may need more or
 #. fewer display columns.
 msgid 4 years, 11 months ago
 msgstr 

 In de.po:
 #. This string is used to tell us the maximum display width for a
 #. relative timestamp in git blame output.  For C locale, 4 years,
 #. 11 months ago, which takes 22 places, is the longest among various
 #. forms of relative timestamps, but your language may need more or
 #. fewer display columns.
 msgid 4 years, 11 months ago
 msgstr vor 4 Jahren, und 11 Monaten

which is essentially how your very original looked like (modulo the
comments).  So let's not try to be clever or cute, and just have a
good instruction in the TRANSLATORS comments.

Sorry for flipping and flopping on this one.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html