[PATCH] i18n: remove i18n from tag reflog message

[PATCH] i18n: remove i18n from tag reflog message

[Jean-Noel Avila]

The building of the reflog message is using strbuf, which is not
friendly with internationalization frameworks. No other reflog
messages are translated right now and switching all the messages to
i18n would require a major rework of the way the messages are built.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 builtin/tag.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 222404522..bdf1e88e9 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -309,7 +309,7 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
 	if (rla) {
 		strbuf_addstr(sb, rla);
 	} else {
-		strbuf_addstr(sb, _("tag: tagging "));
+		strbuf_addstr(sb, "tag: tagging ");
 		strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
 	}
 
@@ -317,14 +317,14 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
 	type = sha1_object_info(sha1, NULL);
 	switch (type) {
 	default:
-		strbuf_addstr(sb, _("object of unknown type"));
+		strbuf_addstr(sb, "object of unknown type");
 		break;
 	case OBJ_COMMIT:
 		if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
 			subject_len = find_commit_subject(buf, &subject_start);
 			strbuf_insert(sb, sb->len, subject_start, subject_len);
 		} else {
-			strbuf_addstr(sb, _("commit object"));
+			strbuf_addstr(sb, "commit object");
 		}
 		free(buf);
 
@@ -332,13 +332,13 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
 			strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
 		break;
 	case OBJ_TREE:
-		strbuf_addstr(sb, _("tree object"));
+		strbuf_addstr(sb, "tree object");
 		break;
 	case OBJ_BLOB:
-		strbuf_addstr(sb, _("blob object"));
+		strbuf_addstr(sb, "blob object");
 		break;
 	case OBJ_TAG:
-		strbuf_addstr(sb, _("other tag object"));
+		strbuf_addstr(sb, "other tag object");
 		break;
 	}
 	strbuf_addch(sb, ')');
-- 
2.12.0

[PATCH v2] i18n patches to apply for rc2

[Jean-Noel Avila]

Please apply the following patches for rc2 so that the localization
can be applied on a cleaned up pot file.

[PATCH v2 1/2] i18n: remove i18n from tag reflog message

[Jean-Noel Avila]

The building of the reflog message is using strbuf, which is not
friendly with internationalization frameworks. No other reflog
messages are translated right now and switching all the messages to
i18n would require a major rework of the way the messages are built.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 builtin/tag.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 222404522..bdf1e88e9 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -309,7 +309,7 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
 	if (rla) {
 		strbuf_addstr(sb, rla);
 	} else {
-		strbuf_addstr(sb, _("tag: tagging "));
+		strbuf_addstr(sb, "tag: tagging ");
 		strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
 	}
 
@@ -317,14 +317,14 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
 	type = sha1_object_info(sha1, NULL);
 	switch (type) {
 	default:
-		strbuf_addstr(sb, _("object of unknown type"));
+		strbuf_addstr(sb, "object of unknown type");
 		break;
 	case OBJ_COMMIT:
 		if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
 			subject_len = find_commit_subject(buf, &subject_start);
 			strbuf_insert(sb, sb->len, subject_start, subject_len);
 		} else {
-			strbuf_addstr(sb, _("commit object"));
+			strbuf_addstr(sb, "commit object");
 		}
 		free(buf);
 
@@ -332,13 +332,13 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
 			strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
 		break;
 	case OBJ_TREE:
-		strbuf_addstr(sb, _("tree object"));
+		strbuf_addstr(sb, "tree object");
 		break;
 	case OBJ_BLOB:
-		strbuf_addstr(sb, _("blob object"));
+		strbuf_addstr(sb, "blob object");
 		break;
 	case OBJ_TAG:
-		strbuf_addstr(sb, _("other tag object"));
+		strbuf_addstr(sb, "other tag object");
 		break;
 	}
 	strbuf_addch(sb, ')');
-- 
2.12.0

[PATCH v2 2/2] i18n: read-cache: Fix typo

[Jean-Noel Avila]

From: Peter Krefting <peter@softwolves.pp.se>

Signed-off-by: Peter Krefting <peter@softwolves.pp.se>
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 read-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index b3d0f3c30..0d0081a11 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2372,7 +2372,7 @@ static int should_delete_shared_index(const char *shared_index_path)
 	if (!expiration)
 		return 0;
 	if (stat(shared_index_path, &st))
-		return error_errno(_("could not stat '%s"), shared_index_path);
+		return error_errno(_("could not stat '%s'"), shared_index_path);
 	if (st.st_mtime > expiration)
 		return 0;
 
-- 
2.12.0

Re: [PATCH] i18n: remove i18n from tag reflog message

[Junio C Hamano]

Jean-Noel Avila <jn.avila@free.fr> writes:

> The building of the reflog message is using strbuf, which is not
> friendly with internationalization frameworks. No other reflog
> messages are translated right now and switching all the messages to
> i18n would require a major rework of the way the messages are built.

