[v5] l10n: localizable upload progress messages
diff mbox series

Message ID 20190625062540.88973-2-dimitriy.ryazantcev@gmail.com
State New
Headers show
Series
  • [v5] l10n: localizable upload progress messages
Related show

Commit Message

Dimitriy Ryazantcev June 25, 2019, 6:25 a.m. UTC
Currenly the data rate in throughput_string(...) method is
output by simple strbuf_humanise_bytes(...) call and '/s' append.
But for proper translation of such string the translator needs
full context.

Add strbuf_humanise_rate(...) method to properly print out
localizable version of data rate ('3.5 MiB/s' etc) with full context.

Strings with the units in strbuf_humanise_bytes(...) are marked
for translation.

Signed-off-by: Dimitriy Ryazantcev <dimitriy.ryazantcev@gmail.com>
---
 progress.c |  3 +--
 strbuf.c   | 37 +++++++++++++++++++++++++++++++++----
 strbuf.h   |  6 ++++++
 3 files changed, 40 insertions(+), 6 deletions(-)

Comments

Junio C Hamano June 25, 2019, 8 p.m. UTC | #1
Dimitriy Ryazantcev <dimitriy.ryazantcev@gmail.com> writes:

> Currenly the data rate in throughput_string(...) method is
> output by simple strbuf_humanise_bytes(...) call and '/s' append.
> But for proper translation of such string the translator needs
> full context.
>
> Add strbuf_humanise_rate(...) method to properly print out
> localizable version of data rate ('3.5 MiB/s' etc) with full context.

Hmph, so idea is that appending translation of "/s" to translation
of "1.4MiB" may not be a good enough translation of "1.4MiB/s"?

That does sounds like a good idea, but looking at the heavy
duplication of implementation, I would have to say "Yuck" to it.

I wonder if an approach like the following illustration would work
better?  I am not sure how well Q_() and N_() would interact with
each other, though.

-- >8 --

struct human_format {
	const char *giga;
	const char *mega;
	const char *kilo;
	const char *byte;
	const char *bytes;
};

static void strbuf_humanise(struct strbuf *buf, off_t bytes, struct human_format *fmt)
{
	if (bytes > 1 << 30) {
		strbuf_addf(buf, _(fmt->giga), 
			    (unsigned)(bytes >> 30),
			    (unsigned)(bytes & ((1 << 30) - 1)) / 10737419);
	} else if (bytes > 1 << 20) {
		unsigned x = bytes + 5243;  /* for rounding */
		strbuf_addf(buf, _(fmt->mega),
			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
		strbuf_addstr(buf, _(""));
	} else if (bytes > 1 << 10) {
		unsigned x = bytes + 5;  /* for rounding */
		strbuf_addf(buf, _(fmt->kilo),
			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
	} else {
		strbuf_addf(buf, Q_(fmt->byte, fmt->bytes,
				    (unsigned)bytes), (unsigned)bytes);
	}
}

void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes)
{
	struct human_format bytes_fmt = {
		.giga = N_("%u.%2.2u GiB"),
		.mega = N_("%u.%2.2u MiB"),
		.kilo = N_("%u.%2.2u KiB"),
		.byte = N_("%u byte"),
		.bytes = N_("%u bytes"),
	};
	strbuf_humanise(buf, bytes, &bytes_fmt);
}

void strbuf_humanise_rate(struct strbuf *buf, off_t bytes)
{
	struct human_format rate_fmt = {
		.giga = N_("%u.%2.2u GiB/s"),
		.mega = N_("%u.%2.2u MiB/s"),
		.kilo = N_("%u.%2.2u KiB/s"),
		.byte = N_("%u byte/s"),
		.bytes = N_("%u bytes/s"),
	};
	strbuf_humanise(buf, bytes, &rate_fmt);
}
Dimitriy Ryazantcev June 26, 2019, 8:59 a.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:
>
> Dimitriy Ryazantcev <dimitriy.ryazantcev@gmail.com> writes:
>
> > Currenly the data rate in throughput_string(...) method is
> > output by simple strbuf_humanise_bytes(...) call and '/s' append.
> > But for proper translation of such string the translator needs
> > full context.
> >
> > Add strbuf_humanise_rate(...) method to properly print out
> > localizable version of data rate ('3.5 MiB/s' etc) with full context.
>
> Hmph, so idea is that appending translation of "/s" to translation
> of "1.4MiB" may not be a good enough translation of "1.4MiB/s"?
>
> That does sounds like a good idea, but looking at the heavy
> duplication of implementation, I would have to say "Yuck" to it.
>
> I wonder if an approach like the following illustration would work
> better?  I am not sure how well Q_() and N_() would interact with
> each other, though.

Good idea!
But unfortunately this will not work properly as xgettext will unable
to extract byte/bytes to one plural msgid in *.pot file.
Seems it can extract plural forms only from Q_ (ngettext() call) and
cannot from _N (gettext_noop() call) in current configuration[0].
Maybe in this case we can try to forward already translated string
to strbuf_humanise():

static void strbuf_humanise(struct strbuf *buf, off_t bytes, struct
human_format *fmt)
{
....
        } else {
                strbuf_addf(buf, fmt->byte, (unsigned)bytes);
....
}

void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes)
{
        struct human_format bytes_fmt = {
                .giga = _("%u.%2.2u GiB"),
                .mega = _("%u.%2.2u MiB"),
                .kilo = _("%u.%2.2u KiB"),
                .byte = Q_("%u byte", "%u bytes", (unsigned)bytes),
        };
        strbuf_humanise(buf, bytes, &bytes_fmt);
}

