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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 3, 2022, 14:47 UTC
Message-ID: <220403.86k0c6kyi5.gmgdl@evledraar.gmail.com>
In-Reply-To: <4bba3e1f6cb9cdc35b0dc8da440e38de256b4d2b.1648915853.git.gitgitgadget@gmail.com>

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

Show 37 quoted lines

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