[PATCH 0/7] More i18n fixes

[PATCH 0/7] More i18n fixes

[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.

This series has also a RFC status: can "bad argument" messages be merged
with unrecognized argument?

Bagas Sanjaya (1):
  sequencer: factor GIT_AUTHOR_* from message strings

Jean-Noël Avila (6):
  i18n: factorize generic failure messages
  i18n: factorize "bad argument" messages
  i18n: factorize "Server does not support foo" messages
  i18n: factorize "foo does not take arguments" messages
  i18n: factorize read-cache error messages
  i18n: factorize unrecognized options arguments messages

 add-patch.c                     |  4 ++--
 builtin/am.c                    |  2 +-
 builtin/bisect--helper.c        |  2 +-
 builtin/commit-graph.c          |  2 +-
 builtin/config.c                |  2 +-
 builtin/env--helper.c           |  2 +-
 builtin/gc.c                    |  8 ++++----
 builtin/merge.c                 |  6 +++---
 builtin/revert.c                |  4 ++--
 contrib/scalar/scalar.c         |  2 +-
 diff.c                          |  4 ++--
 fetch-pack.c                    |  8 ++++----
 read-cache.c                    |  8 ++++----
 ref-filter.c                    |  8 ++++----
 remote-curl.c                   |  2 +-
 sequencer.c                     | 12 ++++++------
 setup.c                         |  4 ++--
 submodule.c                     |  2 +-
 t/t3510-cherry-pick-sequence.sh |  8 ++++----
 t/t6436-merge-overwrite.sh      |  2 +-
 20 files changed, 46 insertions(+), 46 deletions(-)


base-commit: 74cc1aa55f30ed76424a0e7226ab519aa6265061
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1181%2Fjnavila%2Fmore_i18n_fixes-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1181/jnavila/more_i18n_fixes-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1181
-- 
gitgitgadget

[PATCH 1/7] i18n: factorize generic failure messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

In these message the command that failed should not be translated. So
it is simpler to just remove these parts from the message.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 add-patch.c                     | 4 ++--
 builtin/am.c                    | 2 +-
 builtin/gc.c                    | 4 ++--
 builtin/merge.c                 | 6 +++---
 builtin/revert.c                | 4 ++--
 fetch-pack.c                    | 2 +-
 remote-curl.c                   | 2 +-
 setup.c                         | 4 ++--
 t/t3510-cherry-pick-sequence.sh | 8 ++++----
 t/t6436-merge-overwrite.sh      | 2 +-
 10 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 55d719f7845..d545dea2ff2 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1181,7 +1181,7 @@ static int run_apply_check(struct add_p_state *s,
 			    "apply", "--check", NULL);
 	strvec_pushv(&cp.args, s->mode->apply_check_args);
 	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
-		return error(_("'git apply --cached' failed"));
+		return error(_("'%s' failed"), "git apply --cached");
 
 	return 0;
 }
@@ -1683,7 +1683,7 @@ soft_increment:
 			strvec_pushv(&cp.args, s->mode->apply_args);
 			if (pipe_command(&cp, s->buf.buf, s->buf.len,
 					 NULL, 0, NULL, 0))
-				error(_("'git apply' failed"));
+				error(_("'%s' failed"), "git apply");
 		}
 		if (repo_read_index(s->s.r) >= 0)
 			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
diff --git a/builtin/am.c b/builtin/am.c
index 0f4111bafa0..b26a55318e7 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -586,7 +586,7 @@ static int is_mail(FILE *fp)
 	int ret = 1;
 
 	if (fseek(fp, 0L, SEEK_SET))
-		die_errno(_("fseek failed"));
+		die_errno(_("'%s' failed"), "fseek");
 
 	if (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))
 		die("invalid pattern: %s", header_regex);
diff --git a/builtin/gc.c b/builtin/gc.c
index ffaf0daf5d9..d512c127b99 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1104,7 +1104,7 @@ static int multi_pack_index_expire(struct maintenance_run_opts *opts)
 		strvec_push(&child.args, "--no-progress");
 
 	if (run_command(&child))
-		return error(_("'git multi-pack-index expire' failed"));
+		return error(_("'%s' failed"), "git multi-pack-index expire");
 
 	return 0;
 }
@@ -1163,7 +1163,7 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts)
 				  (uintmax_t)get_auto_pack_size());
 
 	if (run_command(&child))
-		return error(_("'git multi-pack-index repack' failed"));
+		return error(_("'%s' failed"), "git multi-pack-index repack");
 
 	return 0;
 }
diff --git a/builtin/merge.c b/builtin/merge.c
index f178f5a3ee1..3155f116610 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -325,7 +325,7 @@ static int save_state(struct object_id *stash)
 	close(cp.out);
 
 	if (finish_command(&cp) || len < 0)
-		die(_("stash failed"));
+		die(_("'%s' failed"), "stash");
 	else if (!len)		/* no changes */
 		goto out;
 	strbuf_setlen(&buffer, buffer.len-1);
@@ -352,7 +352,7 @@ static void read_empty(const struct object_id *oid, int verbose)
 	args[i] = NULL;
 
 	if (run_command_v_opt(args, RUN_GIT_CMD))
-		die(_("read-tree failed"));
+		die(_("'%s' failed"), "read-tree");
 }
 
 static void reset_hard(const struct object_id *oid, int verbose)
@@ -369,7 +369,7 @@ static void reset_hard(const struct object_id *oid, int verbose)
 	args[i] = NULL;
 
 	if (run_command_v_opt(args, RUN_GIT_CMD))
-		die(_("read-tree failed"));
+		die(_("'%s' failed"), "read-tree");
 }
 
 static void restore_state(const struct object_id *head,
diff --git a/builtin/revert.c b/builtin/revert.c
index 51776abea63..ed032eaf0f8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -238,7 +238,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	sequencer_init_config(&opts);
 	res = run_sequencer(argc, argv, &opts);
 	if (res < 0)
-		die(_("revert failed"));
+		die(_("'%s' failed"), "revert");
 	return res;
 }
 
@@ -251,6 +251,6 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 	sequencer_init_config(&opts);
 	res = run_sequencer(argc, argv, &opts);
 	if (res < 0)
-		die(_("cherry-pick failed"));
+		die(_("'%s' failed"), "cherry-pick");
 	return res;
 }
diff --git a/fetch-pack.c b/fetch-pack.c
index 87657907e78..09b1cf6173a 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -989,7 +989,7 @@ static int get_pack(struct fetch_pack_args *args,
 			args->check_self_contained_and_connected &&
 			ret == 0;
 	else
-		die(_("%s failed"), cmd_name);
+		die(_("'%s' failed"), cmd_name);
 	if (use_sideband && finish_async(&demux))
 		die(_("error in sideband demultiplexer"));
 
diff --git a/remote-curl.c b/remote-curl.c
index ff44f41011e..28619583ec5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1287,7 +1287,7 @@ static int push_dav(int nr_spec, const char **specs)
 		strvec_push(&child.args, specs[i]);
 
 	if (run_command(&child))
-		die(_("git-http-push failed"));
+		die(_("'%s' failed"), "git-http-push");
 	return 0;
 }
 
diff --git a/setup.c b/setup.c
index 04ce33cdcd4..3d123cc11fd 100644
--- a/setup.c
+++ b/setup.c
@@ -1506,12 +1506,12 @@ int daemonize(void)
 		case 0:
 			break;
 		case -1:
-			die_errno(_("fork failed"));
+			die_errno(_("'%s' failed"), "fork");
 		default:
 			exit(0);
 	}
 	if (setsid() == -1)
-		die_errno(_("setsid failed"));
+		die_errno(_("'%s' failed"), "setsid");
 	close(0);
 	close(1);
 	close(2);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3b0fa66c33d..75a3fbd469b 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -193,7 +193,7 @@ test_expect_success 'check advice when we move HEAD by committing' '
 	error: there is nothing to skip
 	hint: have you committed already?
 	hint: try "git cherry-pick --continue"
-	fatal: cherry-pick failed
+	fatal: '\''cherry-pick'\'' failed
 	EOF
 	test_must_fail git cherry-pick base..yetanotherpick &&
 	echo c >foo &&
@@ -208,7 +208,7 @@ test_expect_success 'selectively advise --skip while launching another sequence'
 	cat >expect <<-EOF &&
 	error: cherry-pick is already in progress
 	hint: try "git cherry-pick (--continue | --skip | --abort | --quit)"
-	fatal: cherry-pick failed
+	fatal: '\''cherry-pick'\'' failed
 	EOF
 	test_must_fail git cherry-pick picked..yetanotherpick &&
 	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
@@ -216,7 +216,7 @@ test_expect_success 'selectively advise --skip while launching another sequence'
 	cat >expect <<-EOF &&
 	error: cherry-pick is already in progress
 	hint: try "git cherry-pick (--continue | --abort | --quit)"
-	fatal: cherry-pick failed
+	fatal: '\''cherry-pick'\'' failed
 	EOF
 	git reset --merge &&
 	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
@@ -227,7 +227,7 @@ test_expect_success 'allow skipping commit but not abort for a new history' '
 	pristine_detach initial &&
 	cat >expect <<-EOF &&
 	error: cannot abort from a branch yet to be born
-	fatal: cherry-pick failed
+	fatal: '\''cherry-pick'\'' failed
 	EOF
 	git checkout --orphan new_disconnected &&
 	git reset --hard &&
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index c0b7bd7c3fe..e8abcd10f5c 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -166,7 +166,7 @@ test_expect_success 'will not be confused by symlink in leading path' '
 
 cat >expect <<\EOF
 error: Untracked working tree file 'c0.c' would be overwritten by merge.
-fatal: read-tree failed
+fatal: 'read-tree' failed
 EOF
 
 test_expect_success 'will not overwrite untracked file on unborn branch' '
-- 
gitgitgadget

[PATCH 2/7] sequencer: factor GIT_AUTHOR_* from message strings

[Bagas Sanjaya via GitGitGadget]

From: Bagas Sanjaya <bagasdotme@gmail.com>