Thanks for spotting.  Before we declare that reflog messages are for
i18n, we'd need to either drop (or redesign the implementation of)
the "checkout -" feature, which relies on the exact phrasing of how
reflog entries from "git checkout" looks like.

Will queue and merge down to 'master' quickly.

>
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> ---
>  builtin/tag.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 222404522..bdf1e88e9 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -309,7 +309,7 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
>  	if (rla) {
>  		strbuf_addstr(sb, rla);
>  	} else {
> -		strbuf_addstr(sb, _("tag: tagging "));
> +		strbuf_addstr(sb, "tag: tagging ");
>  		strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
>  	}
>  
> @@ -317,14 +317,14 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
>  	type = sha1_object_info(sha1, NULL);
>  	switch (type) {
>  	default:
> -		strbuf_addstr(sb, _("object of unknown type"));
> +		strbuf_addstr(sb, "object of unknown type");
>  		break;
>  	case OBJ_COMMIT:
>  		if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
>  			subject_len = find_commit_subject(buf, &subject_start);
>  			strbuf_insert(sb, sb->len, subject_start, subject_len);
>  		} else {
> -			strbuf_addstr(sb, _("commit object"));
> +			strbuf_addstr(sb, "commit object");
>  		}
>  		free(buf);
>  
> @@ -332,13 +332,13 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
>  			strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
>  		break;
>  	case OBJ_TREE:
> -		strbuf_addstr(sb, _("tree object"));
> +		strbuf_addstr(sb, "tree object");
>  		break;
>  	case OBJ_BLOB:
> -		strbuf_addstr(sb, _("blob object"));
> +		strbuf_addstr(sb, "blob object");
>  		break;
>  	case OBJ_TAG:
> -		strbuf_addstr(sb, _("other tag object"));
> +		strbuf_addstr(sb, "other tag object");
>  		break;
>  	}
>  	strbuf_addch(sb, ')');

Re: [PATCH] i18n: remove i18n from tag reflog message

[Ævar Arnfjörð Bjarmason]

On Sat, Apr 29, 2017 at 12:02 PM, Jean-Noel Avila <jn.avila@free.fr> wrote:
> The building of the reflog message is using strbuf, which is not
> friendly with internationalization frameworks. No other reflog
> messages are translated right now and switching all the messages to
> i18n would require a major rework of the way the messages are built.

[CC'd Cornelius Weig who added the code you're modifying]

If I'm reading this correctly this is a good patch but quite a bad
explanation of what we're changing.

The problem here is not that it's cumbersome to translate this using
strbuf, but that we're injecting i18n'd messages into the reflog, so
e.g. someone with a Chinese locale creating a tag in a shared repo
will cause the Chinese i18n message to be inserted into the reflog,
and then someone with e.g. a German or English locale will see the
Chinese messages when they run "git reflog".

That's a very bad thing, and in some ways worse than translating
plumbing messages, because here we're injecting i18n'd messages into
the on-disk format.

Luckily this hasn't made it into 2.13 yet. Junio, if the above
description is accurate then this is something we definitely want in
the final 2.13.


> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> ---
>  builtin/tag.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 222404522..bdf1e88e9 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -309,7 +309,7 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
>         if (rla) {
>                 strbuf_addstr(sb, rla);
>         } else {
> -               strbuf_addstr(sb, _("tag: tagging "));
> +               strbuf_addstr(sb, "tag: tagging ");
>                 strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
>         }
>
> @@ -317,14 +317,14 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
>         type = sha1_object_info(sha1, NULL);
>         switch (type) {
>         default:
> -               strbuf_addstr(sb, _("object of unknown type"));
> +               strbuf_addstr(sb, "object of unknown type");
>                 break;
>         case OBJ_COMMIT:
>                 if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
>                         subject_len = find_commit_subject(buf, &subject_start);
>                         strbuf_insert(sb, sb->len, subject_start, subject_len);
>                 } else {
> -                       strbuf_addstr(sb, _("commit object"));
> +                       strbuf_addstr(sb, "commit object");
>                 }
>                 free(buf);
>
> @@ -332,13 +332,13 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
>                         strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
>                 break;
>         case OBJ_TREE:
> -               strbuf_addstr(sb, _("tree object"));
> +               strbuf_addstr(sb, "tree object");
>                 break;
>         case OBJ_BLOB:
> -               strbuf_addstr(sb, _("blob object"));
> +               strbuf_addstr(sb, "blob object");
>                 break;
>         case OBJ_TAG:
> -               strbuf_addstr(sb, _("other tag object"));
> +               strbuf_addstr(sb, "other tag object");
>                 break;
>         }
>         strbuf_addch(sb, ')');
> --
> 2.12.0
>

Re: [PATCH] i18n: remove i18n from tag reflog message

[Jean-Noël AVILA]

