Re: [RFC/PATCH] i18n of multi-line messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:

> IMHO, this logic should be moved into vreportf(), and we get proper
> prefixing of multi-line warning(), error(), and die() messages for free.

I agree with this, so here is a rewrite to do so.

Two points to note.

 - Existing users of vreportf() and vwritef() are modified to pass the
   prefix like "error", "warning", etc. without colon+SP. The formatting
   convention regarding how the "prefix" is separated from the body of the
   message should also be locale specific.

 - I expect some tests will fail, as there would be existing users that
   pass multi-line strings to die(), error() and friends.

For the latter, I didn't bother to check, but I would not be surprised if
everybody thinks the updated output that repeats the same prefix line to
every line is easier to read and formatted more consistently. Updating the
tests to go with this change is left as an exercise for the reader.

-- >8 --
Advice messages are by definition meant for human end-users, and prime
candidates for i18n/l10n. They tend to also be more verbose to be helpful,
and need to be longer than just one line.

Although we do not have parameterized multi-line advice messages yet, once
we do, we cannot emit such a message like this:

	advise(_("Please rename %s to something else"), gostak);
        advise(_("so that we can avoid distimming %s unnecessarily."), doshes);

because some translations may need to have the replacement of 'gostak' on
the second line (or 'doshes' on the first line). Some languages may even
need to use three lines in order to fit the same message within a
reasonable width.

Instead, it has to be a single advise() construct, like this:

	advise(_("Please rename %s to something else\n"
                 "so that we can avoid distimming %s unnecessarily."),
		gostak, doshes);

Update the advise() function and its existing callers to

 - take a format string that can be multi-line and translatable as a
   whole;
 - use the string and the parameters to form a localized message; and
 - append each line in the result to localization of the "hint: " prefix.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 advice.c         |   13 +++++------
 builtin/revert.c |    9 +++----
 http-backend.c   |    2 +-
 run-command.c    |    2 +-
 usage.c          |   63 +++++++++++++++++++++++++++++++++++++++++------------
 5 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/advice.c b/advice.c
index e02e632..93a03f5 100644
--- a/advice.c
+++ b/advice.c
@@ -24,7 +24,7 @@ void advise(const char *advice, ...)
 	va_list params;
 
 	va_start(params, advice);
-	vreportf("hint: ", advice, params);
+	vreportf("hint", advice, params);
 	va_end(params);
 }
 
@@ -46,16 +46,15 @@ int git_default_advice_config(const char *var, const char *value)
 int error_resolve_conflict(const char *me)
 {
 	error("'%s' is not possible because you have unmerged files.", me);
-	if (advice_resolve_conflict) {
+	if (advice_resolve_conflict)
 		/*
 		 * Message used both when 'git commit' fails and when
 		 * other commands doing a merge do.
 		 */
-		advise("Fix them up in the work tree,");
-		advise("and then use 'git add/rm <file>' as");
-		advise("appropriate to mark resolution and make a commit,");
-		advise("or use 'git commit -a'.");
-	}
+		advise(_("Fix them up in the work tree,\n"
+			 "and then use 'git add/rm <file>' as\n"
+			 "appropriate to mark resolution and make a commit,\n"
+			 "or use 'git commit -a'."));
 	return -1;
 }
 
diff --git a/builtin/revert.c b/builtin/revert.c
index 1ea525c..3ad14a1 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -332,11 +332,10 @@ static void print_advice(int show_hint)
 		return;
 	}
 
-	if (show_hint) {
-		advise("after resolving the conflicts, mark the corrected paths");
-		advise("with 'git add <paths>' or 'git rm <paths>'");
-		advise("and commit the result with 'git commit'");
-	}
+	if (show_hint)
+		advise(_("after resolving the conflicts, mark the corrected paths\n"
+			 "with 'git add <paths>' or 'git rm <paths>'\n"
+			 "and commit the result with 'git commit'"));
 }
 
 static void write_message(struct strbuf *msgbuf, const char *filename)
