diff mbox series

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

Message ID 1888778902aa1d0d2cf118cfb7fe4b2e407953c6.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>

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(-)

Comments

Junio C Hamano April 3, 2022, 10:29 p.m. UTC | #1 
"Jean-Noël Avila via GitGitGadget"  <gitgitgadget@gmail.com> writes:

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

> @@ -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.
diff mbox series

Patch

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;
 		}
 		return version;
@@ -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);
 		version = INDEX_FORMAT_DEFAULT;
 	}
 	return version;
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 010989f90e6..60339db529f 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -19,7 +19,7 @@  test_expect_success 'bogus GIT_INDEX_VERSION issues warning' '
 		git add a 2>err &&
 		sed "s/[0-9]//" err >actual.err &&
 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
-			warning: GIT_INDEX_VERSION set, but the value is invalid.
+			warning: '\''GIT_INDEX_VERSION'\'' set, but the value is invalid.
 			Using version Z
 		EOF
 		test_cmp expect.err actual.err
@@ -34,7 +34,7 @@  test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
 		git add a 2>err &&
 		sed "s/[0-9]//" err >actual.err &&
 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
-			warning: GIT_INDEX_VERSION set, but the value is invalid.
+			warning: '\''GIT_INDEX_VERSION'\'' set, but the value is invalid.
 			Using version Z
 		EOF
 		test_cmp expect.err actual.err
@@ -58,7 +58,7 @@  test_expect_success 'out of bounds index.version issues warning' '
 		git add a 2>err &&
 		sed "s/[0-9]//" err >actual.err &&
 		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
-			warning: index.version set, but the value is invalid.
+			warning: '\''index.version'\'' set, but the value is invalid.
 			Using version Z
 		EOF
 		test_cmp expect.err actual.err