Feature Request - Better i18n support

Feature Request - Better i18n support

[Emir SARI]

Greetings,

Git is translated into many languages, but as far as I can see, it does not
take localisation formats into account nor it allows them to be translated
into localised formats.

For instance, in Turkish (and in French AFAIK), percentages are indicated
in different formats. In Turkish it precedes the number like %54. However,
all the percentages use the standard 54%. By marking these as translatable,
we could easily provide the correct formats.

Another thing is the decimal signs. In many languages, a comma (,) is used to
indicate decimals, however Git does not allow another formats. I am not sure
how to achieve this in C, but it would be great to have this in order to have
an errorless professional look in Git outputs.

Thank you very much for your attention.

[Sistem Bilgisi]
git sürümü:
git version 2.39.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Darwin 22.3.0 Darwin Kernel Version 22.3.0: Mon Jan 30 20:42:11 PST 2023; root:xnu-8792.81.3~2/RELEASE_X86_64 x86_64
derleyici bilgisi: clang: 14.0.0 (clang-1400.0.29.202)
libc bilgisi: libc bilgisi yok
$SHELL (typically, interactive shell): /bin/zsh

Re: Feature Request - Better i18n support

[Junio C Hamano]

Emir SARI <emir_sari@icloud.com> writes:

> For instance, in Turkish (and in French AFAIK), percentages are indicated
> in different formats. In Turkish it precedes the number like %54. However,
> all the percentages use the standard 54%. By marking these as translatable,
> we could easily provide the correct formats.

Interesting.  Let's take a look to see how bad it is.

