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

[Johannes Sixt <j.sixt@viscovery.net>]

Am 12/22/2011 19:08, schrieb Junio C Hamano:
> I _think_ I liked the simplicity of having the logic to
> 
>  - format localized message to a multi-line buffer; and
>  - split the contents of that buffer into lines and add prefix in an
>    i18n friendly way
> 
> in vreportf(), but there are many problems in this approach, it seems.  We
> may need to limit the message length for error()/die() codepath, but we do
> not want to be limited in others, definitely not from advise().
> 
> Also some calls to error() are meant to produce plumbing error message and
> should never be localized. The approach to localize the prefix in vreportf()
> will not work for this reason.

IMO the solution to not translate plumbing messages is to omit the
initialization of the gettext machinery.

Anyway, here is a patch that modifies vreportf() in an i18n friendly way
(I think). It is not necessarily meant for inclusion. Notably, the
changes to the test suite indicate one problem in a class of error()
messages: A list of file names is produced that are indented by a tab.
But with the "error: " prefix, the visible indentation shrinks to two
spaces (the minimum possible). We may want to do something about it.

--- >8 ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] Put a prefix on all lines of multi-line hint, warning,
 error messages

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 advice.c                             |    2 +-
 git-compat-util.h                    |    2 +-
 http-backend.c                       |    2 +-
 run-command.c                        |    2 +-
 t/t1011-read-tree-sparse-checkout.sh |    6 ++--
 t/t1506-rev-parse-diagnosis.sh       |    2 +-
 t/t7607-merge-overwrite.sh           |    6 ++--
 t/t7609-merge-co-error-msgs.sh       |   40 +++++++++++++++++-----------------
 usage.c                              |   19 +++++++++++-----
 9 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/advice.c b/advice.c
index e02e632..38b55b6 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: %s\n", advice, params);
 	va_end(params);
 }
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 8f3972c..26b7d19 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -239,7 +239,7 @@ extern char *gitbasename(char *);
 #include "compat/bswap.h"
 
 /* General helper functions */
-extern void vreportf(const char *prefix, const char *err, va_list params);
+extern void vreportf(const char *line_fmt, const char *err, va_list params);
 extern void vwritef(int fd, const char *prefix, const char *err, va_list params);
 extern NORETURN void usage(const char *err);
 extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
diff --git a/http-backend.c b/http-backend.c
index 869d515..fb91742 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: %s\n", err, params);
 	}
 	exit(0); /* we successfully reported a failure ;-) */
 }
