Re: [PATCH v3 11/11] i18n: turn even more messages into "cannot be used together" ones

On Thursday, 30 December 2021 23:36:45 CET Junio C Hamano wrote:

"Jean-Noël Avila via GitGitGadget"  [off-list ref] writes:

quoted

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <redacted>

Even if some of these messages are not subject to gettext i18n, this
helps bring a single style of message for a given error type.

Makes sense, but ...

quoted

diff --git a/builtin/add.c b/builtin/add.c
index 067ec53d69f..dcaa3b5f395 100644
--- a/builtin/add.c
+++ b/builtin/add.c

@@ -566,7 +566,7 @@ int cmd_add(int argc, const char **argv, const char 

*prefix)

quoted

 
 	if (pathspec_from_file) {
 		if (pathspec.nr)
-			die(_("--pathspec-from-file is incompatible 

with pathspec arguments"));

quoted

+			die(_("'--pathspec-from-file' and pathspec 

arguments cannot be used together"));

Don't we want the literal `--pathspec-from-file` outside the format
string to prevent it from l10n?  Or have all the changes in this
series to turn _("use '--concrete-option-name' in message") into
_("use '%s' in message") with '--concrete-option-name' as an
argument done only to reduce the number of distinct format strings?

I just skimmed the rest of the patch but the above comment seems to
apply to all the hunks.

Thanks.

The idea was to apply the '%s' shifting only to strings that can be 
factorized,
in order to curb the number of translatable strings.

Anyway, swiching from 

    die(_("use '--concrete-option-name' in message")) 

into

    die(_("use option '%s' in message"), "--concrete-option-name")

in a more generic way fits perfectly in the spirit I had for this series.

On the other hand, the patch list is already quite large and this logic won't 
be extended to every other candidate strings that are not already changed 
here. That may be the object of another series.

JN