Factor messages containing GIT_AUTHOR_* variable.

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 sequencer.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 84eed9e96bc..647f5efa656 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -848,17 +848,17 @@ int read_author_script(const char *path, char **name, char **email, char **date,
 	for (i = 0; i < kv.nr; i++) {
 		if (!strcmp(kv.items[i].string, "GIT_AUTHOR_NAME")) {
 			if (name_i != -2)
-				name_i = error(_("'GIT_AUTHOR_NAME' already given"));
+				name_i = error(_("'%s' already given"), "GIT_AUTHOR_NAME");
 			else
 				name_i = i;
 		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_EMAIL")) {
 			if (email_i != -2)
-				email_i = error(_("'GIT_AUTHOR_EMAIL' already given"));
+				email_i = error(_("'%s' already given"), "GIT_AUTHOR_EMAIL");
 			else
 				email_i = i;
 		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_DATE")) {
 			if (date_i != -2)
-				date_i = error(_("'GIT_AUTHOR_DATE' already given"));
+				date_i = error(_("'%s' already given"), "GIT_AUTHOR_DATE");
 			else
 				date_i = i;
 		} else {
@@ -867,11 +867,11 @@ int read_author_script(const char *path, char **name, char **email, char **date,
 		}
 	}
 	if (name_i == -2)
-		error(_("missing 'GIT_AUTHOR_NAME'"));
+		error(_("missing '%s'"), "GIT_AUTHOR_NAME");
 	if (email_i == -2)
-		error(_("missing 'GIT_AUTHOR_EMAIL'"));
+		error(_("missing '%s'"), "GIT_AUTHOR_EMAIL");
 	if (date_i == -2)
-		error(_("missing 'GIT_AUTHOR_DATE'"));
+		error(_("missing '%s'"), "GIT_AUTHOR_DATE");
 	if (date_i < 0 || email_i < 0 || date_i < 0 || err)
 		goto finish;
 	*name = kv.items[name_i].util;
-- 
gitgitgadget

Re: [PATCH 2/7] sequencer: factor GIT_AUTHOR_* from message strings

[Bagas Sanjaya]

On 21/03/22 04.54, Bagas Sanjaya via GitGitGadget wrote:
> From: Bagas Sanjaya <bagasdotme@gmail.com>
> 
> Factor messages containing GIT_AUTHOR_* variable.
> 
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> ---
>   sequencer.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 84eed9e96bc..647f5efa656 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -848,17 +848,17 @@ int read_author_script(const char *path, char **name, char **email, char **date,
>   	for (i = 0; i < kv.nr; i++) {
>   		if (!strcmp(kv.items[i].string, "GIT_AUTHOR_NAME")) {
>   			if (name_i != -2)
> -				name_i = error(_("'GIT_AUTHOR_NAME' already given"));
> +				name_i = error(_("'%s' already given"), "GIT_AUTHOR_NAME");
>   			else
>   				name_i = i;
>   		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_EMAIL")) {
>   			if (email_i != -2)
> -				email_i = error(_("'GIT_AUTHOR_EMAIL' already given"));
> +				email_i = error(_("'%s' already given"), "GIT_AUTHOR_EMAIL");
>   			else
>   				email_i = i;
>   		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_DATE")) {
>   			if (date_i != -2)
> -				date_i = error(_("'GIT_AUTHOR_DATE' already given"));
> +				date_i = error(_("'%s' already given"), "GIT_AUTHOR_DATE");
>   			else
>   				date_i = i;
>   		} else {
> @@ -867,11 +867,11 @@ int read_author_script(const char *path, char **name, char **email, char **date,
>   		}
>   	}
>   	if (name_i == -2)
> -		error(_("missing 'GIT_AUTHOR_NAME'"));
> +		error(_("missing '%s'"), "GIT_AUTHOR_NAME");
>   	if (email_i == -2)
> -		error(_("missing 'GIT_AUTHOR_EMAIL'"));
> +		error(_("missing '%s'"), "GIT_AUTHOR_EMAIL");
>   	if (date_i == -2)
> -		error(_("missing 'GIT_AUTHOR_DATE'"));
> +		error(_("missing '%s'"), "GIT_AUTHOR_DATE");
>   	if (date_i < 0 || email_i < 0 || date_i < 0 || err)
>   		goto finish;
>   	*name = kv.items[name_i].util;

Looks OK. Thanks for picking this up.

Acked-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[PATCH 3/7] i18n: factorize "bad argument" messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 builtin/bisect--helper.c | 2 +-
 diff.c                   | 4 ++--
 submodule.c              | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8b2b259ff0d..289a7d2bc9e 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -268,7 +268,7 @@ static int bisect_write(const char *state, const char *rev,
 	} else if (one_of(state, terms->term_good, "skip", NULL)) {
 		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
 	} else {
-		res = error(_("Bad bisect_write argument: %s"), state);
+		res = error(_("bad %s argument: %s"), "bisect_write", state);
 		goto finish;
 	}
 
diff --git a/diff.c b/diff.c
index 6b22946cd0e..5e045b4ba87 100644
--- a/diff.c
+++ b/diff.c
@@ -5010,7 +5010,7 @@ static int diff_opt_color_moved(const struct option *opt,
 	} else {
 		int cm = parse_color_moved(arg);
 		if (cm < 0)
-			return error(_("bad --color-moved argument: %s"), arg);
+			return error(_("bad %s argument: %s"), "--color-moved", arg);
 		options->color_moved = cm;
 	}
 	return 0;
@@ -5334,7 +5334,7 @@ static int diff_opt_word_diff(const struct option *opt,
 		else if (!strcmp(arg, "none"))
 			options->word_diff = DIFF_WORDS_NONE;
 		else
-			return error(_("bad --word-diff argument: %s"), arg);
+			return error(_("bad %s argument: %s"), "--word-diff", arg);
 	} else {
 		if (options->word_diff == DIFF_WORDS_NONE)
 			options->word_diff = DIFF_WORDS_PLAIN;
diff --git a/submodule.c b/submodule.c
index 5ace18a7d94..010f8252c5e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -470,7 +470,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
 	else if (!strcmp(arg, "dirty"))
 		diffopt->flags.ignore_dirty_submodules = 1;
 	else if (strcmp(arg, "none"))
-		die(_("bad --ignore-submodules argument: %s"), arg);
+		die(_("bad %s argument: %s"), "--ignore-submodules", arg);
 	/*
 	 * Please update _git_status() in git-completion.bash when you
 	 * add new options
-- 
gitgitgadget

[PATCH 4/7] i18n: factorize "Server does not support foo" messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 fetch-pack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 09b1cf6173a..fcbc5cc5944 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1106,16 +1106,16 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 		print_verbose(args, _("Server supports %s"), "deepen-since");
 		deepen_since_ok = 1;
 	} else if (args->deepen_since)
-		die(_("Server does not support --shallow-since"));
+		die(_("Server does not support %s"), "--shallow-since");
 	if (server_supports("deepen-not")) {
 		print_verbose(args, _("Server supports %s"), "deepen-not");
 		deepen_not_ok = 1;
 	} else if (args->deepen_not)
-		die(_("Server does not support --shallow-exclude"));
+		die(_("Server does not support %s"), "--shallow-exclude");
 	if (server_supports("deepen-relative"))
 		print_verbose(args, _("Server supports %s"), "deepen-relative");
 	else if (args->deepen_relative)
-		die(_("Server does not support --deepen"));
+		die(_("Server does not support %s"), "--deepen");
 	if (!server_supports_hash(the_hash_algo->name, NULL))
 		die(_("Server does not support this repository's object format"));
 
-- 
gitgitgadget

[PATCH 5/7] i18n: factorize "foo does not take arguments" messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 contrib/scalar/scalar.c | 2 +-
 ref-filter.c            | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
index 7db2a97416e..1aeecea1daa 100644
--- a/contrib/scalar/scalar.c
+++ b/contrib/scalar/scalar.c
@@ -502,7 +502,7 @@ cleanup:
 static int cmd_list(int argc, const char **argv)
 {
 	if (argc != 1)
-		die(_("`scalar list` does not take arguments"));
+		die(_("%s does not take arguments"), "`scalar list`");
 
 	if (run_git("config", "--global", "--get-all", "scalar.repo", NULL) < 0)
 		return -1;
diff --git a/ref-filter.c b/ref-filter.c
index 7838bd22b8d..74f84cc0ed7 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -317,7 +317,7 @@ static int objecttype_atom_parser(struct ref_format *format, struct used_atom *a
 				  const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(objecttype) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(objecttype)");
 	if (*atom->name == '*')
 		oi_deref.info.typep = &oi_deref.type;
 	else
@@ -349,7 +349,7 @@ static int deltabase_atom_parser(struct ref_format *format, struct used_atom *at
 				 const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(deltabase)");
 	if (*atom->name == '*')
 		oi_deref.info.delta_base_oid = &oi_deref.delta_base_oid;
 	else
@@ -361,7 +361,7 @@ static int body_atom_parser(struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(body) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(body)");
 	atom->u.contents.option = C_BODY_DEP;
 	return 0;
 }
@@ -565,7 +565,7 @@ static int rest_atom_parser(struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(rest) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(rest)");
 	format->use_rest = 1;
 	return 0;
 }
-- 
gitgitgadget

[PATCH 6/7] i18n: factorize read-cache error messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 read-cache.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 1ad56d02e1d..39a633ea4b8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1735,8 +1735,8 @@ static unsigned int get_index_format_default(struct repository *r)
 		if (r->settings.index_version >= 0)
 			version = r->settings.index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
-			warning(_("index.version set, but the value is invalid.\n"
-				  "Using version %i"), INDEX_FORMAT_DEFAULT);
+			warning(_("%s set, but the value is invalid.\n"
+				  "Using version %i"), "index.version", INDEX_FORMAT_DEFAULT);
 			return INDEX_FORMAT_DEFAULT;
 		}
 		return version;
@@ -1745,8 +1745,8 @@ static unsigned int get_index_format_default(struct repository *r)
 	version = strtoul(envversion, &endp, 10);
 	if (*endp ||
 	    version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
-		warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
-			  "Using version %i"), INDEX_FORMAT_DEFAULT);
+		warning(_("%s set, but the value is invalid.\n"
+			  "Using version %i"), "GIT_INDEX_VERSION", INDEX_FORMAT_DEFAULT);
 		version = INDEX_FORMAT_DEFAULT;
 	}
 	return version;
-- 
gitgitgadget

[PATCH 7/7] i18n: factorize unrecognized options arguments messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 builtin/commit-graph.c | 2 +-
 builtin/config.c       | 2 +-
 builtin/env--helper.c  | 2 +-
 builtin/gc.c           | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 51c4040ea6c..1a4c798f908 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -134,7 +134,7 @@ static int write_option_parse_split(const struct option *opt, const char *arg,
 	else if (!strcmp(arg, "replace"))
 		*flags = COMMIT_GRAPH_SPLIT_REPLACE;
 	else
-		die(_("unrecognized --split argument, %s"), arg);
+		die(_("unrecognized %s argument '%s'"), "--split", arg);
 
 	return 0;
 }
diff --git a/builtin/config.c b/builtin/config.c
index e7b88a9c08d..aaf87761e58 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -106,7 +106,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
 		else if (!strcmp(arg, "color"))
 			new_type = TYPE_COLOR;
 		else
-			die(_("unrecognized --type argument, %s"), arg);
+			die(_("unrecognized %s argument '%s'"), "--type", arg);
 	}
 
 	to_type = opt->value;
diff --git a/builtin/env--helper.c b/builtin/env--helper.c
index 27349098b07..e58e0de9a7b 100644
--- a/builtin/env--helper.c
+++ b/builtin/env--helper.c
@@ -24,7 +24,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
 	else if (!strcmp(arg, "ulong"))
 		*cmdmode = ENV_HELPER_TYPE_ULONG;
 	else
-		die(_("unrecognized --type argument, %s"), arg);
+		die(_("unrecognized %s argument '%s'"), "--type", arg);
 
 	return 0;
 }
diff --git a/builtin/gc.c b/builtin/gc.c
index d512c127b99..1a9ee85a4d3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -750,7 +750,7 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,
 	*priority = parse_schedule(arg);
 
 	if (!*priority)
-		die(_("unrecognized --schedule argument '%s'"), arg);
+		die(_("unrecognized %s argument '%s'"), "--schedule", arg);
 
 	return 0;
 }
@@ -2393,7 +2393,7 @@ static int maintenance_opt_scheduler(const struct option *opt, const char *arg,
 
 	*scheduler = parse_scheduler(arg);
 	if (*scheduler == SCHEDULER_INVALID)
-		return error(_("unrecognized --scheduler argument '%s'"), arg);
+		return error(_("unrecognized %s argument '%s'"), "--scheduler", arg);
 	return 0;
 }
 
-- 
gitgitgadget

Re: [PATCH 0/7] More i18n fixes

[Johannes Sixt]

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.

-- Hannes

Re: [PATCH 0/7] More i18n fixes

[Ævar Arnfjörð Bjarmason]

On Mon, Mar 21 2022, Johannes Sixt wrote:

> 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`");

Re: [PATCH 0/7] More i18n fixes

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> 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.

Hmph, if we gather wisdom from folks with different native languages
and varying rules, I am unsure that we still can say that "careful"
use of sentence lego is OK.

Re: [PATCH 0/7] More i18n fixes

[Jean-Noël AVILA]

On Monday, 21 March 2022 20:03:46 CET Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> >> 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.
> 
> Hmph, if we gather wisdom from folks with different native languages
> and varying rules, I am unsure that we still can say that "careful"
> use of sentence lego is OK.
> 

To me, this is not sentence lego because:
 * the placeholders are replacing constant strings that won't have declensions 
or grammatical agreements
 * the placeholders are linked to content where the variables have the same 
type, which makes them have the same genre for each language. This is the case 
for commands, options, env variables,...

Given these prerequisites, there's nothing that needs careful lego, because 
there is no risk at all. The only needed refinement, which I haven't addressed 
in this series,  is making it clear for the translator what the placeholder is 
referring to in order to get the wording correct.

Re: [PATCH 0/7] More i18n fixes

[Jean-Noël AVILA]

On Monday, 21 March 2022 14:59:24 CET Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Mar 21 2022, Johannes Sixt wrote:
> 
> > 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")

OK. 

> 
>  - 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.
> 

That's part of the workflow, although not hand-made, but greped. There are two types of factorizations:
* strings where introducing a placeholder creates duplicates
* strings that basically mean the same, but expressed differently (require human parsing)

>  - 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`");
> 
> 

OK for disambiguation, lowercasing and quoting all placeholders.
BTW, disambiguation may have the side effect of revealing where the type of placeholders may not be matching the types of replaced variables.


Will reroll.

Thanks.

[PATCH v2 0/6] More i18n fixes

[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.

Changes in v2:

 * Make the messages clearer as to what the placeholders are.
 * Generalize the quoting of placeholders
 * Merge the messages with bad arguments to one

Bagas Sanjaya (1):
  sequencer: factor GIT_AUTHOR_* from message strings

Jean-Noël Avila (5):
  i18n: factorize generic failure messages
  i18n: factorize server support messages in fetch-pack
  i18n: factorize "foo does not take arguments" messages
  i18n: factorize read-cache error messages
  i18n: factorize "bad argument" messages

 add-patch.c                     |  4 ++--
 builtin/am.c                    |  2 +-
 builtin/bisect--helper.c        |  2 +-
 builtin/commit-graph.c          |  2 +-
 builtin/config.c                |  2 +-
 builtin/env--helper.c           |  2 +-
 builtin/gc.c                    |  8 +++----
 builtin/merge.c                 |  6 ++---
 builtin/revert.c                |  4 ++--
 contrib/scalar/scalar.c         |  2 +-
 diff.c                          |  4 ++--
 fetch-pack.c                    | 40 ++++++++++++++++-----------------
 grep.c                          |  2 +-
 read-cache.c                    |  8 +++----
 ref-filter.c                    |  8 +++----
 remote-curl.c                   |  2 +-
 send-pack.c                     |  2 +-
 sequencer.c                     | 12 +++++-----
 setup.c                         |  4 ++--
 submodule-config.c              |  8 +++----
 submodule.c                     |  2 +-
 t/t1300-config.sh               |  2 +-
 t/t1600-index.sh                |  6 ++---
 t/t3510-cherry-pick-sequence.sh |  8 +++----
 t/t6436-merge-overwrite.sh      |  2 +-
 t/t7900-maintenance.sh          |  4 ++--
 26 files changed, 74 insertions(+), 74 deletions(-)


base-commit: 74cc1aa55f30ed76424a0e7226ab519aa6265061
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1181%2Fjnavila%2Fmore_i18n_fixes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1181/jnavila/more_i18n_fixes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1181

Range-diff vs v1:

 1:  73a3a8d7d97 ! 1:  4bba3e1f6cb i18n: factorize generic failure messages
     @@ Metadata
       ## Commit message ##
          i18n: factorize generic failure messages
      
     -    In these message the command that failed should not be translated. So
     -    it is simpler to just remove these parts from the message.
     +    In these message the command or the function that failed should not be
     +    translated. So it is simpler to just remove these parts from the
     +    message.
      
          Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
      
     @@ add-patch.c: static int run_apply_check(struct add_p_state *s,
       	strvec_pushv(&cp.args, s->mode->apply_check_args);
       	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
      -		return error(_("'git apply --cached' failed"));
     -+		return error(_("'%s' failed"), "git apply --cached");
     ++		return error(_("the command '%s' failed"), "git apply --cached");
       
       	return 0;
       }
     @@ add-patch.c: soft_increment:
       			if (pipe_command(&cp, s->buf.buf, s->buf.len,
       					 NULL, 0, NULL, 0))
      -				error(_("'git apply' failed"));
     -+				error(_("'%s' failed"), "git apply");
     ++				error(_("the command '%s' failed"), "git apply");
       		}
       		if (repo_read_index(s->s.r) >= 0)
       			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
     @@ builtin/am.c: static int is_mail(FILE *fp)
       
       	if (fseek(fp, 0L, SEEK_SET))
      -		die_errno(_("fseek failed"));
     -+		die_errno(_("'%s' failed"), "fseek");
     ++		die_errno(_("the function '%s' failed"), "fseek");
       
       	if (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))
       		die("invalid pattern: %s", header_regex);
     @@ builtin/gc.c: static int multi_pack_index_expire(struct maintenance_run_opts *op
       
       	if (run_command(&child))
      -		return error(_("'git multi-pack-index expire' failed"));
     -+		return error(_("'%s' failed"), "git multi-pack-index expire");
     ++		return error(_("the command '%s' failed"), "git multi-pack-index expire");
       
       	return 0;
       }
     @@ builtin/gc.c: static int multi_pack_index_repack(struct maintenance_run_opts *op
       
       	if (run_command(&child))
      -		return error(_("'git multi-pack-index repack' failed"));
     -+		return error(_("'%s' failed"), "git multi-pack-index repack");
     ++		return error(_("the command '%s' failed"), "git multi-pack-index repack");
       
       	return 0;
       }
     @@ builtin/merge.c: static int save_state(struct object_id *stash)
       
       	if (finish_command(&cp) || len < 0)
      -		die(_("stash failed"));
     -+		die(_("'%s' failed"), "stash");
     ++		die(_("the command '%s' failed"), "stash");
       	else if (!len)		/* no changes */
       		goto out;
       	strbuf_setlen(&buffer, buffer.len-1);
     @@ builtin/merge.c: static void read_empty(const struct object_id *oid, int verbose
       
       	if (run_command_v_opt(args, RUN_GIT_CMD))
      -		die(_("read-tree failed"));
     -+		die(_("'%s' failed"), "read-tree");
     ++		die(_("the command '%s' failed"), "read-tree");
       }
       
       static void reset_hard(const struct object_id *oid, int verbose)
     @@ builtin/merge.c: static void reset_hard(const struct object_id *oid, int verbose
       
       	if (run_command_v_opt(args, RUN_GIT_CMD))
      -		die(_("read-tree failed"));
     -+		die(_("'%s' failed"), "read-tree");
     ++		die(_("the command '%s' failed"), "read-tree");
       }
       
       static void restore_state(const struct object_id *head,
     @@ builtin/revert.c: int cmd_revert(int argc, const char **argv, const char *prefix
       	res = run_sequencer(argc, argv, &opts);
       	if (res < 0)
      -		die(_("revert failed"));
     -+		die(_("'%s' failed"), "revert");
     ++		die(_("the command '%s' failed"), "revert");
       	return res;
       }
       
     @@ builtin/revert.c: int cmd_cherry_pick(int argc, const char **argv, const char *p
       	res = run_sequencer(argc, argv, &opts);
       	if (res < 0)
      -		die(_("cherry-pick failed"));
     -+		die(_("'%s' failed"), "cherry-pick");
     ++		die(_("the command '%s' failed"), "cherry-pick");
       	return res;
       }
      
     @@ fetch-pack.c: static int get_pack(struct fetch_pack_args *args,
       			ret == 0;
       	else
      -		die(_("%s failed"), cmd_name);
     -+		die(_("'%s' failed"), cmd_name);
     ++		die(_("the command '%s' failed"), cmd_name);
       	if (use_sideband && finish_async(&demux))
       		die(_("error in sideband demultiplexer"));
       
     @@ remote-curl.c: static int push_dav(int nr_spec, const char **specs)
       
       	if (run_command(&child))
      -		die(_("git-http-push failed"));
     -+		die(_("'%s' failed"), "git-http-push");
     ++		die(_("the command '%s' failed"), "git-http-push");
       	return 0;
       }
       
     @@ setup.c: int daemonize(void)
       			break;
       		case -1:
      -			die_errno(_("fork failed"));
     -+			die_errno(_("'%s' failed"), "fork");
     ++			die_errno(_("the function '%s' failed"), "fork");
       		default:
       			exit(0);
       	}
       	if (setsid() == -1)
      -		die_errno(_("setsid failed"));
     -+		die_errno(_("'%s' failed"), "setsid");
     ++		die_errno(_("the function '%s' failed"), "setsid");
       	close(0);
       	close(1);
       	close(2);
     @@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'check advice when we move
       	hint: have you committed already?
       	hint: try "git cherry-pick --continue"
      -	fatal: cherry-pick failed
     -+	fatal: '\''cherry-pick'\'' failed
     ++	fatal: the command '\''cherry-pick'\'' failed
       	EOF
       	test_must_fail git cherry-pick base..yetanotherpick &&
       	echo c >foo &&
     @@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'selectively advise --skip
       	error: cherry-pick is already in progress
       	hint: try "git cherry-pick (--continue | --skip | --abort | --quit)"
      -	fatal: cherry-pick failed
     -+	fatal: '\''cherry-pick'\'' failed
     ++	fatal: the command '\''cherry-pick'\'' failed
       	EOF
       	test_must_fail git cherry-pick picked..yetanotherpick &&
       	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
     @@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'selectively advise --skip
       	error: cherry-pick is already in progress
       	hint: try "git cherry-pick (--continue | --abort | --quit)"
      -	fatal: cherry-pick failed
     -+	fatal: '\''cherry-pick'\'' failed
     ++	fatal: the command '\''cherry-pick'\'' failed
       	EOF
       	git reset --merge &&
       	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
     @@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'allow skipping commit but
       	cat >expect <<-EOF &&
       	error: cannot abort from a branch yet to be born
      -	fatal: cherry-pick failed
     -+	fatal: '\''cherry-pick'\'' failed
     ++	fatal: the command '\''cherry-pick'\'' failed
       	EOF
       	git checkout --orphan new_disconnected &&
       	git reset --hard &&
     @@ t/t6436-merge-overwrite.sh: test_expect_success 'will not be confused by symlink
       cat >expect <<\EOF
       error: Untracked working tree file 'c0.c' would be overwritten by merge.
      -fatal: read-tree failed
     -+fatal: 'read-tree' failed
     ++fatal: the command 'read-tree' failed
       EOF
       
       test_expect_success 'will not overwrite untracked file on unborn branch' '
 2:  8bdbb32f544 = 2:  03022a2fcd1 sequencer: factor GIT_AUTHOR_* from message strings
 3:  c94d5c932f4 < -:  ----------- i18n: factorize "bad argument" messages
 4:  bf23e1ebe65 ! 3:  b06b7670516 i18n: factorize "Server does not support foo" messages
     @@ Metadata
      Author: Jean-Noël Avila <jn.avila@free.fr>
      
       ## Commit message ##
     -    i18n: factorize "Server does not support foo" messages
     +    i18n: factorize server support messages in fetch-pack
     +
     +    The messages "Server supports foo" and "Server does not support bar"
     +    are uniformized and options are quoted.
      
          Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
      
       ## fetch-pack.c ##
      @@ fetch-pack.c: static struct ref *do_fetch_pack(struct fetch_pack_args *args,
     - 		print_verbose(args, _("Server supports %s"), "deepen-since");
     + 		advertise_sid = 0;
     + 
     + 	if (server_supports("shallow"))
     +-		print_verbose(args, _("Server supports %s"), "shallow");
     ++		print_verbose(args, _("Server supports '%s'"), "shallow");
     + 	else if (args->depth > 0 || is_repository_shallow(r))
     + 		die(_("Server does not support shallow clients"));
     + 	if (args->depth > 0 || args->deepen_since || args->deepen_not)
     + 		args->deepen = 1;
     + 	if (server_supports("multi_ack_detailed")) {
     +-		print_verbose(args, _("Server supports %s"), "multi_ack_detailed");
     ++		print_verbose(args, _("Server supports '%s'"), "multi_ack_detailed");
     + 		multi_ack = 2;
     + 		if (server_supports("no-done")) {
     +-			print_verbose(args, _("Server supports %s"), "no-done");
     ++			print_verbose(args, _("Server supports '%s'"), "no-done");
     + 			if (args->stateless_rpc)
     + 				no_done = 1;
     + 		}
     + 	}
     + 	else if (server_supports("multi_ack")) {
     +-		print_verbose(args, _("Server supports %s"), "multi_ack");
     ++		print_verbose(args, _("Server supports '%s'"), "multi_ack");
     + 		multi_ack = 1;
     + 	}
     + 	if (server_supports("side-band-64k")) {
     +-		print_verbose(args, _("Server supports %s"), "side-band-64k");
     ++		print_verbose(args, _("Server supports '%s'"), "side-band-64k");
     + 		use_sideband = 2;
     + 	}
     + 	else if (server_supports("side-band")) {
     +-		print_verbose(args, _("Server supports %s"), "side-band");
     ++		print_verbose(args, _("Server supports '%s'"), "side-band");
     + 		use_sideband = 1;
     + 	}
     + 	if (server_supports("allow-tip-sha1-in-want")) {
     +-		print_verbose(args, _("Server supports %s"), "allow-tip-sha1-in-want");
     ++		print_verbose(args, _("Server supports '%s'"), "allow-tip-sha1-in-want");
     + 		allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
     + 	}
     + 	if (server_supports("allow-reachable-sha1-in-want")) {
     +-		print_verbose(args, _("Server supports %s"), "allow-reachable-sha1-in-want");
     ++		print_verbose(args, _("Server supports '%s'"), "allow-reachable-sha1-in-want");
     + 		allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
     + 	}
     + 	if (server_supports("thin-pack"))
     +-		print_verbose(args, _("Server supports %s"), "thin-pack");
     ++		print_verbose(args, _("Server supports '%s'"), "thin-pack");
     + 	else
     + 		args->use_thin_pack = 0;
     + 	if (server_supports("no-progress"))
     +-		print_verbose(args, _("Server supports %s"), "no-progress");
     ++		print_verbose(args, _("Server supports '%s'"), "no-progress");
     + 	else
     + 		args->no_progress = 0;
     + 	if (server_supports("include-tag"))
     +-		print_verbose(args, _("Server supports %s"), "include-tag");
     ++		print_verbose(args, _("Server supports '%s'"), "include-tag");
     + 	else
     + 		args->include_tag = 0;
     + 	if (server_supports("ofs-delta"))
     +-		print_verbose(args, _("Server supports %s"), "ofs-delta");
     ++		print_verbose(args, _("Server supports '%s'"), "ofs-delta");
     + 	else
     + 		prefer_ofs_delta = 0;
     + 
     + 	if (server_supports("filter")) {
     + 		server_supports_filtering = 1;
     +-		print_verbose(args, _("Server supports %s"), "filter");
     ++		print_verbose(args, _("Server supports '%s'"), "filter");
     + 	} else if (args->filter_options.choice) {
     + 		warning("filtering not recognized by server, ignoring");
     + 	}
     + 
     + 	if (server_supports("deepen-since")) {
     +-		print_verbose(args, _("Server supports %s"), "deepen-since");
     ++		print_verbose(args, _("Server supports '%s'"), "deepen-since");
       		deepen_since_ok = 1;
       	} else if (args->deepen_since)
      -		die(_("Server does not support --shallow-since"));
     -+		die(_("Server does not support %s"), "--shallow-since");
     ++		die(_("Server does not support '%s'"), "--shallow-since");
       	if (server_supports("deepen-not")) {
     - 		print_verbose(args, _("Server supports %s"), "deepen-not");
     +-		print_verbose(args, _("Server supports %s"), "deepen-not");
     ++		print_verbose(args, _("Server supports '%s'"), "deepen-not");
       		deepen_not_ok = 1;
       	} else if (args->deepen_not)
      -		die(_("Server does not support --shallow-exclude"));
     -+		die(_("Server does not support %s"), "--shallow-exclude");
     ++		die(_("Server does not support '%s'"), "--shallow-exclude");
       	if (server_supports("deepen-relative"))
     - 		print_verbose(args, _("Server supports %s"), "deepen-relative");
     +-		print_verbose(args, _("Server supports %s"), "deepen-relative");
     ++		print_verbose(args, _("Server supports '%s'"), "deepen-relative");
       	else if (args->deepen_relative)
      -		die(_("Server does not support --deepen"));
     -+		die(_("Server does not support %s"), "--deepen");
     ++		die(_("Server does not support '%s'"), "--deepen");
       	if (!server_supports_hash(the_hash_algo->name, NULL))
       		die(_("Server does not support this repository's object format"));
       
 5:  2e1bd504673 ! 4:  6221c37145c i18n: factorize "foo does not take arguments" messages
     @@ Metadata
       ## Commit message ##
          i18n: factorize "foo does not take arguments" messages
      
     +    The messages are split into the ones for ref-filter which deal with
     +    atoms and scalar which has an option.
     +
          Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
      
       ## contrib/scalar/scalar.c ##
     @@ contrib/scalar/scalar.c: cleanup:
       {
       	if (argc != 1)
      -		die(_("`scalar list` does not take arguments"));
     -+		die(_("%s does not take arguments"), "`scalar list`");
     ++		die(_("the '%s' command does not take arguments"), "scalar list");
       
       	if (run_git("config", "--global", "--get-all", "scalar.repo", NULL) < 0)
       		return -1;
     @@ ref-filter.c: static int objecttype_atom_parser(struct ref_format *format, struc
       {
       	if (arg)
      -		return strbuf_addf_ret(err, -1, _("%%(objecttype) does not take arguments"));
     -+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(objecttype)");
     ++		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(objecttype)");
       	if (*atom->name == '*')
       		oi_deref.info.typep = &oi_deref.type;
       	else
     @@ ref-filter.c: static int deltabase_atom_parser(struct ref_format *format, struct
       {
       	if (arg)
      -		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
     -+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(deltabase)");
     ++		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(deltabase)");
       	if (*atom->name == '*')
       		oi_deref.info.delta_base_oid = &oi_deref.delta_base_oid;
       	else
     @@ ref-filter.c: static int body_atom_parser(struct ref_format *format, struct used
       {
       	if (arg)
      -		return strbuf_addf_ret(err, -1, _("%%(body) does not take arguments"));
     -+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(body)");
     ++		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(body)");
       	atom->u.contents.option = C_BODY_DEP;
       	return 0;
       }
     @@ ref-filter.c: static int rest_atom_parser(struct ref_format *format, struct used
       {
       	if (arg)
      -		return strbuf_addf_ret(err, -1, _("%%(rest) does not take arguments"));
     -+		return strbuf_addf_ret(err, -1, _("%s does not take arguments"), "%(rest)");
     ++		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(rest)");
       	format->use_rest = 1;
       	return 0;
       }
 6:  b8f97e6fde8 ! 5:  1888778902a i18n: factorize read-cache error messages
     @@ read-cache.c: static unsigned int get_index_format_default(struct repository *r)
       		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
      -			warning(_("index.version set, but the value is invalid.\n"
      -				  "Using version %i"), INDEX_FORMAT_DEFAULT);
     -+			warning(_("%s set, but the value is invalid.\n"
     ++			warning(_("'%s' set, but the value is invalid.\n"
      +				  "Using version %i"), "index.version", INDEX_FORMAT_DEFAULT);
       			return INDEX_FORMAT_DEFAULT;
       		}
     @@ read-cache.c: static unsigned int get_index_format_default(struct repository *r)
       	    version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
      -		warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
      -			  "Using version %i"), INDEX_FORMAT_DEFAULT);
     -+		warning(_("%s set, but the value is invalid.\n"
     ++		warning(_("'%s' set, but the value is invalid.\n"
      +			  "Using version %i"), "GIT_INDEX_VERSION", INDEX_FORMAT_DEFAULT);
       		version = INDEX_FORMAT_DEFAULT;
       	}
       	return version;
     +
     + ## t/t1600-index.sh ##
     +@@ t/t1600-index.sh: test_expect_success 'bogus GIT_INDEX_VERSION issues warning' '
     + 		git add a 2>err &&
     + 		sed "s/[0-9]//" err >actual.err &&
     + 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
     +-			warning: GIT_INDEX_VERSION set, but the value is invalid.
     ++			warning: '\''GIT_INDEX_VERSION'\'' set, but the value is invalid.
     + 			Using version Z
     + 		EOF
     + 		test_cmp expect.err actual.err
     +@@ t/t1600-index.sh: test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
     + 		git add a 2>err &&
     + 		sed "s/[0-9]//" err >actual.err &&
     + 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
     +-			warning: GIT_INDEX_VERSION set, but the value is invalid.
     ++			warning: '\''GIT_INDEX_VERSION'\'' set, but the value is invalid.
     + 			Using version Z
     + 		EOF
     + 		test_cmp expect.err actual.err
     +@@ t/t1600-index.sh: test_expect_success 'out of bounds index.version issues warning' '
     + 		git add a 2>err &&
     + 		sed "s/[0-9]//" err >actual.err &&
     + 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
     +-			warning: index.version set, but the value is invalid.
     ++			warning: '\''index.version'\'' set, but the value is invalid.
     + 			Using version Z
     + 		EOF
     + 		test_cmp expect.err actual.err
 7:  ca52567b201 ! 6:  febe64a8a06 i18n: factorize unrecognized options arguments messages
     @@ Metadata
      Author: Jean-Noël Avila <jn.avila@free.fr>
      
       ## Commit message ##
     -    i18n: factorize unrecognized options arguments messages
     +    i18n: factorize "bad argument" messages
     +
     +    We also quote the placeholders as they replace constant strings.
      
          Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
      
     + ## builtin/bisect--helper.c ##
     +@@ builtin/bisect--helper.c: static int bisect_write(const char *state, const char *rev,
     + 	} else if (one_of(state, terms->term_good, "skip", NULL)) {
     + 		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
     + 	} else {
     +-		res = error(_("Bad bisect_write argument: %s"), state);
     ++		res = error(_("bad '%s' argument: '%s'"), "bisect_write", state);
     + 		goto finish;
     + 	}
     + 
     +
       ## builtin/commit-graph.c ##
      @@ builtin/commit-graph.c: static int write_option_parse_split(const struct option *opt, const char *arg,
       	else if (!strcmp(arg, "replace"))
       		*flags = COMMIT_GRAPH_SPLIT_REPLACE;
       	else
      -		die(_("unrecognized --split argument, %s"), arg);
     -+		die(_("unrecognized %s argument '%s'"), "--split", arg);
     ++		die(_("bad '%s' argument: '%s'"), "--split", arg);
       
       	return 0;
       }
     @@ builtin/config.c: static int option_parse_type(const struct option *opt, const c
       			new_type = TYPE_COLOR;
       		else
      -			die(_("unrecognized --type argument, %s"), arg);
     -+			die(_("unrecognized %s argument '%s'"), "--type", arg);
     ++			die(_("bad '%s' argument: '%s'"), "--type", arg);
       	}
       
       	to_type = opt->value;
     @@ builtin/env--helper.c: static int option_parse_type(const struct option *opt, co
       		*cmdmode = ENV_HELPER_TYPE_ULONG;
       	else
      -		die(_("unrecognized --type argument, %s"), arg);
     -+		die(_("unrecognized %s argument '%s'"), "--type", arg);
     ++		die(_("bad '%s' argument: '%s'"), "--type", arg);
       
       	return 0;
       }
     @@ builtin/gc.c: static int maintenance_opt_schedule(const struct option *opt, cons
       
       	if (!*priority)
      -		die(_("unrecognized --schedule argument '%s'"), arg);
     -+		die(_("unrecognized %s argument '%s'"), "--schedule", arg);
     ++		die(_("bad '%s' argument: '%s'"), "--schedule", arg);
       
       	return 0;
       }
     @@ builtin/gc.c: static int maintenance_opt_scheduler(const struct option *opt, con
       	*scheduler = parse_scheduler(arg);
       	if (*scheduler == SCHEDULER_INVALID)
      -		return error(_("unrecognized --scheduler argument '%s'"), arg);
     -+		return error(_("unrecognized %s argument '%s'"), "--scheduler", arg);
     ++		return error(_("bad '%s' argument: '%s'"), "--scheduler", arg);
     + 	return 0;
     + }
     + 
     +
     + ## diff.c ##
     +@@ diff.c: static int diff_opt_color_moved(const struct option *opt,
     + 	} else {
     + 		int cm = parse_color_moved(arg);
     + 		if (cm < 0)
     +-			return error(_("bad --color-moved argument: %s"), arg);
     ++			return error(_("bad '%s' argument: '%s'"), "--color-moved", arg);
     + 		options->color_moved = cm;
     + 	}
       	return 0;
     +@@ diff.c: static int diff_opt_word_diff(const struct option *opt,
     + 		else if (!strcmp(arg, "none"))
     + 			options->word_diff = DIFF_WORDS_NONE;
     + 		else
     +-			return error(_("bad --word-diff argument: %s"), arg);
     ++			return error(_("bad '%s' argument: '%s'"), "--word-diff", arg);
     + 	} else {
     + 		if (options->word_diff == DIFF_WORDS_NONE)
     + 			options->word_diff = DIFF_WORDS_PLAIN;
     +
     + ## grep.c ##
     +@@ grep.c: static int parse_pattern_type_arg(const char *opt, const char *arg)
     + 		return GREP_PATTERN_TYPE_FIXED;
     + 	else if (!strcmp(arg, "perl"))
     + 		return GREP_PATTERN_TYPE_PCRE;
     +-	die("bad %s argument: %s", opt, arg);
     ++	die("bad '%s' argument: '%s'", opt, arg);
     + }
     + 
     + define_list_config_array_extra(color_grep_slots, {"match"});
     +
     + ## send-pack.c ##
     +@@ send-pack.c: int option_parse_push_signed(const struct option *opt,
     + 		*(int *)(opt->value) = SEND_PACK_PUSH_CERT_IF_ASKED;
     + 		return 0;
     + 	}
     +-	die("bad %s argument: %s", opt->long_name, arg);
     ++	die("bad '%s' argument: '%s'", opt->long_name, arg);
     + }
     + 
     + static void feed_object(const struct object_id *oid, FILE *fh, int negative)
     +
     + ## submodule-config.c ##
     +@@ submodule-config.c: static int parse_fetch_recurse(const char *opt, const char *arg,
     + 		 * git-completion.bash when you add new options.
     + 		 */
     + 		if (die_on_error)
     +-			die("bad %s argument: %s", opt, arg);
     ++			die("bad '%s' argument: '%s'", opt, arg);
     + 		else
     + 			return RECURSE_SUBMODULES_ERROR;
     + 	}
     +@@ submodule-config.c: static int parse_update_recurse(const char *opt, const char *arg,
     + 		return RECURSE_SUBMODULES_OFF;
     + 	default:
     + 		if (die_on_error)
     +-			die("bad %s argument: %s", opt, arg);
     ++			die("bad '%s' argument: '%s'", opt, arg);
     + 		return RECURSE_SUBMODULES_ERROR;
     + 	}
       }
     +@@ submodule-config.c: static int parse_push_recurse(const char *opt, const char *arg,
     + 	case 1:
     + 		/* There's no simple "on" value when pushing */
     + 		if (die_on_error)
     +-			die("bad %s argument: %s", opt, arg);
     ++			die("bad '%s' argument: '%s'", opt, arg);
     + 		else
     + 			return RECURSE_SUBMODULES_ERROR;
     + 	case 0:
     +@@ submodule-config.c: static int parse_push_recurse(const char *opt, const char *arg,
     + 		 * git-completion.bash when you add new modes.
     + 		 */
     + 		else if (die_on_error)
     +-			die("bad %s argument: %s", opt, arg);
     ++			die("bad '%s' argument: '%s'", opt, arg);
     + 		else
     + 			return RECURSE_SUBMODULES_ERROR;
     + 	}
     +
     + ## submodule.c ##
     +@@ submodule.c: void handle_ignore_submodules_arg(struct diff_options *diffopt,
     + 	else if (!strcmp(arg, "dirty"))
     + 		diffopt->flags.ignore_dirty_submodules = 1;
     + 	else if (strcmp(arg, "none"))
     +-		die(_("bad --ignore-submodules argument: %s"), arg);
     ++		die(_("bad '%s' argument: '%s'"), "--ignore-submodules", arg);
     + 	/*
     + 	 * Please update _git_status() in git-completion.bash when you
     + 	 * add new options
     +
     + ## t/t1300-config.sh ##
     +@@ t/t1300-config.sh: test_expect_success 'unset type specifiers may be reset to conflicting ones' '
     + 
     + test_expect_success '--type rejects unknown specifiers' '
     + 	test_must_fail git config --type=nonsense section.foo 2>error &&
     +-	test_i18ngrep "unrecognized --type argument" error
     ++	test_i18ngrep "bad '\''--type'\'' argument" error
     + '
     + 
     + test_expect_success '--replace-all does not invent newlines' '
     +
     + ## t/t7900-maintenance.sh ##
     +@@ t/t7900-maintenance.sh: test_expect_success '--auto and --schedule incompatible' '
     + 
     + test_expect_success 'invalid --schedule value' '
     + 	test_must_fail git maintenance run --schedule=annually 2>err &&
     +-	test_i18ngrep "unrecognized --schedule" err
     ++	test_i18ngrep "bad '\''--schedule'\''" err
     + '
     + 
     + test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
     +@@ t/t7900-maintenance.sh: test_expect_success !MINGW 'register and unregister with regex metacharacters' '
     + 
     + test_expect_success 'start --scheduler=<scheduler>' '
     + 	test_expect_code 129 git maintenance start --scheduler=foo 2>err &&
     +-	test_i18ngrep "unrecognized --scheduler argument" err &&
     ++	test_i18ngrep "bad '\''--scheduler'\'' argument" err &&
       
     + 	test_expect_code 129 git maintenance start --no-scheduler 2>err &&
     + 	test_i18ngrep "unknown option" err &&

-- 
gitgitgadget

[PATCH v2 1/6] i18n: factorize generic failure messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

In these message the command or the function that failed should not be
translated. So it is simpler to just remove these parts from the
message.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 add-patch.c                     | 4 ++--
 builtin/am.c                    | 2 +-
 builtin/gc.c                    | 4 ++--
 builtin/merge.c                 | 6 +++---
 builtin/revert.c                | 4 ++--
 fetch-pack.c                    | 2 +-
 remote-curl.c                   | 2 +-
 setup.c                         | 4 ++--
 t/t3510-cherry-pick-sequence.sh | 8 ++++----
 t/t6436-merge-overwrite.sh      | 2 +-
 10 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 55d719f7845..8c9e81ec78e 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1181,7 +1181,7 @@ static int run_apply_check(struct add_p_state *s,
 			    "apply", "--check", NULL);
 	strvec_pushv(&cp.args, s->mode->apply_check_args);
 	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
-		return error(_("'git apply --cached' failed"));
+		return error(_("the command '%s' failed"), "git apply --cached");
 
 	return 0;
 }
@@ -1683,7 +1683,7 @@ soft_increment:
 			strvec_pushv(&cp.args, s->mode->apply_args);
 			if (pipe_command(&cp, s->buf.buf, s->buf.len,
 					 NULL, 0, NULL, 0))
-				error(_("'git apply' failed"));
+				error(_("the command '%s' failed"), "git apply");
 		}
 		if (repo_read_index(s->s.r) >= 0)
 			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
diff --git a/builtin/am.c b/builtin/am.c
index 0f4111bafa0..a0a57049510 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -586,7 +586,7 @@ static int is_mail(FILE *fp)
 	int ret = 1;
 
 	if (fseek(fp, 0L, SEEK_SET))
-		die_errno(_("fseek failed"));
+		die_errno(_("the function '%s' failed"), "fseek");
 
 	if (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))
 		die("invalid pattern: %s", header_regex);
diff --git a/builtin/gc.c b/builtin/gc.c
index ffaf0daf5d9..c062d7bceeb 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1104,7 +1104,7 @@ static int multi_pack_index_expire(struct maintenance_run_opts *opts)
 		strvec_push(&child.args, "--no-progress");
 
 	if (run_command(&child))
-		return error(_("'git multi-pack-index expire' failed"));
+		return error(_("the command '%s' failed"), "git multi-pack-index expire");
 
 	return 0;
 }
@@ -1163,7 +1163,7 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts)
 				  (uintmax_t)get_auto_pack_size());
 
 	if (run_command(&child))
-		return error(_("'git multi-pack-index repack' failed"));
+		return error(_("the command '%s' failed"), "git multi-pack-index repack");
 
 	return 0;
 }
diff --git a/builtin/merge.c b/builtin/merge.c
index f178f5a3ee1..78468ff43a4 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -325,7 +325,7 @@ static int save_state(struct object_id *stash)
 	close(cp.out);
 
 	if (finish_command(&cp) || len < 0)
-		die(_("stash failed"));
+		die(_("the command '%s' failed"), "stash");
 	else if (!len)		/* no changes */
 		goto out;
 	strbuf_setlen(&buffer, buffer.len-1);
@@ -352,7 +352,7 @@ static void read_empty(const struct object_id *oid, int verbose)
 	args[i] = NULL;
 
 	if (run_command_v_opt(args, RUN_GIT_CMD))
-		die(_("read-tree failed"));
+		die(_("the command '%s' failed"), "read-tree");
 }
 
 static void reset_hard(const struct object_id *oid, int verbose)
@@ -369,7 +369,7 @@ static void reset_hard(const struct object_id *oid, int verbose)
 	args[i] = NULL;
 
 	if (run_command_v_opt(args, RUN_GIT_CMD))
-		die(_("read-tree failed"));
+		die(_("the command '%s' failed"), "read-tree");
 }
 
 static void restore_state(const struct object_id *head,
diff --git a/builtin/revert.c b/builtin/revert.c
index 51776abea63..d293036e790 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -238,7 +238,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	sequencer_init_config(&opts);
 	res = run_sequencer(argc, argv, &opts);
 	if (res < 0)
-		die(_("revert failed"));
+		die(_("the command '%s' failed"), "revert");
 	return res;
 }
 
@@ -251,6 +251,6 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 	sequencer_init_config(&opts);
 	res = run_sequencer(argc, argv, &opts);
 	if (res < 0)
-		die(_("cherry-pick failed"));
+		die(_("the command '%s' failed"), "cherry-pick");
 	return res;
 }
diff --git a/fetch-pack.c b/fetch-pack.c
index 87657907e78..2e6795cd439 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -989,7 +989,7 @@ static int get_pack(struct fetch_pack_args *args,
 			args->check_self_contained_and_connected &&
 			ret == 0;
 	else
-		die(_("%s failed"), cmd_name);
+		die(_("the command '%s' failed"), cmd_name);
 	if (use_sideband && finish_async(&demux))
 		die(_("error in sideband demultiplexer"));
 
diff --git a/remote-curl.c b/remote-curl.c
index ff44f41011e..8393f56652b 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1287,7 +1287,7 @@ static int push_dav(int nr_spec, const char **specs)
 		strvec_push(&child.args, specs[i]);
 
 	if (run_command(&child))
-		die(_("git-http-push failed"));
+		die(_("the command '%s' failed"), "git-http-push");
 	return 0;
 }
 
diff --git a/setup.c b/setup.c
index 04ce33cdcd4..30a4b81257d 100644
--- a/setup.c
+++ b/setup.c
@@ -1506,12 +1506,12 @@ int daemonize(void)
 		case 0:
 			break;
 		case -1:
-			die_errno(_("fork failed"));
+			die_errno(_("the function '%s' failed"), "fork");
 		default:
 			exit(0);
 	}
 	if (setsid() == -1)
-		die_errno(_("setsid failed"));
+		die_errno(_("the function '%s' failed"), "setsid");
 	close(0);
 	close(1);
 	close(2);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3b0fa66c33d..141d217dc3f 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -193,7 +193,7 @@ test_expect_success 'check advice when we move HEAD by committing' '
 	error: there is nothing to skip
 	hint: have you committed already?
 	hint: try "git cherry-pick --continue"
-	fatal: cherry-pick failed
+	fatal: the command '\''cherry-pick'\'' failed
 	EOF
 	test_must_fail git cherry-pick base..yetanotherpick &&
 	echo c >foo &&
@@ -208,7 +208,7 @@ test_expect_success 'selectively advise --skip while launching another sequence'
 	cat >expect <<-EOF &&
 	error: cherry-pick is already in progress
 	hint: try "git cherry-pick (--continue | --skip | --abort | --quit)"
-	fatal: cherry-pick failed
+	fatal: the command '\''cherry-pick'\'' failed
 	EOF
 	test_must_fail git cherry-pick picked..yetanotherpick &&
 	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
@@ -216,7 +216,7 @@ test_expect_success 'selectively advise --skip while launching another sequence'
 	cat >expect <<-EOF &&
 	error: cherry-pick is already in progress
 	hint: try "git cherry-pick (--continue | --abort | --quit)"
-	fatal: cherry-pick failed
+	fatal: the command '\''cherry-pick'\'' failed
 	EOF
 	git reset --merge &&
 	test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
@@ -227,7 +227,7 @@ test_expect_success 'allow skipping commit but not abort for a new history' '
 	pristine_detach initial &&
 	cat >expect <<-EOF &&
 	error: cannot abort from a branch yet to be born
-	fatal: cherry-pick failed
+	fatal: the command '\''cherry-pick'\'' failed
 	EOF
 	git checkout --orphan new_disconnected &&
 	git reset --hard &&
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index c0b7bd7c3fe..c714ac2cbc5 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -166,7 +166,7 @@ test_expect_success 'will not be confused by symlink in leading path' '
 
 cat >expect <<\EOF
 error: Untracked working tree file 'c0.c' would be overwritten by merge.
-fatal: read-tree failed
+fatal: the command 'read-tree' failed
 EOF
 
 test_expect_success 'will not overwrite untracked file on unborn branch' '
-- 
gitgitgadget

Re: [PATCH v2 1/6] i18n: factorize generic failure messages

[Bagas Sanjaya]

On 02/04/22 23.10, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> 
> In these message the command or the function that failed should not be
> translated. So it is simpler to just remove these parts from the
> message.
> 
> Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
> ---
>   add-patch.c                     | 4 ++--
>   builtin/am.c                    | 2 +-
>   builtin/gc.c                    | 4 ++--
>   builtin/merge.c                 | 6 +++---
>   builtin/revert.c                | 4 ++--
>   fetch-pack.c                    | 2 +-
>   remote-curl.c                   | 2 +-
>   setup.c                         | 4 ++--
>   t/t3510-cherry-pick-sequence.sh | 8 ++++----
>   t/t6436-merge-overwrite.sh      | 2 +-
>   10 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/add-patch.c b/add-patch.c
> index 55d719f7845..8c9e81ec78e 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1181,7 +1181,7 @@ static int run_apply_check(struct add_p_state *s,
>   			    "apply", "--check", NULL);
>   	strvec_pushv(&cp.args, s->mode->apply_check_args);
>   	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
> -		return error(_("'git apply --cached' failed"));
> +		return error(_("the command '%s' failed"), "git apply --cached");
>   
>   	return 0;
>   }
> @@ -1683,7 +1683,7 @@ soft_increment:
>   			strvec_pushv(&cp.args, s->mode->apply_args);
>   			if (pipe_command(&cp, s->buf.buf, s->buf.len,
>   					 NULL, 0, NULL, 0))
> -				error(_("'git apply' failed"));
> +				error(_("the command '%s' failed"), "git apply");
>   		}
>   		if (repo_read_index(s->s.r) >= 0)
>   			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
> diff --git a/builtin/am.c b/builtin/am.c
> index 0f4111bafa0..a0a57049510 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -586,7 +586,7 @@ static int is_mail(FILE *fp)
>   	int ret = 1;
>   
>   	if (fseek(fp, 0L, SEEK_SET))
> -		die_errno(_("fseek failed"));
> +		die_errno(_("the function '%s' failed"), "fseek");
>   
>   	if (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))
>   		die("invalid pattern: %s", header_regex);
> diff --git a/builtin/gc.c b/builtin/gc.c
> index ffaf0daf5d9..c062d7bceeb 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1104,7 +1104,7 @@ static int multi_pack_index_expire(struct maintenance_run_opts *opts)
>   		strvec_push(&child.args, "--no-progress");
>   
>   	if (run_command(&child))
> -		return error(_("'git multi-pack-index expire' failed"));
> +		return error(_("the command '%s' failed"), "git multi-pack-index expire");
>   
>   	return 0;
>   }
> @@ -1163,7 +1163,7 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts)
>   				  (uintmax_t)get_auto_pack_size());
>   
>   	if (run_command(&child))
> -		return error(_("'git multi-pack-index repack' failed"));
> +		return error(_("the command '%s' failed"), "git multi-pack-index repack");
>   
>   	return 0;
>   }
> diff --git a/builtin/merge.c b/builtin/merge.c
> index f178f5a3ee1..78468ff43a4 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -325,7 +325,7 @@ static int save_state(struct object_id *stash)
>   	close(cp.out);
>   
>   	if (finish_command(&cp) || len < 0)
> -		die(_("stash failed"));
> +		die(_("the command '%s' failed"), "stash");
>   	else if (!len)		/* no changes */
>   		goto out;
>   	strbuf_setlen(&buffer, buffer.len-1);
> @@ -352,7 +352,7 @@ static void read_empty(const struct object_id *oid, int verbose)
>   	args[i] = NULL;
>   
>   	if (run_command_v_opt(args, RUN_GIT_CMD))
> -		die(_("read-tree failed"));
> +		die(_("the command '%s' failed"), "read-tree");
>   }
>   
>   static void reset_hard(const struct object_id *oid, int verbose)
> @@ -369,7 +369,7 @@ static void reset_hard(const struct object_id *oid, int verbose)
>   	args[i] = NULL;
>   
>   	if (run_command_v_opt(args, RUN_GIT_CMD))
> -		die(_("read-tree failed"));
> +		die(_("the command '%s' failed"), "read-tree");
>   }
>   
>   static void restore_state(const struct object_id *head,
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 51776abea63..d293036e790 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -238,7 +238,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
>   	sequencer_init_config(&opts);
>   	res = run_sequencer(argc, argv, &opts);
>   	if (res < 0)
> -		die(_("revert failed"));
> +		die(_("the command '%s' failed"), "revert");
>   	return res;
>   }
>   
> @@ -251,6 +251,6 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
>   	sequencer_init_config(&opts);
>   	res = run_sequencer(argc, argv, &opts);
>   	if (res < 0)
> -		die(_("cherry-pick failed"));
> +		die(_("the command '%s' failed"), "cherry-pick");
>   	return res;
>   }
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 87657907e78..2e6795cd439 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -989,7 +989,7 @@ static int get_pack(struct fetch_pack_args *args,
>   			args->check_self_contained_and_connected &&
>   			ret == 0;
>   	else
> -		die(_("%s failed"), cmd_name);
> +		die(_("the command '%s' failed"), cmd_name);
>   	if (use_sideband && finish_async(&demux))
>   		die(_("error in sideband demultiplexer"));
>   
> diff --git a/remote-curl.c b/remote-curl.c
> index ff44f41011e..8393f56652b 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -1287,7 +1287,7 @@ static int push_dav(int nr_spec, const char **specs)
>   		strvec_push(&child.args, specs[i]);
>   
>   	if (run_command(&child))
> -		die(_("git-http-push failed"));
> +		die(_("the command '%s' failed"), "git-http-push");
>   	return 0;
>   }
>   
> diff --git a/setup.c b/setup.c
> index 04ce33cdcd4..30a4b81257d 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1506,12 +1506,12 @@ int daemonize(void)
>   		case 0:
>   			break;
>   		case -1:
> -			die_errno(_("fork failed"));
> +			die_errno(_("the function '%s' failed"), "fork");
>   		default:
>   			exit(0);
>   	}
>   	if (setsid() == -1)
> -		die_errno(_("setsid failed"));
> +		die_errno(_("the function '%s' failed"), "setsid");
>   	close(0);
>   	close(1);
>   	close(2);

