{"thread":{"id":"36248","subject":"Bug? git status --porcelain --branch is translated","startedAt":"2014-03-20T10:31:59Z","lastAt":"2014-03-21T17:38:14Z","messageCount":5,"participants":["Anarky","Matthieu Moy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"237163","messageId":"CAPP0M7Rw50hPHm49FetNXPGESXOoAND4NXU9F=rajv+NA=G32Q@mail.gmail.com","threadId":"36248","inReplyTo":null,"subject":"Bug? git status --porcelain --branch is translated","fromName":"Anarky","fromEmail":"ghostanarky@gmail.com","sentAt":"2014-03-20T10:31:59Z","receivedAt":"2014-03-20T10:31:59Z","isPatch":false,"sender":{"key":"ghostanarky@gmail.com","avatar":null},"body":"Hello,\n\nThe porcelain format of git status is described as not based on user\nconfiguration.\nBut with --branch, behind/ahead are translated following the user's locale.\nIs it normal that scripts need to take care of that?\n\nThanks.\n"},{"id":"237169","messageId":"1395317561-9380-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"36248","inReplyTo":"CAPP0M7Rw50hPHm49FetNXPGESXOoAND4NXU9F=rajv+NA=G32Q@mail.gmail.com","subject":"[PATCH] status: disable translation when --porcelain is used","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2014-03-20T12:12:41Z","receivedAt":"2014-03-20T12:12:41Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\"git status --branch --porcelain\" displays the status of the branch\n(ahead, behind, gone), and used gettext to translate the string.\n\nUse hardcoded strings when --porcelain is used, but keep the gettext\ntranslation for \"git status --short\" which is essentially the same, but\nmeant to be read by a human.\n\nReported-by: Anarky <ghostanarky@gmail.com>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n> The porcelain format of git status is described as not based on user\n> configuration.\n> But with --branch, behind/ahead are translated following the user's locale.\n> Is it normal that scripts need to take care of that?\n\nIndeed, I'd call that a bug. Here's a fix.\n\n wt-status.c | 15 ++++++++++-----\n wt-status.h |  1 +\n 2 files changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex a452407..e55e5b9 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \t\treturn;\n \t}\n \n+\tconst char *gone   = s->no_gettext ? \"gone\"   : _(\"gone\");\n+\tconst char *behind = s->no_gettext ? \"behind \" : _(\"behind \");\n+\tconst char *ahead  = s->no_gettext ? \"ahead \"  : _(\"ahead \");\n+\n \tcolor_fprintf(s->fp, header_color, \" [\");\n \tif (upstream_is_gone) {\n-\t\tcolor_fprintf(s->fp, header_color, _(\"gone\"));\n+\t\tcolor_fprintf(s->fp, header_color, gone);\n \t} else if (!num_ours) {\n-\t\tcolor_fprintf(s->fp, header_color, _(\"behind \"));\n+\t\tcolor_fprintf(s->fp, header_color, behind);\n \t\tcolor_fprintf(s->fp, branch_color_remote, \"%d\", num_theirs);\n \t} else if (!num_theirs) {\n-\t\tcolor_fprintf(s->fp, header_color, _(\"ahead \"));\n+\t\tcolor_fprintf(s->fp, header_color, ahead);\n \t\tcolor_fprintf(s->fp, branch_color_local, \"%d\", num_ours);\n \t} else {\n-\t\tcolor_fprintf(s->fp, header_color, _(\"ahead \"));\n+\t\tcolor_fprintf(s->fp, header_color, ahead);\n \t\tcolor_fprintf(s->fp, branch_color_local, \"%d\", num_ours);\n-\t\tcolor_fprintf(s->fp, header_color, _(\", behind \"));\n+\t\tcolor_fprintf(s->fp, header_color, \", %s\", behind);\n \t\tcolor_fprintf(s->fp, branch_color_remote, \"%d\", num_theirs);\n \t}\n \n@@ -1566,5 +1570,6 @@ void wt_porcelain_print(struct wt_status *s)\n \ts->use_color = 0;\n \ts->relative_paths = 0;\n \ts->prefix = NULL;\n+\ts->no_gettext = 1;\n \twt_shortstatus_print(s);\n }\ndiff --git a/wt-status.h b/wt-status.h\nindex 30a4812..82f6ce6 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -50,6 +50,7 @@ struct wt_status {\n \tenum commit_whence whence;\n \tint nowarn;\n \tint use_color;\n+\tint no_gettext;\n \tint display_comment_prefix;\n \tint relative_paths;\n \tint submodule_summary;\n-- \n1.9.1.295.g2661741\n"},{"id":"237188","messageId":"xmqq38iczrr4.fsf@gitster.dls.corp.google.com","threadId":"36248","inReplyTo":"1395317561-9380-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] status: disable translation when --porcelain is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-20T17:47:43Z","receivedAt":"2014-03-20T17:47:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> \"git status --branch --porcelain\" displays the status of the branch\n> (ahead, behind, gone), and used gettext to translate the string.\n>\n> Use hardcoded strings when --porcelain is used, but keep the gettext\n> translation for \"git status --short\" which is essentially the same, but\n> meant to be read by a human.\n>\n> Reported-by: Anarky <ghostanarky@gmail.com>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n>> The porcelain format of git status is described as not based on user\n>> configuration.\n>> But with --branch, behind/ahead are translated following the user's locale.\n>> Is it normal that scripts need to take care of that?\n>\n> Indeed, I'd call that a bug. Here's a fix.\n\nGood thing to fix.  Thanks.\n\n>  wt-status.c | 15 ++++++++++-----\n>  wt-status.h |  1 +\n>  2 files changed, 11 insertions(+), 5 deletions(-)\n>\n> diff --git a/wt-status.c b/wt-status.c\n> index a452407..e55e5b9 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n>  \t\treturn;\n>  \t}\n>  \n> +\tconst char *gone   = s->no_gettext ? \"gone\"   : _(\"gone\");\n> +\tconst char *behind = s->no_gettext ? \"behind \" : _(\"behind \");\n> +\tconst char *ahead  = s->no_gettext ? \"ahead \"  : _(\"ahead \");\n\nHaving to repeat the same string constant twice (and a half for the\nvariable name) each is an eyesore.  I wonder if we can do better,\nperhaps with:\n\n#define LABEL(string) (s->no_gettext ? (string) : _(string))\n\nand then\n\n\tcolor_fprintf(s->fp, header_color, LABEL(N_(\"gone\")));\n\nor something along those lines?\n\n>  \tcolor_fprintf(s->fp, header_color, \" [\");\n>  \tif (upstream_is_gone) {\n> -\t\tcolor_fprintf(s->fp, header_color, _(\"gone\"));\n> +\t\tcolor_fprintf(s->fp, header_color, gone);\n>  \t} else if (!num_ours) {\n> -\t\tcolor_fprintf(s->fp, header_color, _(\"behind \"));\n> +\t\tcolor_fprintf(s->fp, header_color, behind);\n>  \t\tcolor_fprintf(s->fp, branch_color_remote, \"%d\", num_theirs);\n>  \t} else if (!num_theirs) {\n> -\t\tcolor_fprintf(s->fp, header_color, _(\"ahead \"));\n> +\t\tcolor_fprintf(s->fp, header_color, ahead);\n>  \t\tcolor_fprintf(s->fp, branch_color_local, \"%d\", num_ours);\n>  \t} else {\n> -\t\tcolor_fprintf(s->fp, header_color, _(\"ahead \"));\n> +\t\tcolor_fprintf(s->fp, header_color, ahead);\n>  \t\tcolor_fprintf(s->fp, branch_color_local, \"%d\", num_ours);\n> -\t\tcolor_fprintf(s->fp, header_color, _(\", behind \"));\n> +\t\tcolor_fprintf(s->fp, header_color, \", %s\", behind);\n>  \t\tcolor_fprintf(s->fp, branch_color_remote, \"%d\", num_theirs);\n>  \t}\n>  \n> @@ -1566,5 +1570,6 @@ void wt_porcelain_print(struct wt_status *s)\n>  \ts->use_color = 0;\n>  \ts->relative_paths = 0;\n>  \ts->prefix = NULL;\n> +\ts->no_gettext = 1;\n>  \twt_shortstatus_print(s);\n>  }\n> diff --git a/wt-status.h b/wt-status.h\n> index 30a4812..82f6ce6 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -50,6 +50,7 @@ struct wt_status {\n>  \tenum commit_whence whence;\n>  \tint nowarn;\n>  \tint use_color;\n> +\tint no_gettext;\n>  \tint display_comment_prefix;\n>  \tint relative_paths;\n>  \tint submodule_summary;\n"},{"id":"237305","messageId":"vpqior8dke6.fsf@anie.imag.fr","threadId":"36248","inReplyTo":"xmqq38iczrr4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] status: disable translation when --porcelain is used","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-03-21T08:29:53Z","receivedAt":"2014-03-21T08:29:53Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> diff --git a/wt-status.c b/wt-status.c\n>> index a452407..e55e5b9 100644\n>> --- a/wt-status.c\n>> +++ b/wt-status.c\n>> @@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n>>  \t\treturn;\n>>  \t}\n>>  \n>> +\tconst char *gone   = s->no_gettext ? \"gone\"   : _(\"gone\");\n>> +\tconst char *behind = s->no_gettext ? \"behind \" : _(\"behind \");\n>> +\tconst char *ahead  = s->no_gettext ? \"ahead \"  : _(\"ahead \");\n>\n> Having to repeat the same string constant twice (and a half for the\n> variable name) each is an eyesore.  I wonder if we can do better,\n> perhaps with:\n>\n> #define LABEL(string) (s->no_gettext ? (string) : _(string))\n>\n> and then\n>\n> \tcolor_fprintf(s->fp, header_color, LABEL(N_(\"gone\")));\n>\n> or something along those lines?\n\nI first thought about trying something clever with the preprocessor, but\nsince it's only for 3 strings, I went the KISS way. I tend to prefer my\nversion for simplicity, but no strong opinion here.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"237330","messageId":"xmqqd2hfv4e1.fsf@gitster.dls.corp.google.com","threadId":"36248","inReplyTo":"vpqior8dke6.fsf@anie.imag.fr","subject":"Re: [PATCH] status: disable translation when --porcelain is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-21T17:38:14Z","receivedAt":"2014-03-21T17:38:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>>> diff --git a/wt-status.c b/wt-status.c\n>>> index a452407..e55e5b9 100644\n>>> --- a/wt-status.c\n>>> +++ b/wt-status.c\n>>> @@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n>>>  \t\treturn;\n>>>  \t}\n>>>  \n>>> +\tconst char *gone   = s->no_gettext ? \"gone\"   : _(\"gone\");\n>>> +\tconst char *behind = s->no_gettext ? \"behind \" : _(\"behind \");\n>>> +\tconst char *ahead  = s->no_gettext ? \"ahead \"  : _(\"ahead \");\n>>\n>> Having to repeat the same string constant twice (and a half for the\n>> variable name) each is an eyesore.  I wonder if we can do better,\n>> perhaps with:\n>>\n>> #define LABEL(string) (s->no_gettext ? (string) : _(string))\n>>\n>> and then\n>>\n>> \tcolor_fprintf(s->fp, header_color, LABEL(N_(\"gone\")));\n>>\n>> or something along those lines?\n>\n> I first thought about trying something clever with the preprocessor, but\n> since it's only for 3 strings, I went the KISS way. I tend to prefer my\n> version for simplicity, but no strong opinion here.\n\nThen I'll squash 61bf9709 (SQUASH??? fix decl-after-stmt and\nsimplify, 2014-03-20) in before merging the patch to 'next'.\n"}]}