Le dimanche 30 avril 2017, 18:45:05 CEST Junio C Hamano a écrit :
> Jean-Noel Avila <jn.avila@free.fr> writes:
> > The building of the reflog message is using strbuf, which is not
> > friendly with internationalization frameworks. No other reflog
> > messages are translated right now and switching all the messages to
> > i18n would require a major rework of the way the messages are built.
> 
> Thanks for spotting.  Before we declare that reflog messages are for
> i18n, we'd need to either drop (or redesign the implementation of)
> the "checkout -" feature, which relies on the exact phrasing of how
> reflog entries from "git checkout" looks like.
> 
> Will queue and merge down to 'master' quickly.
> 

I didn't know this "side effect". Maybe adding a test against it would be 
requiered. Unfortunately, I don't know enough of it.

Re: [PATCH] i18n: remove i18n from tag reflog message

[Junio C Hamano]

Jean-Noël AVILA <jn.avila@free.fr> writes:

> Le dimanche 30 avril 2017, 18:45:05 CEST Junio C Hamano a écrit :
>> Jean-Noel Avila <jn.avila@free.fr> writes:
>> > The building of the reflog message is using strbuf, which is not
>> > friendly with internationalization frameworks. No other reflog
>> > messages are translated right now and switching all the messages to
>> > i18n would require a major rework of the way the messages are built.
>> 
>> Thanks for spotting.  Before we declare that reflog messages are for
>> i18n, we'd need to either drop (or redesign the implementation of)
>> the "checkout -" feature, which relies on the exact phrasing of how
>> reflog entries from "git checkout" looks like.
>> 
>> Will queue and merge down to 'master' quickly.
>> 
>
> I didn't know this "side effect". Maybe adding a test against it would be 
> requiered. Unfortunately, I don't know enough of it.

Sorry, but I am not sure what "side effect" you are referring to.  I
am saying that in the current codebase not translating tag reflog
message (i.e. your fix) is the right thing to do.

builtin/checkout.c does

		strbuf_addf(&msg, "checkout: moving from %s to %s",
			old_desc ? old_desc : "(invalid)", new->name);

when "git checkout" is used to switch from old to new branch.  The
above phrasing is used by sha1_name.c::grab_nth_branch_switch() to
know what branch you were on, and not having _() around the message
is the right thing to do, at least in the current codebase.

If somebody wants to i18n the reflog message, that needs to be done
at the reading/dispaying end, I would think.  Instead of being purely
textual records, a new reflog format would have to be a machine
parsable message-id with parameters, and the code that wants to
understand the reflog record like grab_nth_branch_switch(), as
opposed to show the record to the humans, would just use the machine
parsable raw data, while "git reflog" and friends that format the
reflog record would give the message-id to gettext and feed the
parameters recorded using the result of gettext() as the format
string to fprintf(), or something like that.

Re: [PATCH] i18n: remove i18n from tag reflog message

[Junio C Hamano]

Jean-Noël AVILA <jn.avila@free.fr> writes:

> Le dimanche 30 avril 2017, 18:45:05 CEST Junio C Hamano a écrit :
>> Jean-Noel Avila <jn.avila@free.fr> writes:
>> > The building of the reflog message is using strbuf, which is not
>> > friendly with internationalization frameworks. No other reflog
>> > messages are translated right now and switching all the messages to
>> > i18n would require a major rework of the way the messages are built.
>> 
>> Thanks for spotting.  Before we declare that reflog messages are for
>> i18n, we'd need to either drop (or redesign the implementation of)
>> the "checkout -" feature, which relies on the exact phrasing of how
>> reflog entries from "git checkout" looks like.
>> 
>> Will queue and merge down to 'master' quickly.
>> 
>
> I didn't know this "side effect". Maybe adding a test against it would be 
> requiered. Unfortunately, I don't know enough of it.

Sorry, but I am not sure what "side effect" you are referring to.  I
am saying that in the current codebase not translating tag reflog
message (i.e. your fix) is the right thing to do.

builtin/checkout.c does

		strbuf_addf(&msg, "checkout: moving from %s to %s",
			old_desc ? old_desc : "(invalid)", new->name);

when "git checkout" is used to switch from old to new branch.  The
above phrasing is used by sha1_name.c::grab_nth_branch_switch() to
know what branch you were on, and not having _() around the message
is the right thing to do, at least in the current codebase.

If somebody wants to i18n the reflog message, that needs to be done
at the reading/dispaying end, I would think.  Instead of being purely
textual records, a new reflog format would have to be a machine
parsable message-id with parameters, and the code that wants to
understand the reflog record like grab_nth_branch_switch(), as
opposed to show the record to the humans, would just use the machine
parsable raw data, while "git reflog" and friends that format the
reflog record would give the message-id to gettext and feed the
parameters recorded using the result of gettext() as the format
string to fprintf(), or something like that.

That would solve the issue in multi-user repository Ævar raised,
too.  You'd see Chinese reflog entry not because the person who did
the operation that created the reflog entry were writing Chinese,
but because your locale is set to show Chinese when you tried to
read the reflog.  German users would see the reflog entry that
records what the Chinese developer did in German if we follow that
route.