Why not simply "'%s' failed"?

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v2 1/6] i18n: factorize generic failure messages

[Ævar Arnfjörð Bjarmason]

On Sun, Apr 03 2022, Bagas Sanjaya wrote:

> On 02/04/22 23.10, Jean-Noël Avila via GitGitGadget wrote:
> [...]
>> index 04ce33cdcd4..30a4b81257d 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -1506,12 +1506,12 @@ int daemonize(void)
>>   		case 0:
>>   			break;
>>   		case -1:
>> -			die_errno(_("fork failed"));
>> +			die_errno(_("the function '%s' failed"), "fork");
>>   		default:
>>   			exit(0);
>>   	}
>>   	if (setsid() == -1)
>> -		die_errno(_("setsid failed"));
>> +		die_errno(_("the function '%s' failed"), "setsid");
>>   	close(0);
>>   	close(1);
>>   	close(2);
>
> Why not simply "'%s' failed"?

I think saying what failed is helpful in any case, a user who knows
nothing about *nix APIs might be quite perplexed at their source control
telling them their fork failed, why not the spoon? :)

So including more context helps.

But also because there's languages where adjectives like "failed" are
different depending on the grammatical gender of the subject.

So even if the original and translation would be needlessly terse in any
case, the message would also become imppossible to translate in a
gramatically correct way.