diff --git a/run-command.c b/run-command.c
index 1c51043..664c215 100644
--- a/run-command.c
+++ b/run-command.c
@@ -466,7 +466,7 @@ static void *run_thread(void *data)
 
 static NORETURN void die_async(const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf("fatal: %s\n", err, params);
 
 	if (!pthread_equal(main_thread, pthread_self())) {
 		struct async *async = pthread_getspecific(async_key);
diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 5c0053a..22b6a20 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -242,9 +242,9 @@ test_expect_success 'print errors when failed to update worktree' '
 	test_must_fail git checkout top 2>actual &&
 	cat >expected <<\EOF &&
 error: The following untracked working tree files would be overwritten by checkout:
-	sub/added
-	sub/addedtoo
-Please move or remove them before you can switch branches.
+error: 	sub/added
+error: 	sub/addedtoo
+error: Please move or remove them before you can switch branches.
 Aborting
 EOF
 	test_cmp expected actual
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index 0843a1c..292f2fb 100755
--- a/t/t1506-rev-parse-diagnosis.sh
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -11,7 +11,7 @@ test_did_you_mean ()
 	sq="'" &&
 	cat >expected <<-EOF &&
 	fatal: Path '$2$3' $4, but not ${5:-$sq$3$sq}.
-	Did you mean '$1:$2$3'${2:+ aka $sq$1:./$3$sq}?
+	fatal: Did you mean '$1:$2$3'${2:+ aka $sq$1:./$3$sq}?
 	EOF
 	test_cmp expected error
 }
diff --git a/t/t7607-merge-overwrite.sh b/t/t7607-merge-overwrite.sh
index aa74184..7d6498d 100755
--- a/t/t7607-merge-overwrite.sh
+++ b/t/t7607-merge-overwrite.sh
@@ -104,9 +104,9 @@ test_expect_success 'will not overwrite untracked subtree' '
 
 cat >expect <<\EOF
 error: The following untracked working tree files would be overwritten by merge:
-	sub
-	sub2
-Please move or remove them before you can merge.
+error: 	sub
+error: 	sub2
+error: Please move or remove them before you can merge.
 Aborting
 EOF
 
diff --git a/t/t7609-merge-co-error-msgs.sh b/t/t7609-merge-co-error-msgs.sh
index 0e4a682..653eb7e 100755
--- a/t/t7609-merge-co-error-msgs.sh
+++ b/t/t7609-merge-co-error-msgs.sh
@@ -27,11 +27,11 @@ test_expect_success 'setup' '
 
 cat >expect <<\EOF
 error: The following untracked working tree files would be overwritten by merge:
-	five
-	four
-	three
-	two
-Please move or remove them before you can merge.
+error: 	five
+error: 	four
+error: 	three
+error: 	two
+error: Please move or remove them before you can merge.
 Aborting
 EOF
 
@@ -50,13 +50,13 @@ test_expect_success 'untracked files overwritten by merge (fast and non-fast for
 
 cat >expect <<\EOF
 error: Your local changes to the following files would be overwritten by merge:
-	four
-	three
-	two
-Please, commit your changes or stash them before you can merge.
+error: 	four
+error: 	three
+error: 	two
+error: Please, commit your changes or stash them before you can merge.
 error: The following untracked working tree files would be overwritten by merge:
-	five
-Please move or remove them before you can merge.
+error: 	five
+error: Please move or remove them before you can merge.
 Aborting
 EOF
 
@@ -70,9 +70,9 @@ test_expect_success 'untracked files or local changes ovewritten by merge' '
 
 cat >expect <<\EOF
 error: Your local changes to the following files would be overwritten by checkout:
-	rep/one
-	rep/two
-Please, commit your changes or stash them before you can switch branches.
+error: 	rep/one
+error: 	rep/two
+error: Please, commit your changes or stash them before you can switch branches.
 Aborting
 EOF
 
@@ -92,9 +92,9 @@ test_expect_success 'cannot switch branches because of local changes' '
 
 cat >expect <<\EOF
 error: Your local changes to the following files would be overwritten by checkout:
-	rep/one
-	rep/two
-Please, commit your changes or stash them before you can switch branches.
+error: 	rep/one
+error: 	rep/two
+error: Please, commit your changes or stash them before you can switch branches.
 Aborting
 EOF
 
@@ -106,9 +106,9 @@ test_expect_success 'not uptodate file porcelain checkout error' '
 
 cat >expect <<\EOF
 error: Updating the following directories would lose untracked files in it:
-	rep
-	rep2
-
+error: 	rep
+error: 	rep2
+error: 
 Aborting
 EOF
 
diff --git a/usage.c b/usage.c
index a2a6678..1b55afd 100644
--- a/usage.c
+++ b/usage.c
@@ -6,11 +6,18 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
-void vreportf(const char *prefix, const char *err, va_list params)
+void vreportf(const char *line_fmt, const char *err, va_list params)
 {
 	char msg[4096];
+	char *line, *end;
+
 	vsnprintf(msg, sizeof(msg), err, params);
-	fprintf(stderr, "%s%s\n", prefix, msg);
+
+	for (line = msg; (end = strchr(line, '\n')); line = end) {
+		*end++ = 0;
+		fprintf(stderr, line_fmt, line);
+	}
+	fprintf(stderr, line_fmt, line);
 }
 
 void vwritef(int fd, const char *prefix, const char *err, va_list params)
@@ -27,24 +34,24 @@ void vwritef(int fd, const char *prefix, const char *err, va_list params)
 
 static NORETURN void usage_builtin(const char *err, va_list params)
 {
-	vreportf("usage: ", err, params);
+	vreportf("usage: %s\n", err, params);
 	exit(129);
 }
 
 static NORETURN void die_builtin(const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf("fatal: %s\n", err, params);
 	exit(128);
 }
 
 static void error_builtin(const char *err, va_list params)
 {
-	vreportf("error: ", err, params);
+	vreportf("error: %s\n", err, params);
 }
 
 static void warn_builtin(const char *warn, va_list params)
 {
-	vreportf("warning: ", warn, params);
+	vreportf("warning: %s\n", warn, params);
 }
 
 /* If we are in a dlopen()ed .so write to a global variable would segfault
-- 
1.7.8.rc0.126.gd15506