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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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?