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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 3, 2022, 22:29 UTC
Message-ID: <xmqqlewlu7gh.fsf@gitster.g>
In-Reply-To: <1888778902aa1d0d2cf118cfb7fe4b2e407953c6.1648915853.git.gitgitgadget@gmail.com>

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

Show 21 quoted lines

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

Show 8 quoted lines

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