{"thread":{"id":"35498","subject":"[PATCH] i18n: move the trailing space out of translatable strings","startedAt":"2013-12-08T02:11:44Z","lastAt":"2013-12-09T19:46:47Z","messageCount":4,"participants":["Nguyễn Thái Ngọc Duy","Duy Nguyen","Keshav Kini","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231751","messageId":"1386468704-18339-1-git-send-email-pclouds@gmail.com","threadId":"35498","inReplyTo":null,"subject":"[PATCH] i18n: move the trailing space out of translatable strings","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-08T02:11:44Z","receivedAt":"2013-12-08T02:11:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I've got this with Vietnamese translation\n\n    $ git status\n    HEAD được tách rời từorigin/master\n\nOne does not need to understand Vietnamese to see that a space is\nmissing before \"origin/master\" and this is because the original string\nhas a trailing space, but the translated one omits it.\n\nI could fix vi.po alone, but it'd be better to avoid similar mistakes\nfor all translations by moving the trailing space out of all\ntranslatable strings (*). This is inline with how we handle newlines\n(e.g. *printf_ln wrappers) but it's not widespread enough to make new\n*printf_space wrappers.\n\n(*) the strings are detected by\n\n    make pot; msgcat --no-wrap po/git.pot|grep 'msgid.* \"$'\n\nand if you do it after this patch, you will see maybe 3 matched lines\nfrom git-bisect.sh and git-am.sh that I didn't bother to fix.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n BTW \"msgcat...|grep 'msgid.*\\n\"$'\" gives 159 matches. Low hanging\n fruit..\n\n builtin/clean.c |  5 +++--\n builtin/clone.c |  4 ++--\n wt-status.c     | 26 +++++++++++++-------------\n 3 files changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 615cd57..2b7e694 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -656,7 +656,7 @@ static int filter_by_patterns_cmd(void)\n \t\t\tpretty_print_dels();\n \n \t\tclean_print_color(CLEAN_COLOR_PROMPT);\n-\t\tprintf(_(\"Input ignore patterns>> \"));\n+\t\tprintf(\"%s \", _(\"Input ignore patterns>>\"));\n \t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF)\n \t\t\tstrbuf_trim(&confirm);\n@@ -754,7 +754,8 @@ static int ask_each_cmd(void)\n \t\t/* Ctrl-D should stop removing files */\n \t\tif (!eof) {\n \t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n-\t\t\tprintf(_(\"remove %s? \"), qname);\n+\t\t\tprintf(_(\"remove %s?\"), qname);\n+\t\t\tputchar(' ');\n \t\t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF) {\n \t\t\t\tstrbuf_trim(&confirm);\n \t\t\t} else {\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 874e0fd..d48ccee 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,\n \n \tif (check_connectivity) {\n \t\tif (transport->progress)\n-\t\t\tfprintf(stderr, _(\"Checking connectivity... \"));\n+\t\t\tfprintf(stderr, _(\"Checking connectivity...\"));\n \t\tif (check_everything_connected_with_transport(iterate_ref_map,\n \t\t\t\t\t\t\t      0, &rm, transport))\n \t\t\tdie(_(\"remote did not send all necessary objects\"));\n \t\tif (transport->progress)\n-\t\t\tfprintf(stderr, _(\"done.\\n\"));\n+\t\t\tfprintf(stderr, \" %s\", _(\"done.\\n\"));\n \t}\n \n \tif (refs) {\ndiff --git a/wt-status.c b/wt-status.c\nindex 4625cdb..3637656 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -330,11 +330,11 @@ static void wt_status_print_change_data(struct wt_status *s,\n \t\tif (d->new_submodule_commits || d->dirty_submodule) {\n \t\t\tstrbuf_addstr(&extra, \" (\");\n \t\t\tif (d->new_submodule_commits)\n-\t\t\t\tstrbuf_addf(&extra, _(\"new commits, \"));\n+\t\t\t\tstrbuf_addf(&extra, \"%s, \", _(\"new commits\"));\n \t\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n-\t\t\t\tstrbuf_addf(&extra, _(\"modified content, \"));\n+\t\t\t\tstrbuf_addf(&extra, \"%s, \", _(\"modified content\"));\n \t\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n-\t\t\t\tstrbuf_addf(&extra, _(\"untracked content, \"));\n+\t\t\t\tstrbuf_addf(&extra, \"%s, \", _(\"untracked content\"));\n \t\t\tstrbuf_setlen(&extra, extra.len - 2);\n \t\t\tstrbuf_addch(&extra, ')');\n \t\t}\n@@ -1244,30 +1244,30 @@ void wt_status_print(struct wt_status *s)\n \t\t\t    s->branch && !strcmp(s->branch, \"HEAD\"));\n \n \tif (s->branch) {\n-\t\tconst char *on_what = _(\"On branch \");\n+\t\tconst char *on_what = _(\"On branch\");\n \t\tconst char *branch_name = s->branch;\n \t\tif (!prefixcmp(branch_name, \"refs/heads/\"))\n \t\t\tbranch_name += 11;\n \t\telse if (!strcmp(branch_name, \"HEAD\")) {\n \t\t\tbranch_status_color = color(WT_STATUS_NOBRANCH, s);\n \t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress) {\n-\t\t\t\ton_what = _(\"rebase in progress; onto \");\n+\t\t\t\ton_what = _(\"rebase in progress; onto\");\n \t\t\t\tbranch_name = state.onto;\n \t\t\t} else if (state.detached_from) {\n \t\t\t\tunsigned char sha1[20];\n \t\t\t\tbranch_name = state.detached_from;\n \t\t\t\tif (!get_sha1(\"HEAD\", sha1) &&\n \t\t\t\t    !hashcmp(sha1, state.detached_sha1))\n-\t\t\t\t\ton_what = _(\"HEAD detached at \");\n+\t\t\t\t\ton_what = _(\"HEAD detached at\");\n \t\t\t\telse\n-\t\t\t\t\ton_what = _(\"HEAD detached from \");\n+\t\t\t\t\ton_what = _(\"HEAD detached from\");\n \t\t\t} else {\n \t\t\t\tbranch_name = \"\";\n \t\t\t\ton_what = _(\"Not currently on any branch.\");\n \t\t\t}\n \t\t}\n \t\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\");\n-\t\tstatus_printf_more(s, branch_status_color, \"%s\", on_what);\n+\t\tstatus_printf_more(s, branch_status_color, \"%s \", on_what);\n \t\tstatus_printf_more(s, branch_color, \"%s\\n\", branch_name);\n \t\tif (!s->is_initial)\n \t\t\twt_status_print_tracking(s);\n@@ -1456,7 +1456,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \n \tbranch = branch_get(s->branch + 11);\n \tif (s->is_initial)\n-\t\tcolor_fprintf(s->fp, header_color, _(\"Initial commit on \"));\n+\t\tcolor_fprintf(s->fp, header_color, \"%s \", _(\"Initial commit on\"));\n \n \tcolor_fprintf(s->fp, branch_color_local, \"%s\", branch_name);\n \n@@ -1488,15 +1488,15 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \tif (upstream_is_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, \"%s \", _(\"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, \"%s \", _(\"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, \"%s \", _(\"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-- \n1.8.5.1.77.g42c48fa\n"},{"id":"231752","messageId":"CACsJy8CBC3qk7NQPR3UWhUdA+o+hXPLPXV+8fz4ctCSV1J2hcA@mail.gmail.com","threadId":"35498","inReplyTo":"1386468704-18339-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] i18n: move the trailing space out of translatable strings","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-08T02:15:25Z","receivedAt":"2013-12-08T02:15:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Dec 8, 2013 at 9:11 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> I could fix vi.po alone, but it'd be better to avoid similar mistakes\n> for all translations by moving the trailing space out of all\n> translatable strings (*). This is inline with how we handle newlines\n> (e.g. *printf_ln wrappers) but it's not widespread enough to make new\n> *printf_space wrappers.\n\nAnd I just realized spaces are more common in languages using latin\nalphabet but are not a rule. CJK languages do not have/need spaces so\nthis might be a wrong move..\n-- \nDuy\n"},{"id":"231753","messageId":"87vbz0f11t.fsf@gmail.com","threadId":"35498","inReplyTo":"CACsJy8CBC3qk7NQPR3UWhUdA+o+hXPLPXV+8fz4ctCSV1J2hcA@mail.gmail.com","subject":"Re: [PATCH] i18n: move the trailing space out of translatable strings","fromName":"Keshav Kini","fromEmail":"keshav.kini@gmail.com","sentAt":"2013-12-08T03:56:30Z","receivedAt":"2013-12-08T03:56:30Z","isPatch":true,"sender":{"key":"keshav.kini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/691290?v=4"},"body":"The following message is a courtesy copy of an article\nthat has been posted to gmane.comp.version-control.git as well.\n\nDuy Nguyen <pclouds@gmail.com> writes:\n> On Sun, Dec 8, 2013 at 9:11 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n>> I could fix vi.po alone, but it'd be better to avoid similar mistakes\n>> for all translations by moving the trailing space out of all\n>> translatable strings (*). This is inline with how we handle newlines\n>> (e.g. *printf_ln wrappers) but it's not widespread enough to make new\n>> *printf_space wrappers.\n>\n> And I just realized spaces are more common in languages using latin\n> alphabet but are not a rule. CJK languages do not have/need spaces so\n> this might be a wrong move..\n\nThere are already other issues as well.  The strings also seem to be\nassuming certain word orders.  For example:\n\n> -\t\t\t\t\ton_what = _(\"HEAD detached at \");\n> +\t\t\t\t\ton_what = _(\"HEAD detached at\");\n\nBoth versions of this assume that the location at which HEAD was\ndetached should come at the end of the sentence, whereas in\nrightward-headed languages such as Japanese it would be more natural to\nput the location at the middle or beginning of the sentence (while it is\npossible to rewrite the text a bit to force the location to come at the\nend).\n\nI think the best practice is to just have one long string per \"message\"\nthe program is supposed to display, and only display that string with %s\nsubstituted for data, rather than printing that string and then printing\nthe data, or printing the data and then printing the string, or\nwhatever.  With printf() in C, the %s can even be reordered wrt to the\ncalling order with a special syntax, \"%m$\", if necessary, as I just\nlearned from `man 3 printf`...\n\n-Keshav\n"},{"id":"231802","messageId":"xmqqeh5lzu1k.fsf@gitster.dls.corp.google.com","threadId":"35498","inReplyTo":"1386468704-18339-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] i18n: move the trailing space out of translatable strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-09T19:46:47Z","receivedAt":"2013-12-09T19:46:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> I've got this with Vietnamese translation\n>\n>     $ git status\n>     HEAD được tách rời từorigin/master\n>\n> One does not need to understand Vietnamese to see that a space is\n> missing before \"origin/master\"...\n\nNot really.  Only if one guesses (or knows) that Vietnamese is among\nthe languages that use SP to separate words.  There are languages\nthat do not use inter-word spaces.\n\nBecause those languages are in the minority, it would be easier to\nunconditionally add SP after translatable string regardless of the\nl10n target languages, but I am afraid that it is going in a wrong\ndirection in the longer term.\n\n>  \t\tclean_print_color(CLEAN_COLOR_PROMPT);\n> -\t\tprintf(_(\"Input ignore patterns>> \"));\n> +\t\tprintf(\"%s \", _(\"Input ignore patterns>>\"));\n>  \t\tclean_print_color(CLEAN_COLOR_RESET);\n\nAs this space after the prompt may be different from inter-word\nspace after all, this one may probably be on the borderline.\n\n> @@ -754,7 +754,8 @@ static int ask_each_cmd(void)\n>  \t\t/* Ctrl-D should stop removing files */\n>  \t\tif (!eof) {\n>  \t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n> -\t\t\tprintf(_(\"remove %s? \"), qname);\n> +\t\t\tprintf(_(\"remove %s?\"), qname);\n> +\t\t\tputchar(' ');\n\nAs is this change.\n\nBut it is an example that can be used to illustrate my earlier\npoint.  The l10n target language may want to have _no_ space between\nwords, i.e. to turn the above into:\n\n\tprintf(\"%sを削除しますか?\", qname);\n        putchar(' ');\n\ni.e. no space between the object of the verb (\"the path being\nremoved\") and the verb (\"to remove\"), while still wanting to have SP\nbetween the prompt and its answer like everybody else.  And having\nSP after \"remove\" in the gettext key _is_ a good thing, as l10n\npeople can choose not to have any SP between the verb and its\nobject.\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 874e0fd..d48ccee 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,\n>  \n>  \tif (check_connectivity) {\n>  \t\tif (transport->progress)\n> -\t\t\tfprintf(stderr, _(\"Checking connectivity... \"));\n> +\t\t\tfprintf(stderr, _(\"Checking connectivity...\"));\n>  \t\tif (check_everything_connected_with_transport(iterate_ref_map,\n>  \t\t\t\t\t\t\t      0, &rm, transport))\n>  \t\t\tdie(_(\"remote did not send all necessary objects\"));\n>  \t\tif (transport->progress)\n> -\t\t\tfprintf(stderr, _(\"done.\\n\"));\n> +\t\t\tfprintf(stderr, \" %s\", _(\"done.\\n\"));\n\nBut I think that this changes enforces the inter-word SP policy that\ncould go against convention by specific l10n target languages.\n"}]}