Re: [PATCH v2 1/6] i18n: factorize generic failure messages

[Ævar Arnfjörð Bjarmason]

On Sat, Apr 02 2022, Jean-Noël Avila via GitGitGadget wrote:

> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> [...]
> diff --git a/add-patch.c b/add-patch.c
> index 55d719f7845..8c9e81ec78e 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1181,7 +1181,7 @@ static int run_apply_check(struct add_p_state *s,
>  			    "apply", "--check", NULL);
>  	strvec_pushv(&cp.args, s->mode->apply_check_args);
>  	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
> -		return error(_("'git apply --cached' failed"));
> +		return error(_("the command '%s' failed"), "git apply --cached");
>  
>  	return 0;
>  }
> @@ -1683,7 +1683,7 @@ soft_increment:
>  			strvec_pushv(&cp.args, s->mode->apply_args);
>  			if (pipe_command(&cp, s->buf.buf, s->buf.len,
>  					 NULL, 0, NULL, 0))
> -				error(_("'git apply' failed"));
> +				error(_("the command '%s' failed"), "git apply");
>  		}
>  		if (repo_read_index(s->s.r) >= 0)
>  			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
> diff --git a/builtin/am.c b/builtin/am.c
> index 0f4111bafa0..a0a57049510 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -586,7 +586,7 @@ static int is_mail(FILE *fp)
>  	int ret = 1;
>  
>  	if (fseek(fp, 0L, SEEK_SET))
> -		die_errno(_("fseek failed"));
> +		die_errno(_("the function '%s' failed"), "fseek");
>  
>  	if (regcomp(&regex, header_regex, REG_NOSUB | REG_EXTENDED))
>  		die("invalid pattern: %s", header_regex);