What do you think?

[0] https://github.com/git/git/blob/master/Makefile#L2498


>
> -- >8 --
>
> struct human_format {
>         const char *giga;
>         const char *mega;
>         const char *kilo;
>         const char *byte;
>         const char *bytes;
> };
>
> static void strbuf_humanise(struct strbuf *buf, off_t bytes, struct human_format *fmt)
> {
>         if (bytes > 1 << 30) {
>                 strbuf_addf(buf, _(fmt->giga),
>                             (unsigned)(bytes >> 30),
>                             (unsigned)(bytes & ((1 << 30) - 1)) / 10737419);
>         } else if (bytes > 1 << 20) {
>                 unsigned x = bytes + 5243;  /* for rounding */
>                 strbuf_addf(buf, _(fmt->mega),
>                             x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
>                 strbuf_addstr(buf, _(""));
>         } else if (bytes > 1 << 10) {
>                 unsigned x = bytes + 5;  /* for rounding */
>                 strbuf_addf(buf, _(fmt->kilo),
>                             x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
>         } else {
>                 strbuf_addf(buf, Q_(fmt->byte, fmt->bytes,
>                                     (unsigned)bytes), (unsigned)bytes);
>         }
> }
>
> void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes)
> {
>         struct human_format bytes_fmt = {
>                 .giga = N_("%u.%2.2u GiB"),
>                 .mega = N_("%u.%2.2u MiB"),
>                 .kilo = N_("%u.%2.2u KiB"),
>                 .byte = N_("%u byte"),
>                 .bytes = N_("%u bytes"),
>         };
>         strbuf_humanise(buf, bytes, &bytes_fmt);
> }
>
> void strbuf_humanise_rate(struct strbuf *buf, off_t bytes)
> {
>         struct human_format rate_fmt = {
>                 .giga = N_("%u.%2.2u GiB/s"),
>                 .mega = N_("%u.%2.2u MiB/s"),
>                 .kilo = N_("%u.%2.2u KiB/s"),
>                 .byte = N_("%u byte/s"),
>                 .bytes = N_("%u bytes/s"),
>         };
>         strbuf_humanise(buf, bytes, &rate_fmt);
> }



--
Sincerely,
Dimitriy Ryazantcev
Junio C Hamano June 26, 2019, 5:58 p.m. UTC | #3
Dimitriy <dimitriy.ryazantcev@gmail.com> writes:

> Seems it can extract plural forms only from Q_ (ngettext() call) and
> cannot from _N (gettext_noop() call) in current configuration[0].
> Maybe in this case we can try to forward already translated string
> to strbuf_humanise():
>
> static void strbuf_humanise(struct strbuf *buf, off_t bytes, struct
> human_format *fmt)
> {
> ....
>         } else {
>                 strbuf_addf(buf, fmt->byte, (unsigned)bytes);
> ....
> }
>
> void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes)
> {
>         struct human_format bytes_fmt = {
>                 .giga = _("%u.%2.2u GiB"),
>                 .mega = _("%u.%2.2u MiB"),
>                 .kilo = _("%u.%2.2u KiB"),
>                 .byte = Q_("%u byte", "%u bytes", (unsigned)bytes),
>         };
>         strbuf_humanise(buf, bytes, &bytes_fmt);
> }
>
> What do you think?

