Re: [PATCH v3 24/39] i18n: bisect: enable l10n of bisect terms in messages

2 messages, 2 authors, 2016-06-16 · open the first message on its own page

Re: [PATCH v3 24/39] i18n: bisect: enable l10n of bisect terms in messages

From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:41

Vasco Almeida [off-list ref] writes:
quoted hunk
+enum term { BAD, GOOD, OLD, NEW };
+static const char *term_names[] = {
+/* TRANSLATORS: in bisect.c source code file, the following terms are
+   used to describe a "bad commit", "good commit", "new revision", etc.
+   Please, if you can, check the source when you are not sure if a %s
+   would be replaced by one of the following terms. */
+	N_("bad"), N_("good"), N_("old"), N_("new"),  NULL
+};
+
 /* Remember to update object flag allocation in object.h */
 #define COUNTED		(1u<<16)
 
@@ -725,12 +734,12 @@ static void handle_bad_merge_base(void)
 	if (is_expected_rev(current_bad_oid)) {
 		char *bad_hex = oid_to_hex(current_bad_oid);
 		char *good_hex = join_sha1_array_hex(&good_revs, ' ');
-		if (!strcmp(term_bad, "bad") && !strcmp(term_good, "good")) {
+		if (!strcmp(term_bad, term_names[BAD]) && !strcmp(term_good, term_names[GOOD])) {
 			fprintf(stderr, _("The merge base %s is bad.\n"
 				"This means the bug has been fixed "
 				"between %s and [%s].\n"),
 				bad_hex, bad_hex, good_hex);
-		} else if (!strcmp(term_bad, "new") && !strcmp(term_good, "old")) {
+		} else if (!strcmp(term_bad, term_names[NEW]) && !strcmp(term_good, term_names[OLD])) {
 			fprintf(stderr, _("The merge base %s is new.\n"
 				"The property has changed "
 				"between %s and [%s].\n"),
@@ -739,7 +748,7 @@ static void handle_bad_merge_base(void)
 			fprintf(stderr, _("The merge base %s is %s.\n"
 				"This means the first '%s' commit is "
 				"between %s and [%s].\n"),
-				bad_hex, term_bad, term_good, bad_hex, good_hex);
+				bad_hex, _(term_bad), _(term_good), bad_hex, good_hex);
These "bad" and "good" that are compared with term_bad and term_good
are the literal tokens the end user gives from the "git bisect"
command line.  I do not think you would want to catch them with

    $ git bisect novo <rev>
    $ git bisect velho <rev>

unless the user has done

    $ git bisect --term-old=velho --term-new=novo

previously.

And that "custom bisect terms" case is covered by the last "else"
clause in this if/elseif cascade (outside the context we can see in
your message).

The only thing you need to do around here is to mark the string as
translatable.  I do not think we need "enum term", or term_names[].
quoted hunk
@@ -747,7 +756,7 @@ static void handle_bad_merge_base(void)
 	fprintf(stderr, _("Some %s revs are not ancestor of the %s rev.\n"
 		"git bisect cannot work properly in this case.\n"
 		"Maybe you mistook %s and %s revs?\n"),
-		term_good, term_bad, term_good, term_bad);
+		_(term_good), _(term_bad), _(term_good), _(term_bad));
Likewise for all _(term_good), _(term_bad) and use of term_names[]
we see in this patch.

Re: [PATCH v3 24/39] i18n: bisect: enable l10n of bisect terms in messages

From: Vasco Almeida <hidden>
Date: 2016-06-16 02:19:42

Às 17:38 de 01-06-2016, Junio C Hamano escreveu:
Vasco Almeida [off-list ref] writes:
quoted
+enum term { BAD, GOOD, OLD, NEW };
+static const char *term_names[] = {
+/* TRANSLATORS: in bisect.c source code file, the following terms are
+   used to describe a "bad commit", "good commit", "new revision", etc.
+   Please, if you can, check the source when you are not sure if a %s
+   would be replaced by one of the following terms. */
+	N_("bad"), N_("good"), N_("old"), N_("new"),  NULL
+};
+
 /* Remember to update object flag allocation in object.h */
 #define COUNTED		(1u<<16)
 
@@ -725,12 +734,12 @@ static void handle_bad_merge_base(void)
 	if (is_expected_rev(current_bad_oid)) {
 		char *bad_hex = oid_to_hex(current_bad_oid);
 		char *good_hex = join_sha1_array_hex(&good_revs, ' ');
-		if (!strcmp(term_bad, "bad") && !strcmp(term_good, "good")) {
+		if (!strcmp(term_bad, term_names[BAD]) && !strcmp(term_good, term_names[GOOD])) {
 			fprintf(stderr, _("The merge base %s is bad.\n"
 				"This means the bug has been fixed "
 				"between %s and [%s].\n"),
 				bad_hex, bad_hex, good_hex);
-		} else if (!strcmp(term_bad, "new") && !strcmp(term_good, "old")) {
+		} else if (!strcmp(term_bad, term_names[NEW]) && !strcmp(term_good, term_names[OLD])) {
 			fprintf(stderr, _("The merge base %s is new.\n"
 				"The property has changed "
 				"between %s and [%s].\n"),
@@ -739,7 +748,7 @@ static void handle_bad_merge_base(void)
 			fprintf(stderr, _("The merge base %s is %s.\n"
 				"This means the first '%s' commit is "
 				"between %s and [%s].\n"),
-				bad_hex, term_bad, term_good, bad_hex, good_hex);
+				bad_hex, _(term_bad), _(term_good), bad_hex, good_hex);
These "bad" and "good" that are compared with term_bad and term_good
are the literal tokens the end user gives from the "git bisect"
command line.  I do not think you would want to catch them with

    $ git bisect novo <rev>
    $ git bisect velho <rev>

unless the user has done

    $ git bisect --term-old=velho --term-new=novo

previously.
I may be misunderstanding you, but we do not "catch" those terms with
this patch, although I'm not sure what you mean by "catch them". I think
you forget that no-operation N_("good"), does not affect in any way the
string "good", it only enables xgettext to extract it to .pot file, does
not trigger translation.
Overall, I don't understand what are you trying to tell me here.
And that "custom bisect terms" case is covered by the last "else"
clause in this if/elseif cascade (outside the context we can see in
your message).

The only thing you need to do around here is to mark the string as
translatable.  I do not think we need "enum term", or term_names[].
This patch tries to make bisect output those term translated within the
also translated message. To enable this, it is handy to have
term_names[] in order to mark each term, although I could have mark them
anywhere they appeared in the source. It was only for that I chose to
have term_names[].
quoted
@@ -747,7 +756,7 @@ static void handle_bad_merge_base(void)
 	fprintf(stderr, _("Some %s revs are not ancestor of the %s rev.\n"
 		"git bisect cannot work properly in this case.\n"
 		"Maybe you mistook %s and %s revs?\n"),
-		term_good, term_bad, term_good, term_bad);
+		_(term_good), _(term_bad), _(term_good), _(term_bad));
Likewise for all _(term_good), _(term_bad) and use of term_names[]
we see in this patch.
Indeed this was more of a PATCH/RFC to see what people would think of
it. If nobody contest and there is no value in it, I'll happily drop
this patch in the next re-roll.

My motivation for this patch was that a user could feel embarrassment
reading a message in her language with those terms untranslated.
Although I do believe that no translating them is also a possibility.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help