I don't think this needs to happen now, but I wonder if it would be
worth it as a follow-up to e.g. create a gettext-common.h or something,
with macros like:

	#define I18N_COMMAND_FAILED N_("the command '%s' failed")
	#define I18N_FUNCTION_FAILED_ERRNO N_("the library function '%s' failed")

Then:

	error(_(I18N_FUNCTION_FAILED_ERRNO), "git apply");
	die_errno(_(I18N_FUNCTION_FAILED_ERRNO), "fseek");

But OTOH all the gettext tooling already takes care of that, so maybe
it's not worth it. I.e. "jump to definition" would jump to the wrapper
header, as opposed to the actual code involved.

So having written that, probably not. Maybe the only worthwhile thing
would be some Levenshtein distance check in CI or something to see if
we're adding strings that are too similar to existing ones...

[PATCH v2 2/6] sequencer: factor GIT_AUTHOR_* from message strings

[Bagas Sanjaya via GitGitGadget]

From: Bagas Sanjaya <bagasdotme@gmail.com>

Factor messages containing GIT_AUTHOR_* variable.

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 sequencer.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 84eed9e96bc..647f5efa656 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -848,17 +848,17 @@ int read_author_script(const char *path, char **name, char **email, char **date,
 	for (i = 0; i < kv.nr; i++) {
 		if (!strcmp(kv.items[i].string, "GIT_AUTHOR_NAME")) {
 			if (name_i != -2)
-				name_i = error(_("'GIT_AUTHOR_NAME' already given"));
+				name_i = error(_("'%s' already given"), "GIT_AUTHOR_NAME");
 			else
 				name_i = i;
 		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_EMAIL")) {
 			if (email_i != -2)
-				email_i = error(_("'GIT_AUTHOR_EMAIL' already given"));
+				email_i = error(_("'%s' already given"), "GIT_AUTHOR_EMAIL");
 			else
 				email_i = i;
 		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_DATE")) {
 			if (date_i != -2)
-				date_i = error(_("'GIT_AUTHOR_DATE' already given"));
+				date_i = error(_("'%s' already given"), "GIT_AUTHOR_DATE");
 			else
 				date_i = i;
 		} else {
@@ -867,11 +867,11 @@ int read_author_script(const char *path, char **name, char **email, char **date,
 		}
 	}
 	if (name_i == -2)
-		error(_("missing 'GIT_AUTHOR_NAME'"));
+		error(_("missing '%s'"), "GIT_AUTHOR_NAME");
 	if (email_i == -2)
-		error(_("missing 'GIT_AUTHOR_EMAIL'"));
+		error(_("missing '%s'"), "GIT_AUTHOR_EMAIL");
 	if (date_i == -2)
-		error(_("missing 'GIT_AUTHOR_DATE'"));
+		error(_("missing '%s'"), "GIT_AUTHOR_DATE");
 	if (date_i < 0 || email_i < 0 || date_i < 0 || err)
 		goto finish;
 	*name = kv.items[name_i].util;
-- 
gitgitgadget

[PATCH v2 3/6] i18n: factorize server support messages in fetch-pack

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

The messages "Server supports foo" and "Server does not support bar"
are uniformized and options are quoted.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 fetch-pack.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 2e6795cd439..da3075f61f2 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1044,78 +1044,78 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 		advertise_sid = 0;
 
 	if (server_supports("shallow"))
-		print_verbose(args, _("Server supports %s"), "shallow");
+		print_verbose(args, _("Server supports '%s'"), "shallow");
 	else if (args->depth > 0 || is_repository_shallow(r))
 		die(_("Server does not support shallow clients"));
 	if (args->depth > 0 || args->deepen_since || args->deepen_not)
 		args->deepen = 1;
 	if (server_supports("multi_ack_detailed")) {
-		print_verbose(args, _("Server supports %s"), "multi_ack_detailed");
+		print_verbose(args, _("Server supports '%s'"), "multi_ack_detailed");
 		multi_ack = 2;
 		if (server_supports("no-done")) {
-			print_verbose(args, _("Server supports %s"), "no-done");
+			print_verbose(args, _("Server supports '%s'"), "no-done");
 			if (args->stateless_rpc)
 				no_done = 1;
 		}
 	}
 	else if (server_supports("multi_ack")) {
-		print_verbose(args, _("Server supports %s"), "multi_ack");
+		print_verbose(args, _("Server supports '%s'"), "multi_ack");
 		multi_ack = 1;
 	}
 	if (server_supports("side-band-64k")) {
-		print_verbose(args, _("Server supports %s"), "side-band-64k");
+		print_verbose(args, _("Server supports '%s'"), "side-band-64k");
 		use_sideband = 2;
 	}
 	else if (server_supports("side-band")) {
-		print_verbose(args, _("Server supports %s"), "side-band");
+		print_verbose(args, _("Server supports '%s'"), "side-band");
 		use_sideband = 1;
 	}
 	if (server_supports("allow-tip-sha1-in-want")) {
-		print_verbose(args, _("Server supports %s"), "allow-tip-sha1-in-want");
+		print_verbose(args, _("Server supports '%s'"), "allow-tip-sha1-in-want");
 		allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
 	}
 	if (server_supports("allow-reachable-sha1-in-want")) {
-		print_verbose(args, _("Server supports %s"), "allow-reachable-sha1-in-want");
+		print_verbose(args, _("Server supports '%s'"), "allow-reachable-sha1-in-want");
 		allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
 	}
 	if (server_supports("thin-pack"))
-		print_verbose(args, _("Server supports %s"), "thin-pack");
+		print_verbose(args, _("Server supports '%s'"), "thin-pack");
 	else
 		args->use_thin_pack = 0;
 	if (server_supports("no-progress"))
-		print_verbose(args, _("Server supports %s"), "no-progress");
+		print_verbose(args, _("Server supports '%s'"), "no-progress");
 	else
 		args->no_progress = 0;
 	if (server_supports("include-tag"))
-		print_verbose(args, _("Server supports %s"), "include-tag");
+		print_verbose(args, _("Server supports '%s'"), "include-tag");
 	else
 		args->include_tag = 0;
 	if (server_supports("ofs-delta"))
-		print_verbose(args, _("Server supports %s"), "ofs-delta");
+		print_verbose(args, _("Server supports '%s'"), "ofs-delta");
 	else
 		prefer_ofs_delta = 0;
 
 	if (server_supports("filter")) {
 		server_supports_filtering = 1;
-		print_verbose(args, _("Server supports %s"), "filter");
+		print_verbose(args, _("Server supports '%s'"), "filter");
 	} else if (args->filter_options.choice) {
 		warning("filtering not recognized by server, ignoring");
 	}
 
 	if (server_supports("deepen-since")) {
-		print_verbose(args, _("Server supports %s"), "deepen-since");
+		print_verbose(args, _("Server supports '%s'"), "deepen-since");
 		deepen_since_ok = 1;
 	} else if (args->deepen_since)
-		die(_("Server does not support --shallow-since"));
+		die(_("Server does not support '%s'"), "--shallow-since");
 	if (server_supports("deepen-not")) {
-		print_verbose(args, _("Server supports %s"), "deepen-not");
+		print_verbose(args, _("Server supports '%s'"), "deepen-not");
 		deepen_not_ok = 1;
 	} else if (args->deepen_not)
-		die(_("Server does not support --shallow-exclude"));
+		die(_("Server does not support '%s'"), "--shallow-exclude");
 	if (server_supports("deepen-relative"))
-		print_verbose(args, _("Server supports %s"), "deepen-relative");
+		print_verbose(args, _("Server supports '%s'"), "deepen-relative");
 	else if (args->deepen_relative)
-		die(_("Server does not support --deepen"));
+		die(_("Server does not support '%s'"), "--deepen");
 	if (!server_supports_hash(the_hash_algo->name, NULL))
 		die(_("Server does not support this repository's object format"));
 
-- 
gitgitgadget

[PATCH v2 4/6] i18n: factorize "foo does not take arguments" messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

The messages are split into the ones for ref-filter which deal with
atoms and scalar which has an option.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 contrib/scalar/scalar.c | 2 +-
 ref-filter.c            | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
index 7db2a97416e..7ef1f141366 100644
--- a/contrib/scalar/scalar.c
+++ b/contrib/scalar/scalar.c
@@ -502,7 +502,7 @@ cleanup:
 static int cmd_list(int argc, const char **argv)
 {
 	if (argc != 1)
-		die(_("`scalar list` does not take arguments"));
+		die(_("the '%s' command does not take arguments"), "scalar list");
 
 	if (run_git("config", "--global", "--get-all", "scalar.repo", NULL) < 0)
 		return -1;
diff --git a/ref-filter.c b/ref-filter.c
index 7838bd22b8d..8497f9e25d8 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -317,7 +317,7 @@ static int objecttype_atom_parser(struct ref_format *format, struct used_atom *a
 				  const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(objecttype) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(objecttype)");
 	if (*atom->name == '*')
 		oi_deref.info.typep = &oi_deref.type;
 	else
@@ -349,7 +349,7 @@ static int deltabase_atom_parser(struct ref_format *format, struct used_atom *at
 				 const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(deltabase)");
 	if (*atom->name == '*')
 		oi_deref.info.delta_base_oid = &oi_deref.delta_base_oid;
 	else
@@ -361,7 +361,7 @@ static int body_atom_parser(struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(body) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(body)");
 	atom->u.contents.option = C_BODY_DEP;
 	return 0;
 }
@@ -565,7 +565,7 @@ static int rest_atom_parser(struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
 	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(rest) does not take arguments"));
+		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(rest)");
 	format->use_rest = 1;
 	return 0;
 }
-- 
gitgitgadget

Re: [PATCH v2 4/6] i18n: factorize "foo does not take arguments" messages

[Ævar Arnfjörð Bjarmason]

On Sat, Apr 02 2022, Jean-Noël Avila via GitGitGadget wrote:

> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>
> The messages are split into the ones for ref-filter which deal with
> atoms and scalar which has an option.

I see the git-for-each-ref manpage doesn't really refer to these
consistently, but I tihnk s/atom/format/g or s/atom/name/g would be lot
more obvious, especially in the context of how these are already
discussed in the manpage.

> @@ -317,7 +317,7 @@ static int objecttype_atom_parser(struct ref_format *format, struct used_atom *a
>  				  const char *arg, struct strbuf *err)
>  {
>  	if (arg)
> -		return strbuf_addf_ret(err, -1, _("%%(objecttype) does not take arguments"));
> +		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(objecttype)");
>  	if (*atom->name == '*')
>  		oi_deref.info.typep = &oi_deref.type;
>  	else

Re: [PATCH v2 4/6] i18n: factorize "foo does not take arguments" messages

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Sat, Apr 02 2022, Jean-Noël Avila via GitGitGadget wrote:
>
>> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>>
>> The messages are split into the ones for ref-filter which deal with
>> atoms and scalar which has an option.
>
> I see the git-for-each-ref manpage doesn't really refer to these
> consistently, but I tihnk s/atom/format/g or s/atom/name/g would be lot
> more obvious, especially in the context of how these are already
> discussed in the manpage.

I do not necessarily think so, even though "atom" is a word that
directly faces those who wrote the code in for-each-ref.c that have
been moved to ref-filter.c and not the end users.

These are only parts of a string that is given to --format=..., so
"format" makes it more confusing than even the original.

I can buy

    '%(objectype)' in format does not take arguments

though.  If you did not find a specific word to refer to these
"field names" that the documentation consistently uses, it is a way
to clarify which '%(objecttype)' we are referring to, without having
to commit to a single word.

Or we can call them "field names" like the documentation calls them,
which would make it into

    field name '%(objecttype)' does not take arguments

which is not too bad, but I somehow find the former (i.e. "X in
format string does not take arguments") probably the easiest to
follow, if you want to change the original.

Just my 2 yen.

>> @@ -317,7 +317,7 @@ static int objecttype_atom_parser(struct ref_format *format, struct used_atom *a
>>  				  const char *arg, struct strbuf *err)
>>  {
>>  	if (arg)
>> -		return strbuf_addf_ret(err, -1, _("%%(objecttype) does not take arguments"));
>> +		return strbuf_addf_ret(err, -1, _("the atom '%s' does not take arguments"), "%(objecttype)");
>>  	if (*atom->name == '*')
>>  		oi_deref.info.typep = &oi_deref.type;
>>  	else

[PATCH v2 5/6] i18n: factorize read-cache error messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 read-cache.c     | 8 ++++----
 t/t1600-index.sh | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 1ad56d02e1d..639765e8ff1 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1735,8 +1735,8 @@ static unsigned int get_index_format_default(struct repository *r)
 		if (r->settings.index_version >= 0)
 			version = r->settings.index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
-			warning(_("index.version set, but the value is invalid.\n"
-				  "Using version %i"), INDEX_FORMAT_DEFAULT);
+			warning(_("'%s' set, but the value is invalid.\n"
+				  "Using version %i"), "index.version", INDEX_FORMAT_DEFAULT);
 			return INDEX_FORMAT_DEFAULT;
 		}
 		return version;
@@ -1745,8 +1745,8 @@ static unsigned int get_index_format_default(struct repository *r)
 	version = strtoul(envversion, &endp, 10);
 	if (*endp ||
 	    version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
-		warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
-			  "Using version %i"), INDEX_FORMAT_DEFAULT);
+		warning(_("'%s' set, but the value is invalid.\n"
+			  "Using version %i"), "GIT_INDEX_VERSION", INDEX_FORMAT_DEFAULT);
 		version = INDEX_FORMAT_DEFAULT;
 	}
 	return version;
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 010989f90e6..60339db529f 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -19,7 +19,7 @@ test_expect_success 'bogus GIT_INDEX_VERSION issues warning' '
 		git add a 2>err &&
 		sed "s/[0-9]//" err >actual.err &&
 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
-			warning: GIT_INDEX_VERSION set, but the value is invalid.
+			warning: '\''GIT_INDEX_VERSION'\'' set, but the value is invalid.
 			Using version Z
 		EOF
 		test_cmp expect.err actual.err
@@ -34,7 +34,7 @@ test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
 		git add a 2>err &&
 		sed "s/[0-9]//" err >actual.err &&
 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
-			warning: GIT_INDEX_VERSION set, but the value is invalid.
+			warning: '\''GIT_INDEX_VERSION'\'' set, but the value is invalid.
 			Using version Z
 		EOF
 		test_cmp expect.err actual.err
@@ -58,7 +58,7 @@ test_expect_success 'out of bounds index.version issues warning' '
 		git add a 2>err &&
 		sed "s/[0-9]//" err >actual.err &&
 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
-			warning: index.version set, but the value is invalid.
+			warning: '\''index.version'\'' set, but the value is invalid.
 			Using version Z
 		EOF
 		test_cmp expect.err actual.err
-- 
gitgitgadget

Re: [PATCH v2 5/6] i18n: factorize read-cache error messages

[Junio C Hamano]

"Jean-Noël Avila via GitGitGadget"  <gitgitgadget@gmail.com> writes:

> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>
> Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
> ---
>  read-cache.c     | 8 ++++----
>  t/t1600-index.sh | 6 +++---
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 1ad56d02e1d..639765e8ff1 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1735,8 +1735,8 @@ static unsigned int get_index_format_default(struct repository *r)
>  		if (r->settings.index_version >= 0)
>  			version = r->settings.index_version;
>  		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
> -			warning(_("index.version set, but the value is invalid.\n"
> -				  "Using version %i"), INDEX_FORMAT_DEFAULT);
> +			warning(_("'%s' set, but the value is invalid.\n"
> +				  "Using version %i"), "index.version", INDEX_FORMAT_DEFAULT);
>  			return INDEX_FORMAT_DEFAULT;

Not the focus of this patch, which (I am guessing without the log
message) is to prevent translators from touching the constant
'index.version' by mistake, but I wonder if the message is helpful
to users.  When I see such a warning, how would I figure out what
to fix so that I do not have to see the same warning again?

We should somehow at least hint that we are talking about an entry
in .git/config file (or it may be coming ~/.gitconfig by mistake, in
which case telling where we see a funny version string does help the
user even more).

At least, telling that index.version is a configuration variable may
nudge the user in the right direction, or is it too obvious and
everybody who touches Git instinctively know when told that their
qfwfq.zfzfz is set to an incorrect value, we are referring to their
configuration variable?  I dunno.

> @@ -1745,8 +1745,8 @@ static unsigned int get_index_format_default(struct repository *r)
>  	version = strtoul(envversion, &endp, 10);
>  	if (*endp ||
>  	    version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
> -		warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
> -			  "Using version %i"), INDEX_FORMAT_DEFAULT);
> +		warning(_("'%s' set, but the value is invalid.\n"
> +			  "Using version %i"), "GIT_INDEX_VERSION", INDEX_FORMAT_DEFAULT);

Likewise, but this is for an envirionment variable.

	The environment variable '%s' set, but the value is invalid.

may be overkill and more importantly makes it impossible to reuse
the previous message, but the latter may be an improvement.
configuration and environment variables may have different
gramatical gender or something, perhaps.

[PATCH v2 6/6] i18n: factorize "bad argument" messages

[Jean-Noël Avila via GitGitGadget]

From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>

We also quote the placeholders as they replace constant strings.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
---
 builtin/bisect--helper.c | 2 +-
 builtin/commit-graph.c   | 2 +-
 builtin/config.c         | 2 +-
 builtin/env--helper.c    | 2 +-
 builtin/gc.c             | 4 ++--
 diff.c                   | 4 ++--
 grep.c                   | 2 +-
 send-pack.c              | 2 +-
 submodule-config.c       | 8 ++++----
 submodule.c              | 2 +-
 t/t1300-config.sh        | 2 +-
 t/t7900-maintenance.sh   | 4 ++--
 12 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8b2b259ff0d..3ed8a5f40b6 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -268,7 +268,7 @@ static int bisect_write(const char *state, const char *rev,
 	} else if (one_of(state, terms->term_good, "skip", NULL)) {
 		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
 	} else {
-		res = error(_("Bad bisect_write argument: %s"), state);
+		res = error(_("bad '%s' argument: '%s'"), "bisect_write", state);
 		goto finish;
 	}
 
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 51c4040ea6c..913478bb029 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -134,7 +134,7 @@ static int write_option_parse_split(const struct option *opt, const char *arg,
 	else if (!strcmp(arg, "replace"))
 		*flags = COMMIT_GRAPH_SPLIT_REPLACE;
 	else
-		die(_("unrecognized --split argument, %s"), arg);
+		die(_("bad '%s' argument: '%s'"), "--split", arg);
 
 	return 0;
 }
diff --git a/builtin/config.c b/builtin/config.c
index e7b88a9c08d..2ac36e4f641 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -106,7 +106,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
 		else if (!strcmp(arg, "color"))
 			new_type = TYPE_COLOR;
 		else
-			die(_("unrecognized --type argument, %s"), arg);
+			die(_("bad '%s' argument: '%s'"), "--type", arg);
 	}
 
 	to_type = opt->value;
diff --git a/builtin/env--helper.c b/builtin/env--helper.c
index 27349098b07..49790dfa2fb 100644
--- a/builtin/env--helper.c
+++ b/builtin/env--helper.c
@@ -24,7 +24,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
 	else if (!strcmp(arg, "ulong"))
 		*cmdmode = ENV_HELPER_TYPE_ULONG;
 	else
-		die(_("unrecognized --type argument, %s"), arg);
+		die(_("bad '%s' argument: '%s'"), "--type", arg);
 
 	return 0;
 }
diff --git a/builtin/gc.c b/builtin/gc.c
index c062d7bceeb..cca6987fcbd 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -750,7 +750,7 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,
 	*priority = parse_schedule(arg);
 
 	if (!*priority)
-		die(_("unrecognized --schedule argument '%s'"), arg);
+		die(_("bad '%s' argument: '%s'"), "--schedule", arg);
 
 	return 0;
 }
@@ -2393,7 +2393,7 @@ static int maintenance_opt_scheduler(const struct option *opt, const char *arg,
 
 	*scheduler = parse_scheduler(arg);
 	if (*scheduler == SCHEDULER_INVALID)
-		return error(_("unrecognized --scheduler argument '%s'"), arg);
+		return error(_("bad '%s' argument: '%s'"), "--scheduler", arg);
 	return 0;
 }
 
diff --git a/diff.c b/diff.c
index 6b22946cd0e..c74f6e7b920 100644
--- a/diff.c
+++ b/diff.c
@@ -5010,7 +5010,7 @@ static int diff_opt_color_moved(const struct option *opt,
 	} else {
 		int cm = parse_color_moved(arg);
 		if (cm < 0)
-			return error(_("bad --color-moved argument: %s"), arg);
+			return error(_("bad '%s' argument: '%s'"), "--color-moved", arg);
 		options->color_moved = cm;
 	}
 	return 0;
@@ -5334,7 +5334,7 @@ static int diff_opt_word_diff(const struct option *opt,
 		else if (!strcmp(arg, "none"))
 			options->word_diff = DIFF_WORDS_NONE;
 		else
-			return error(_("bad --word-diff argument: %s"), arg);
+			return error(_("bad '%s' argument: '%s'"), "--word-diff", arg);
 	} else {
 		if (options->word_diff == DIFF_WORDS_NONE)
 			options->word_diff = DIFF_WORDS_PLAIN;
diff --git a/grep.c b/grep.c
index 82eb7da1022..6aabfc58bb3 100644
--- a/grep.c
+++ b/grep.c
@@ -43,7 +43,7 @@ static int parse_pattern_type_arg(const char *opt, const char *arg)
 		return GREP_PATTERN_TYPE_FIXED;
 	else if (!strcmp(arg, "perl"))
 		return GREP_PATTERN_TYPE_PCRE;
-	die("bad %s argument: %s", opt, arg);
+	die("bad '%s' argument: '%s'", opt, arg);
 }
 
 define_list_config_array_extra(color_grep_slots, {"match"});
diff --git a/send-pack.c b/send-pack.c
index bc0fcdbb000..68205e8a913 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -36,7 +36,7 @@ int option_parse_push_signed(const struct option *opt,
 		*(int *)(opt->value) = SEND_PACK_PUSH_CERT_IF_ASKED;
 		return 0;
 	}
-	die("bad %s argument: %s", opt->long_name, arg);
+	die("bad '%s' argument: '%s'", opt->long_name, arg);
 }
 
 static void feed_object(const struct object_id *oid, FILE *fh, int negative)
