Re: [PATCH 0/7] More i18n fixes

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 21, 2022, 13:59 UTC
Message-ID: <220321.86ils79z0c.gmgdl@evledraar.gmail.com>
In-Reply-To: <e44b6ccf-21a2-72c6-4d40-dc0004895255@kdbg.org>

On Mon, Mar 21 2022, Johannes Sixt wrote:

Show 22 quoted lines

> Am 20.03.22 um 22:54 schrieb Jean-Noël Avila via GitGitGadget:
>> This is another i18n PR (and hopefully the last for a while).
>> 
>> As usual, the intent is kept the same: curbing the number of strings to
>> translate, remove constant, error prone parts out of the way, trying in some
>> sense to "put a precedent" so that the template strings can be reused later.
>
> I feel that many of the example conversions look like sentence lego
> because there remains only one English word, e.g., "'%s' failed". The
> converted code does not leave a hint for the translators what the %s
> will be. Is it a command, a function name, somehting else? Even if the
> hint was provided, different translations may be required depending on
> the substituted entity. Did you investigate the existing translations
> whether all of them can be converted to the new scheme?
>
>> This series has also a RFC status: can "bad argument" messages be merged
>> with unrecognized argument?
>
> The cases that patch 7/7 transforms look like they need not keep
> "unrecognized argument", but can be converted to "bad argument".
>
> Disclaimer: neither am I a translator nor a user of a translated Git.

Just to add to this:

 - Careful use of sentence lego is OK, but e.g. in my native language a
   command-line option would use a male noun article, whereas commands
   would be feminine.

   (I still haven't submitted an Icelandic translation, but this applies
   in general).

   As a result string like "'%s' failed" can be *workable*, i.e. you can
   translate it assuming you'll get any arbitrary string, but the
   translation will often be rather tortured.

   So it's much preferred (and this also goes to Johannes's comment) to
   instead do e.g.:

       "failed to run the '%s' command"
       "failed to use the '%s' argument"

   Or whatever, and e.g. for:
	
	-		return strbuf_addf_ret(err, -1, _("%%(objecttype) does not take arguments"));
	+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(objecttype)");

   Instead say "the '%s' format does not...", i.e. disambiguate with
   "format".

 - While perfect shouldn't be the enemy of the good, it would be most
   welcome to improve some of the warts revealed by these messages,
   notably that e.g. the "failed to run X command" don't report
   errno. E.g. this in git.c is a good template (except for the "\n" we
   should ideally get rid of):

       _("failed to run command '%s': %s\n")

 - On that topic, it would be really useful to see if we can unify some
   of these with *existing* po/git.pot messaging, I don't know if that's
   part of your workflow, but in some cases I've seen we can either
   tweak wording slightly to match an existing message, or could further
   unify some existing similar messages.

 - Even if we say "failed to run git-apply" or whatever now we should
   really be adding quotes to these as we convert them. In some cases
   the changes that (good):
	
	-		die(_("git-http-push failed"));
	+		die(_("'%s' failed"), "git-http-push");

   But not in others (bad):
	
	-		res = error(_("Bad bisect_write argument: %s"), state);
	+		res = error(_("bad %s argument: %s"), "bisect_write", state);

   I.e. that should be 'bad '%s' argument. And also on the "unify" point
   above, e.g. grep.c has this:

       grep.c: die("bad %s argument: %s", opt, arg);

   So we could covert that one to "bad '%s' argument: '%s"" while we're
   at it...

- In some cases there's ucase to lcase conversions, like Bad->bad above
  (good), but others are missed, e.g. (also missing quotes as noted
  above):

	-		die(_("Server does not support --shallow-since"));
	+		die(_("Server does not support %s"), "--shallow-since");

 - On quotes, let's consistently use '' quotes, and not e.g.g:
	
	-		die(_("`scalar list` does not take arguments"));
	+		die(_("%s does not take arguments"), "`scalar list`");