Well, I was trying to avoid making Q_() call when we did not have
to, but if we are running with gettext, we cannot avoid calling Q_()
every time this helper is called, as .byte field has to depend on
the value of bytes, so...

Patch
diff mbox series

diff --git a/progress.c b/progress.c
index a2e8cf64a8..951f7c7461 100644
--- a/progress.c
+++ b/progress.c
@@ -150,8 +150,7 @@  static void throughput_string(struct strbuf *buf, uint64_t total,
 	strbuf_addstr(buf, ", ");
 	strbuf_humanise_bytes(buf, total);
 	strbuf_addstr(buf, " | ");
-	strbuf_humanise_bytes(buf, rate * 1024);
-	strbuf_addstr(buf, "/s");
+	strbuf_humanise_rate(buf, rate * 1024);
 }
 
 void display_throughput(struct progress *progress, uint64_t total)
diff --git a/strbuf.c b/strbuf.c
index 0e18b259ce..785c9e5b55 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -814,19 +814,48 @@  void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,
 void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes)
 {
 	if (bytes > 1 << 30) {
-		strbuf_addf(buf, "%u.%2.2u GiB",
+		/* TRANSLATORS: IEC 80000-13:2008 gibibyte */
+		strbuf_addf(buf, _("%u.%2.2u GiB"),
 			    (unsigned)(bytes >> 30),
 			    (unsigned)(bytes & ((1 << 30) - 1)) / 10737419);
 	} else if (bytes > 1 << 20) {
 		unsigned x = bytes + 5243;  /* for rounding */
-		strbuf_addf(buf, "%u.%2.2u MiB",
+		/* TRANSLATORS: IEC 80000-13:2008 mebibyte */
+		strbuf_addf(buf, _("%u.%2.2u MiB"),
 			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
+		strbuf_addstr(buf, _(""));
 	} else if (bytes > 1 << 10) {
 		unsigned x = bytes + 5;  /* for rounding */
-		strbuf_addf(buf, "%u.%2.2u KiB",
+		/* TRANSLATORS: IEC 80000-13:2008 kibibyte */
+		strbuf_addf(buf, _("%u.%2.2u KiB"),
 			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
 	} else {
-		strbuf_addf(buf, "%u bytes", (unsigned)bytes);
+		/* TRANSLATORS: IEC 80000-13:2008 byte */
+		strbuf_addf(buf, Q_("%u byte", "%u bytes", (unsigned)bytes), (unsigned)bytes);
+	}
+}
+
+void strbuf_humanise_rate(struct strbuf *buf, off_t bytes)
+{
+	if (bytes > 1 << 30) {
+		/* TRANSLATORS: IEC 80000-13:2008 gibibyte/second */
+		strbuf_addf(buf, _("%u.%2.2u GiB/s"),
+			    (unsigned)(bytes >> 30),
+			    (unsigned)(bytes & ((1 << 30) - 1)) / 10737419);
+	} else if (bytes > 1 << 20) {
+		unsigned x = bytes + 5243;  /* for rounding */
+		/* TRANSLATORS: IEC 80000-13:2008 mebibyte/second */
+		strbuf_addf(buf, _("%u.%2.2u MiB/s"),
+			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
+		strbuf_addstr(buf, _(""));
+	} else if (bytes > 1 << 10) {
+		unsigned x = bytes + 5;  /* for rounding */
+		/* TRANSLATORS: IEC 80000-13:2008 kibibyte/second */
+		strbuf_addf(buf, _("%u.%2.2u KiB/s"),
+			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
+	} else {
+		/* TRANSLATORS: IEC 80000-13:2008 byte/second */
+		strbuf_addf(buf, Q_("%u byte/s", "%u bytes/s", (unsigned)bytes), (unsigned)bytes);
 	}
 }
 
diff --git a/strbuf.h b/strbuf.h
index c8d98dfb95..f62278a0be 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -372,6 +372,12 @@  void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src);
  */
 void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes);
 
+/**
+ * Append the given byte rate as a human-readable string (i.e. 12.23 KiB/s,
+ * 3.50 MiB/s).
+ */
+void strbuf_humanise_rate(struct strbuf *buf, off_t bytes);
+
 /**
  * Add a formatted string to the buffer.
  */