Re: [PATCH v2 4/7] i18n: builtin/pull.c: mark strings for translation

From: Vasco Almeida <vascomalmeida@sapo.pt>
Date: Apr 11, 2016, 10:43 UTC
Message-ID: <570B7FCD.5050809@sapo.pt>
In-Reply-To: <xmqqinzptkaw.fsf@gitster.mtv.corp.google.com>

Às 18:01 de 10-04-2016, Junio C Hamano escreveu:

Show 11 quoted lines

> Vasco Almeida <vascomalmeida@sapo.pt> writes:
> 
>> Some translations might also translate "<remote>" and "<branch>".
> 
> This offers an interesting observation that I didn't think of while
> reviewing the first round of this series.
> 
> Do translations want to translate "remote" and "branch" without the
> "<bra" and "ket>"?  Or is it better to allow translations to adjust
> the "quote around a placeholder" in a locale dependent way?
>

I think they want to translate without brackets. From what I can see, translator are using the same quoting. The use of this kind of placeholders should be a new thing to languages, and might be limited to some backgrounds or situations. So, translator might adopt the English way, since there are no better or equivalent way in their languages. Or I may be be assuming to much?

Show 36 quoted lines

>> @@ -458,13 +458,13 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
>>  			fprintf_ln(stderr, _("Please specify which branch you want to merge with."));
>>  		fprintf_ln(stderr, _("See git-pull(1) for details."));
>>  		fprintf(stderr, "\n");
>> -		fprintf_ln(stderr, "    git pull <remote> <branch>");
>> +		fprintf_ln(stderr, "    git pull <%s> <%s>", _("remote"), _("branch"));
> 
> I know this hunk follows I suggested, i.e. "quotes around a
> placeholder is universal and locale independent".  However, ...
> 
>>  		fprintf(stderr, "\n");
>>  	} else if (!curr_branch->merge_nr) {
>>  		const char *remote_name = NULL;
>>  
>>  		if (for_each_remote(get_only_remote, &remote_name) || !remote_name)
>> -			remote_name = "<remote>";
>> +			remote_name = _("<remote>");
>>  
> ... this does not.  It allows to translate the "quote around a
> placeholder".  And where this phony "remote_name" string is used,
> there is also this reference to <branch>:
> 
>     fprintf_ln(stderr, _("If you wish to set tracking information for ..."
>                     "\n"
>                     "    git branch --set-upstream-to=%s/<branch> %s\n"),
>                     remote_name, curr_branch->name);
> 
> which also does.
> 
> Perhaps the first hunk at around ll.458 would want to do
> 
>> +	fprintf_ln(stderr, "    git pull %s %s", _("<remote>"), _("<branch>"));
> 
> to be consistent and more flexible for the translator's needs?  The
> quoting convention may be locale dependent after all.
>

If we think placeholders quoting is important for localization, we could mark them for translation in usage_argh() function in parse-options.c:

s = literal ? " %s" : _(" <%s>");

and do the same for other cases of <%s> in that function. Plus, add a comment for translator, perhaps.

Otherwise, something like the following just to be consistent:

diff --git a/builtin/pull.c b/builtin/pull.c
index 617893c..317c3ce 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -463,8 +463,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
        } else if (!curr_branch->merge_nr) {
                const char *remote_name = NULL;
 
-               if (for_each_remote(get_only_remote, &remote_name) || !remote_name)
-                       remote_name = _("<remote>");
+               for_each_remote(get_only_remote, &remote_name);
 
                fprintf_ln(stderr, _("There is no tracking information for the current branch."));
                if (opt_rebase)
@@ -477,8 +476,11 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
                fprintf(stderr, "\n");
                fprintf_ln(stderr, _("If you wish to set tracking information for this branch you can do so with:"));
                fprintf(stderr, "\n");
-               fprintf_ln(stderr, "    git branch --set-upstream-to=%s/<%s> %s\n",
-                               remote_name, _("branch"), curr_branch->name);
+               fprintf_ln(stderr, remote_name
+                                  ? "    git branch --set-upstream-to=%s/<%s> %s\n"
+                                  : "    git branch --set-upstream-to=<%s>/<%s> %s\n",
+                               remote_name ? remote_name : _("remote"),
+                               _("branch"), curr_branch->name);
        } else
                fprintf_ln(stderr, _("Your configuration specifies to merge with the ref '%s'\n"
                        "from the remote, but no such ref was fetched."),

I think there is no need to internationalize the bracket. If we did so,
they would end up just in the way without adding much value for
translators.
What do you think?