diff --git a/http-backend.c b/http-backend.c
index 59ad7da..d372252 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -490,7 +490,7 @@ static NORETURN void die_webcgi(const char *err, va_list params)
 		hdr_nocache();
 		end_headers();
 
-		vreportf("fatal: ", err, params);
+		vreportf("fatal", err, params);
 	}
 	exit(0); /* we successfully reported a failure ;-) */
 }
diff --git a/run-command.c b/run-command.c
index 1c51043..f7a7b5c 100644
--- a/run-command.c
+++ b/run-command.c
@@ -78,7 +78,7 @@ static void notify_parent(void)
 
 static NORETURN void die_child(const char *err, va_list params)
 {
-	vwritef(child_err, "fatal: ", err, params);
+	vwritef(child_err, "fatal", err, params);
 	exit(128);
 }
 
diff --git a/usage.c b/usage.c
index a2a6678..2d392a4 100644
--- a/usage.c
+++ b/usage.c
@@ -6,45 +6,78 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
+typedef void (*emit_fn)(struct strbuf *, void *);
+
+static void v_format(const char *prefix, const char *fmt, va_list params,
+		     emit_fn emit, void *cb_data)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf line = STRBUF_INIT;
+	const char *cp, *np;
+
+	strbuf_vaddf(&buf, fmt, params);
+	for (cp = buf.buf; *cp; cp = np) {
+		np = strchrnul(cp, '\n');
+		/*
+		 * TRANSLATORS: the format is designed so that in RTL
+		 * languages you could reorder and put the "prefix" at
+		 * the end instead of the beginning of a line if you
+		 * wanted to.
+		 */
+		strbuf_addf(&line,
+			    _("%s: %.*s\n"),
+			    prefix,
+			    (int)(np - cp), cp);
+		emit(&line, cb_data);
+		strbuf_reset(&line);
+		if (*np)
+			np++;
+	}
+	strbuf_release(&buf);
+	strbuf_release(&line);
+}
+
+static void emit_report(struct strbuf *line, void *cb_data)
+{
+	fprintf(stderr, "%.*s", (int)line->len, line->buf);
+}
+
 void vreportf(const char *prefix, const char *err, va_list params)
 {
-	char msg[4096];
-	vsnprintf(msg, sizeof(msg), err, params);
-	fprintf(stderr, "%s%s\n", prefix, msg);
+	v_format(prefix, err, params, emit_report, NULL);
 }
 
-void vwritef(int fd, const char *prefix, const char *err, va_list params)
+static void emit_write(struct strbuf *line, void *cb_data)
 {
-	char msg[4096];
-	int len = vsnprintf(msg, sizeof(msg), err, params);
-	if (len > sizeof(msg))
-		len = sizeof(msg);
+	int *fd = cb_data;
+	write_in_full(*fd, line->buf, line->len);
+}
 
-	write_in_full(fd, prefix, strlen(prefix));
-	write_in_full(fd, msg, len);
-	write_in_full(fd, "\n", 1);
+void vwritef(int fd, const char *prefix, const char *err, va_list params)
+{
+	v_format(prefix, err, params, emit_write, &fd);
 }
 
 static NORETURN void usage_builtin(const char *err, va_list params)
 {
-	vreportf("usage: ", err, params);
+	vreportf("usage", err, params);
 	exit(129);
 }
 
 static NORETURN void die_builtin(const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf("fatal", err, params);
 	exit(128);
 }
 
 static void error_builtin(const char *err, va_list params)
 {
-	vreportf("error: ", err, params);
+	vreportf("error", err, params);
 }
 
 static void warn_builtin(const char *warn, va_list params)
 {
-	vreportf("warning: ", warn, params);
+	vreportf("warning", warn, params);
 }
 
 /* If we are in a dlopen()ed .so write to a global variable would segfault
-- 
1.7.8.1.389.gc5932

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]