diff --git a/submodule-config.c b/submodule-config.c
index 29668b0620d..76264c6e6b6 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -292,7 +292,7 @@ static int parse_fetch_recurse(const char *opt, const char *arg,
 		 * git-completion.bash when you add new options.
 		 */
 		if (die_on_error)
-			die("bad %s argument: %s", opt, arg);
+			die("bad '%s' argument: '%s'", opt, arg);
 		else
 			return RECURSE_SUBMODULES_ERROR;
 	}
@@ -342,7 +342,7 @@ static int parse_update_recurse(const char *opt, const char *arg,
 		return RECURSE_SUBMODULES_OFF;
 	default:
 		if (die_on_error)
-			die("bad %s argument: %s", opt, arg);
+			die("bad '%s' argument: '%s'", opt, arg);
 		return RECURSE_SUBMODULES_ERROR;
 	}
 }
@@ -359,7 +359,7 @@ static int parse_push_recurse(const char *opt, const char *arg,
 	case 1:
 		/* There's no simple "on" value when pushing */
 		if (die_on_error)
-			die("bad %s argument: %s", opt, arg);
+			die("bad '%s' argument: '%s'", opt, arg);
 		else
 			return RECURSE_SUBMODULES_ERROR;
 	case 0:
@@ -376,7 +376,7 @@ static int parse_push_recurse(const char *opt, const char *arg,
 		 * git-completion.bash when you add new modes.
 		 */
 		else if (die_on_error)
-			die("bad %s argument: %s", opt, arg);
+			die("bad '%s' argument: '%s'", opt, arg);
 		else
 			return RECURSE_SUBMODULES_ERROR;
 	}
diff --git a/submodule.c b/submodule.c
index 5ace18a7d94..c08477762d9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -470,7 +470,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
 	else if (!strcmp(arg, "dirty"))
 		diffopt->flags.ignore_dirty_submodules = 1;
 	else if (strcmp(arg, "none"))
-		die(_("bad --ignore-submodules argument: %s"), arg);
+		die(_("bad '%s' argument: '%s'"), "--ignore-submodules", arg);
 	/*
 	 * Please update _git_status() in git-completion.bash when you
 	 * add new options
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 7dd9b325d90..69cb3af0885 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2215,7 +2215,7 @@ test_expect_success 'unset type specifiers may be reset to conflicting ones' '
 
 test_expect_success '--type rejects unknown specifiers' '
 	test_must_fail git config --type=nonsense section.foo 2>error &&
-	test_i18ngrep "unrecognized --type argument" error
+	test_i18ngrep "bad '\''--type'\'' argument" error
 '
 
 test_expect_success '--replace-all does not invent newlines' '
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 74aa6384755..c5e62e95533 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -401,7 +401,7 @@ test_expect_success '--auto and --schedule incompatible' '
 
 test_expect_success 'invalid --schedule value' '
 	test_must_fail git maintenance run --schedule=annually 2>err &&
-	test_i18ngrep "unrecognized --schedule" err
+	test_i18ngrep "bad '\''--schedule'\''" err
 '
 
 test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
@@ -529,7 +529,7 @@ test_expect_success !MINGW 'register and unregister with regex metacharacters' '
 
 test_expect_success 'start --scheduler=<scheduler>' '
 	test_expect_code 129 git maintenance start --scheduler=foo 2>err &&
-	test_i18ngrep "unrecognized --scheduler argument" err &&
+	test_i18ngrep "bad '\''--scheduler'\'' argument" err &&
 
 	test_expect_code 129 git maintenance start --no-scheduler 2>err &&
 	test_i18ngrep "unknown option" err &&
-- 
gitgitgadget

Re: [PATCH v2 6/6] i18n: factorize "bad argument" messages

[Ævar Arnfjörð Bjarmason]

On Sat, Apr 02 2022, Jean-Noël Avila via GitGitGadget wrote:

> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>
> We also quote the placeholders as they replace constant strings.

I don't think this change is good in that it takes two different cases
we cared about distinguishing before, and squashes them into one. I.e.:

> +++ b/builtin/bisect--helper.c
> @@ -268,7 +268,7 @@ static int bisect_write(const char *state, const char *rev,
>  	} else if (one_of(state, terms->term_good, "skip", NULL)) {
>  		strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
>  	} else {
> -		res = error(_("Bad bisect_write argument: %s"), state);
> +		res = error(_("bad '%s' argument: '%s'"), "bisect_write", state);
>  		goto finish;
>  	}

I think these should all say "bad value provided for '%s' argument:
'%s'", or similar, or maybe the terse version is better. Just a
suggestion.

> diff --git a/builtin/config.c b/builtin/config.c
> index e7b88a9c08d..2ac36e4f641 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -106,7 +106,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
>  		else if (!strcmp(arg, "color"))
>  			new_type = TYPE_COLOR;
>  		else
> -			die(_("unrecognized --type argument, %s"), arg);
> +			die(_("bad '%s' argument: '%s'"), "--type", arg);
>  	}

I thought some of these were introducing logic errors, because we were
conflating unrecognized arguments with bad values, but these all seem to
actually mean "bad value", not "unknown flag".

> diff --git a/grep.c b/grep.c
> index 82eb7da1022..6aabfc58bb3 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -43,7 +43,7 @@ static int parse_pattern_type_arg(const char *opt, const char *arg)
>  		return GREP_PATTERN_TYPE_FIXED;
>  	else if (!strcmp(arg, "perl"))
>  		return GREP_PATTERN_TYPE_PCRE;
> -	die("bad %s argument: %s", opt, arg);
> +	die("bad '%s' argument: '%s'", opt, arg);
>  }

This and many other changes that follow have nothing to do with i18n.

I think it's a worthwhile cleanup to mark some of these for _(), but
shouldn't that come first, or at least after?