Re: [PATCH v3 24/39] i18n: bisect: enable l10n of bisect terms in messages

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 2, 2016, 17:33 UTC
Message-ID: <xmqqbn3jv6ka.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <574FF6A6.8030609@sapo.pt>

Vasco Almeida <vascomalmeida@sapo.pt> writes:

Show 11 quoted lines

> Às 17:38 de 01-06-2016, Junio C Hamano escreveu:
>> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>> 
>>> +enum term { BAD, GOOD, OLD, NEW };
>>> +static const char *term_names[] = {
>>> +/* TRANSLATORS: in bisect.c source code file, the following terms are
>>> +   used to describe a "bad commit", "good commit", "new revision", etc.
>>> +   Please, if you can, check the source when you are not sure if a %s
>>> +   would be replaced by one of the following terms. */
>>> +	N_("bad"), N_("good"), N_("old"), N_("new"),  NULL
>>> +};

As you pointed out (below), I _did_ misread these N_() ...

Show 15 quoted lines

>>>  /* Remember to update object flag allocation in object.h */
>>>  #define COUNTED		(1u<<16)
>>>  
>>> @@ -725,12 +734,12 @@ static void handle_bad_merge_base(void)
>>>  	if (is_expected_rev(current_bad_oid)) {
>>>  		char *bad_hex = oid_to_hex(current_bad_oid);
>>>  		char *good_hex = join_sha1_array_hex(&good_revs, ' ');
>>> -		if (!strcmp(term_bad, "bad") && !strcmp(term_good, "good")) {
>>> +		if (!strcmp(term_bad, term_names[BAD]) && !strcmp(term_good, term_names[GOOD])) {
>>>  			fprintf(stderr, _("The merge base %s is bad.\n"
>>>  				"This means the bug has been fixed "
>>>  				"between %s and [%s].\n"),
>>>  				bad_hex, bad_hex, good_hex);
>>> -		} else if (!strcmp(term_bad, "new") && !strcmp(term_good, "old")) {
>>> +		} else if (!strcmp(term_bad, term_names[NEW]) && !strcmp(term_good, term_names[OLD])) {

... which means the above two changes are "since we have these four word constants in an array up there, let's use them", i.e. there is no change in behaviour when running the program in any locale, but it is a good programming practice (as long as the term_names[] array that holds the four word constants and indexes into it with enum term is justifiable, that is).

Show 6 quoted lines

>>> @@ -739,7 +748,7 @@ static void handle_bad_merge_base(void)
>>>  			fprintf(stderr, _("The merge base %s is %s.\n"
>>>  				"This means the first '%s' commit is "
>>>  				"between %s and [%s].\n"),
>>> -				bad_hex, term_bad, term_good, bad_hex, good_hex);
>>> +				bad_hex, _(term_bad), _(term_good), bad_hex, good_hex);

But this translation still does not make much sense to me.

If the user did not change term_bad and term_good from the default ones (i.e. the four words above), the flow of control would not come here; one of the above two !strcmp() lines you changed would trigger.

On the other hand, if the user did change term_bad and term_good by using --term-old=velho --term-new=novo earlier (which is stored in BISECT_TERMS and read into these two variables), these are strings that Git encounters at runtime; you cannot expect _(term_bad) aka _("novo") to be translated by *.po files for the locale.

And even if it were somehow translatable, you do not want to.

Imagine a case where there is a Portuguese word the user used in place of 'novo' in the example above, and there is an English word with the same spelling with a totally different meaning. And further imagine that English word is used elsewhere in Git and has translation to Portuguese in your *.po file. _(term_bad) would become a translation of that English word into Portuguese, which would be a word totally unrelated to the word the user used originally when she did --term-old=<that word in portuguese>.

So I do not think _(term_bad) and _(term_good) is a good idea in the last hunk quoted above (it is not just "not a good idea", but actually would be harmful), which would make term_names[] and "enum term" unnecessary.

There is a case where one of the default four words could seep through to the last else clause. Is that what you are trying to address?

That is, when term_bad is left as "bad" but term_good is customized to "super", neither "term_bad is 'bad' and term_good is 'good'" nor "term_bad is 'new' and term_good is 'old" is true, and we would say

	Merge base is BAD.  This means the first SUPER commit is ...

With your change to mark _(term_bad) and _(term_good), the message would translate term_bad to whatever word it is in your language, and term_good that comes from the end user will be left as SUPER as the user typed. I am not sure if it is a good thing to translate only BAD while leaving SUPER as-is in such a case.