git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [RFC/PATCH] i18n of multi-line messages
Date: Fri, 23 Dec 2011 07:42:27 +0100	[thread overview]
Message-ID: <4EF422D3.2050802@viscovery.net> (raw)
In-Reply-To: <7v39cceay0.fsf@alter.siamese.dyndns.org>

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

  reply	other threads:[~2011-12-23  6:42 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-21 23:55 [RFC/PATCH] i18n of multi-line messages Junio C Hamano
2011-12-22  0:14 ` Ævar Arnfjörð Bjarmason
2011-12-22  0:20   ` Junio C Hamano
2011-12-22 17:07     ` Thomas Rast
2011-12-22  6:54 ` Johannes Sixt
2011-12-22  7:00   ` Junio C Hamano
2011-12-22  7:00   ` Junio C Hamano
2011-12-22  7:38   ` Junio C Hamano
2011-12-22  8:19     ` Johannes Sixt
2011-12-22 18:08       ` Junio C Hamano
2011-12-23  6:42         ` Johannes Sixt [this message]
2011-12-23 20:54           ` Junio C Hamano
2011-12-22 10:40     ` Chris Packham
2011-12-22 11:56       ` Andreas Schwab
2011-12-22 21:44     ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EF422D3.2050802@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).