On Sat, Apr 02 2022, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@xxxxxxx>
>
> 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?
