diff mbox series

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

Message ID 4bba3e1f6cb9cdc35b0dc8da440e38de256b4d2b.1648915853.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series More i18n fixes | expand

Commit Message

Jean-Noël Avila April 2, 2022, 4:10 p.m. UTC 
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(-)

Comments

Bagas Sanjaya April 3, 2022, 5:56 a.m. UTC | #1 
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"?
Ævar Arnfjörð Bjarmason April 3, 2022, 2:34 p.m. UTC | #2 
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.
Ævar Arnfjörð Bjarmason April 3, 2022, 2:47 p.m. UTC | #3 
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...
diff mbox series

Patch

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