sideband: mark "remote error:" prefix for translation

Commit Message

Jeff King Aug. 7, 2020, 8:56 a.m. UTC

On Wed, Aug 05, 2020 at 09:28:42AM -0700, Junio C Hamano wrote:

> Jeff King <[e-mail]> writes:
> 
> > By the way, grepping for "remote error:" shows that when we get an error
> > over sideband 3 we produce the same message but _don't_ translate it.
> > That seems inconsistent.
> 
> IOW
> 
>     die(_("remote error: %s"), buf + 1);
> 
> in sideband.c?  I think it makes sense.

Yes. Patch is below so we don't forget about it. I'm not sure if we
ought to be going further, though. The "remote:" prefix for sideband 2
isn't translated either. It would be easy to do so, but it's much more
lego-like. We don't have "remote: %s" ever as a string. We just have
"remote:", and then we maybe_colorize_sideband() the result.

Would that be annoying for translators, especially with RTL languages?
Do people actually want to see "remote:" (or "remote error:" for that
matter) translated, or does mixing translated and untranslated messages
on one line end up more confusing? I'm out of my element here, as I
wouldn't ever use the translations myself.

> IIRC, the current thinking is to let the remote side localize their
> message before sending them over the wire and we'll worry about how
> we let the receiving end tell what l10n it wants later.  So "remote
> error:" prefix may have to be translated on receiving end and the
> remainder of the line, which is already localized, can just be
> interpolated.

Yeah, that part makes sense. The local client shouldn't be translating
what it gets from the server (and won't, because it is filled in via the
%s). Adding a capability for preferred language would be easy, though I
imagine it might be irritating in practice.  As a server admin, I want
to see everything in the C locale; but what gets shown to users and what
might get dumped into server logs is not well specified in Git. I have a
feeling that just setting LANG based on the user's request would be a
bit broad.

Anyway, here's the patch. It doesn't seem to cause any test failures,
even with GETTEXT_POISON. :)

-- >8 --
Subject: [PATCH] sideband: mark "remote error:" prefix for translation

A Git client may produce a "remote error:" message (along with whatever
error the other side sent us) in two places:

  - when we see an ERR packet

  - when we're using a sideband and see sideband 3

We can't reliably translate the message the other side sent us, but we
can do so for our own prefix. However, we translate only the ERR-packet
case but not the sideband-3 case. Let's make them consistent (by marking
both for translation).

Signed-off-by: Jeff King <[e-mail]>
---
I really just care about consistency between the two spots, so swapping
this to translate neither would be to me, too. I guess this does create
inconsistency with "remote: " though. Not sure if it's incremental
forward progress, or just a bad idea. ;)

 sideband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano Aug. 7, 2020, 8:18 p.m. UTC | #1

Jeff King <[e-mail]> writes:

> On Wed, Aug 05, 2020 at 09:28:42AM -0700, Junio C Hamano wrote:
>
>> Jeff King <[e-mail]> writes:
>> 
>> > By the way, grepping for "remote error:" shows that when we get an error
>> > over sideband 3 we produce the same message but _don't_ translate it.
>> > That seems inconsistent.
>> 
>> IOW
>> 
>>     die(_("remote error: %s"), buf + 1);
>> 
>> in sideband.c?  I think it makes sense.
>
> Yes. Patch is below so we don't forget about it. I'm not sure if we
> ought to be going further, though. The "remote:" prefix for sideband 2
> isn't translated either. It would be easy to do so, but it's much more
> lego-like. We don't have "remote: %s" ever as a string. We just have
> "remote:", and then we maybe_colorize_sideband() the result.
>
> Would that be annoying for translators, especially with RTL languages?
> Do people actually want to see "remote:" (or "remote error:" for that
> matter) translated, or does mixing translated and untranslated messages
> on one line end up more confusing? I'm out of my element here, as I
> wouldn't ever use the translations myself.
>
>> IIRC, the current thinking is to let the remote side localize their
>> message before sending them over the wire and we'll worry about how
>> we let the receiving end tell what l10n it wants later.  So "remote
>> error:" prefix may have to be translated on receiving end and the
>> remainder of the line, which is already localized, can just be
>> interpolated.
>
> Yeah, that part makes sense. The local client shouldn't be translating
> what it gets from the server (and won't, because it is filled in via the
> %s). Adding a capability for preferred language would be easy, though I
> imagine it might be irritating in practice.  As a server admin, I want
> to see everything in the C locale; but what gets shown to users and what
> might get dumped into server logs is not well specified in Git. I have a
> feeling that just setting LANG based on the user's request would be a
> bit broad.
>
> Anyway, here's the patch. It doesn't seem to cause any test failures,
> even with GETTEXT_POISON. :)

;-)  Thanks.  Queued.

Patch

diff --git a/sideband.c b/sideband.c
index ef851113c4..0a60662fa6 100644
--- a/sideband.c
+++ b/sideband.c
@@ -147,7 +147,7 @@  int demultiplex_sideband(const char *me, char *buf, int len,
 	switch (band) {
 	case 3:
 		if (die_on_error)
-			die("remote error: %s", buf + 1);
+			die(_("remote error: %s"), buf + 1);
 		strbuf_addf(scratch, "%s%s", scratch->len ? "\n" : "",
 			    DISPLAY_PREFIX);
 		maybe_colorize_sideband(scratch, buf + 1, len);