$ git grep '[fd]%%' \*.[ch] ':!compat/' ':!t/'
apply.c:		printf(" %s %.*s{%s => %s} (%d%%)\n", renamecopy,
apply.c:		printf(" %s %s => %s (%d%%)\n", renamecopy,
apply.c:					printf(" rewrite %s (%d%%)\n",

These are currently not even marked with _() to be translatable, and
it should be just the matter of enclosing them in _().

If these should be translated in the first place, that is.  I do not
think these are originally meant to be machine parseable (it is
shown as part of "git apply --summary"), but existing users may
already be abusing them as such.

diff.c:				fprintf(opt->file, "%s%4d.%01d%% %.*s\n", line_prefix,

This is part of "git diff --dirstat" output.  I do not think it
should be translated, even though it probably is not machine
parseable.

diff.c:		strbuf_addf(msg, "%s%ssimilarity index %d%%",
diff.c:		strbuf_addf(msg, "%s%ssimilarity index %d%%",
diff.c:			strbuf_addf(msg, "%s%sdissimilarity index %d%%%s\n",
diff.c:	strbuf_addf(&sb, " %s %s (%d%%)\n",

These are part of the extended diff headers (e.g. "rename A to B
with similarity index of X%") that are very much designed to be
machine parseable, and cannot be touched.

diff.c:			strbuf_addf(&sb, " (%d%%)\n", similarity_index(p));

This is part of "git diff --summary" to show "rewrite", similarly to
"rename" and "copy" that we saw earlier.

wt-status.c:				_("You are in a sparse checkout with %d%% of tracked files present."),

This is already marked _(translatable) and your l10n files should be
able to do "%d%%" -> "%%%d" as needed.

Re: Feature Request - Better i18n support

[Emir SARI]

Hello,

Thank you very much for the prompt responses.

> Junio C Hamano <gitster@pobox.com> şunları yazdı (10 Mar 2023 01:16):
> 
> These are currently not even marked with _() to be translatable, and
> it should be just the matter of enclosing them in _().
> 
> If these should be translated in the first place, that is.  I do not
> think these are originally meant to be machine parseable (it is
> shown as part of "git apply --summary"), but existing users may
> already be abusing them as such.

My general idea is, if the percentages appear with the translated
strings, they should use the localised format. Otherwise if they
only appear with some machine-output without any localised context
it is perfectly fine to leave them as-is.

> brian m. carlson <sandals@crustytoothpaste.net> şunları yazdı (10 Mar 2023 01:27):
> 
> More specifically, I think it could be fixed for progress output, but
> there are a few places in diff output where it couldn't because it might
> be parsed.  Would you be willing to try a patch for this?  I think the
> code you're probably thinking about is in progress.c (search for "%%"),
> but there might be other places you have in mind as well.

The progress output was my biggest motivation in reporting this. It was
rather disturbing to see translated strings with unlocalised values.

I’d love to assist in testing any possible patches. Just let me know of
any possible scenarios, and I’ll get back to you. Being in vacation, I
have plenty of time available.

> Can you clarify what places you're thinking about?  Is this printing
> decimal values, or is it parsing decimal values?  If we know what
> specifically is affected (for example, an affected command or message),
> it's a lot easier to fix this.

In general, I am not concerned with parsing, it’s more about printing
them. As with the percentage situation, having unlocalised content with
localised strings is not ideal. For instance, my ideal pull output would
look like this:

Nesneler sayılıyor: %100 (12/12), bitti.
Delta sıkıştırması 8 iş parçacığı kullanıyor
Nesneler sıkıştırılıyor: %100 (7/7), bitti.
Nesneler yazılıyor: %100 (7/7), 3,79 KiB | 3,79 MiB/sn, bitti.

As I’ve indicated above, I am ready to test anything you might throw
at me. :)

Thank you again very much!

Best regards,
Emir (𐰽𐰺𐰍)

** E-mail needs to stay simple
** Use plain text e-mail

Re: Feature Request - Better i18n support

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1933 bytes --]

On 2023-03-09 at 12:56:21, Emir SARI wrote:
> Greetings,

Hey,

> Git is translated into many languages, but as far as I can see, it does not
> take localisation formats into account nor it allows them to be translated
> into localised formats.

Thanks for the report.

> For instance, in Turkish (and in French AFAIK), percentages are indicated
> in different formats. In Turkish it precedes the number like %54. However,
> all the percentages use the standard 54%. By marking these as translatable,
> we could easily provide the correct formats.

I think this should be relatively easy to do.  I didn't know Turkish did
this (and I suspect nobody else did or we'd have fixed it), but I
haven't personally seen this in French (which doesn't mean that it isn't
done in some places).  We've already done human-readable byte values,
which do vary in French (due to it using o, for octet, instead of B), so
I don't see any reason why this couldn't be fixed.

More specifically, I think it could be fixed for progress output, but
there are a few places in diff output where it couldn't because it might
be parsed.  Would you be willing to try a patch for this?  I think the
code you're probably thinking about is in progress.c (search for "%%"),
but there might be other places you have in mind as well.

> Another thing is the decimal signs. In many languages, a comma (,) is used to
> indicate decimals, however Git does not allow another formats. I am not sure
> how to achieve this in C, but it would be great to have this in order to have
> an errorless professional look in Git outputs.

Can you clarify what places you're thinking about?  Is this printing
decimal values, or is it parsing decimal values?  If we know what
specifically is affected (for example, an affected command or message),
it's a lot easier to fix this.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Feature Request - Better i18n support

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> More specifically, I think it could be fixed for progress output, but
> there are a few places in diff output where it couldn't because it might
> be parsed.  Would you be willing to try a patch for this?  I think the
> code you're probably thinking about is in progress.c (search for "%%"),
> but there might be other places you have in mind as well.

Ah, my earlier grep missed this, as it tried only to catch [fdi]%%
and the progress thing uses "%u%%" like so:

		if (percent != progress->last_percent || progress_update) {
			progress->last_percent = percent;

			strbuf_reset(counters_sb);
			strbuf_addf(counters_sb,
				    "%3u%% (%"PRIuMAX"/%"PRIuMAX")%s", percent,
				    (uintmax_t)n, (uintmax_t)progress->total,
				    tp);
			show_update = 1;
		}

With the PRIuMAX thing, it won't be as simple as enclosing the
entire format string inside _(), though.

Thanks.

Re: Feature Request - Better i18n support

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]

On 2023-03-09 at 22:35:57, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > More specifically, I think it could be fixed for progress output, but
> > there are a few places in diff output where it couldn't because it might
> > be parsed.  Would you be willing to try a patch for this?  I think the
> > code you're probably thinking about is in progress.c (search for "%%"),
> > but there might be other places you have in mind as well.
> 
> Ah, my earlier grep missed this, as it tried only to catch [fdi]%%
> and the progress thing uses "%u%%" like so:
> 
> 		if (percent != progress->last_percent || progress_update) {
> 			progress->last_percent = percent;
> 
> 			strbuf_reset(counters_sb);
> 			strbuf_addf(counters_sb,
> 				    "%3u%% (%"PRIuMAX"/%"PRIuMAX")%s", percent,
> 				    (uintmax_t)n, (uintmax_t)progress->total,
> 				    tp);
> 			show_update = 1;
> 		}
> 
> With the PRIuMAX thing, it won't be as simple as enclosing the
> entire format string inside _(), though.

Sure, but we can split it into two invocations of strbuf_addf, one
localized, one not, no big deal.  This was the place that immediately
stood out to me as the place we see lots of percentages, but I'm glad
you mentioned a couple other places as well.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Feature Request - Better i18n support

[Emir SARI]

Hello,

How does the below diff look:

diff --git a/apply.c b/apply.c
index 6212ab3..3e1d08d 100644
--- a/apply.c
+++ b/apply.c
@@ -4247,11 +4247,11 @@ static void show_rename_copy(struct patch *p)
 	 * new_name through the end of names are renames
 	 */
 	if (old_name != p->old_name)
-		printf(" %s %.*s{%s => %s} (%d%%)\n", renamecopy,
+		printf(_(" %s %.*s{%s => %s} (%d%%)\n"), renamecopy,
 		       (int)(old_name - p->old_name), p->old_name,
 		       old_name, new_name, p->score);
 	else
-		printf(" %s %s => %s (%d%%)\n", renamecopy,
+		printf(_(" %s %s => %s (%d%%)\n"), renamecopy,
 		       p->old_name, p->new_name, p->score);
 	show_mode_change(p, 0);
 }
@@ -4270,8 +4270,8 @@ static void summary_patch_list(struct patch *patch)
 				show_rename_copy(p);
 			else {
 				if (p->score) {
-					printf(" rewrite %s (%d%%)\n",
-					       p->new_name, p->score);
+					printf(" rewrite %s ", p->new_name);
+					printf(_("(%d%%)\n"), p->score);
 					show_mode_change(p, 0);
 				}
 				else
diff --git a/progress.c b/progress.c
index f695798..1f8d372 100644
--- a/progress.c
+++ b/progress.c
@@ -124,10 +124,24 @@ static void display(struct progress *progress, uint64_t n, const char *done)
 			progress->last_percent = percent;
 
 			strbuf_reset(counters_sb);
-			strbuf_addf(counters_sb,
-				    "%3u%% (%"PRIuMAX"/%"PRIuMAX")%s", percent,
+
+			struct strbuf progress_sb = STRBUF_INIT;
+			strbuf_addf(&progress_sb,
+				_("%u%% (%"PRIuMAX"/%"PRIuMAX")%s"), percent,
 				    (uintmax_t)n, (uintmax_t)progress->total,
 				    tp);
+			struct strbuf progress_str = STRBUF_INIT;
+			strbuf_addstr(&progress_str, progress_sb.buf);
+			strbuf_release(&progress_sb);
+
+			if (percent < 10)
+			    strbuf_insert(&progress_str, 0, "  ", 2);
+			else if (percent < 100)
+			    strbuf_insert(&progress_str, 0, " ", 1);
+
+			strbuf_addf(counters_sb, "%s", progress_str.buf);
+			strbuf_release(&progress_str);
+
 			show_update = 1;
 		}
 	} else if (progress_update) {

I’ve left diff.c out, since there are no accompanying localised strings to my knowledge. If this looks good, then only remaining thing to tackle would be localised decimal separators I think.

Best regards,
Emir (𐰽𐰺𐰍)

** E-mail needs to stay simple
** Use plain text e-mail