Re: [PATCH] i18n: move the trailing space out of translatable strings

2013-12-09 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> I've got this with Vietnamese translation
>
> $ git status
> HEAD được tách rời từorigin/master
>
> One does not need to understand Vietnamese to see that a space is
> missing before "origin/master"...

Not really.  Only if one guesses (or knows) that Vietnamese is among
the languages that use SP to separate words.  There are languages
that do not use inter-word spaces.

Because those languages are in the minority, it would be easier to
unconditionally add SP after translatable string regardless of the
l10n target languages, but I am afraid that it is going in a wrong
direction in the longer term.

>   clean_print_color(CLEAN_COLOR_PROMPT);
> - printf(_("Input ignore patterns>> "));
> + printf("%s ", _("Input ignore patterns>>"));
>   clean_print_color(CLEAN_COLOR_RESET);

As this space after the prompt may be different from inter-word
space after all, this one may probably be on the borderline.

> @@ -754,7 +754,8 @@ static int ask_each_cmd(void)
>   /* Ctrl-D should stop removing files */
>   if (!eof) {
>   qname = quote_path_relative(item->string, NULL, &buf);
> - printf(_("remove %s? "), qname);
> + printf(_("remove %s?"), qname);
> + putchar(' ');

As is this change.

But it is an example that can be used to illustrate my earlier
point.  The l10n target language may want to have _no_ space between
words, i.e. to turn the above into:

printf("%sを削除しますか?", qname);
putchar(' ');

i.e. no space between the object of the verb ("the path being
removed") and the verb ("to remove"), while still wanting to have SP
between the prompt and its answer like everybody else.  And having
SP after "remove" in the gettext key _is_ a good thing, as l10n
people can choose not to have any SP between the verb and its
object.

> diff --git a/builtin/clone.c b/builtin/clone.c
> index 874e0fd..d48ccee 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,
>  
>   if (check_connectivity) {
>   if (transport->progress)
> - fprintf(stderr, _("Checking connectivity... "));
> + fprintf(stderr, _("Checking connectivity..."));
>   if (check_everything_connected_with_transport(iterate_ref_map,
> 0, &rm, 
> transport))
>   die(_("remote did not send all necessary objects"));
>   if (transport->progress)
> - fprintf(stderr, _("done.\n"));
> + fprintf(stderr, " %s", _("done.\n"));

But I think that this changes enforces the inter-word SP policy that
could go against convention by specific l10n target languages.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i18n: move the trailing space out of translatable strings

2013-12-07 Thread Keshav Kini
The following message is a courtesy copy of an article
that has been posted to gmane.comp.version-control.git as well.

Duy Nguyen  writes:
> On Sun, Dec 8, 2013 at 9:11 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> I could fix vi.po alone, but it'd be better to avoid similar mistakes
>> for all translations by moving the trailing space out of all
>> translatable strings (*). This is inline with how we handle newlines
>> (e.g. *printf_ln wrappers) but it's not widespread enough to make new
>> *printf_space wrappers.
>
> And I just realized spaces are more common in languages using latin
> alphabet but are not a rule. CJK languages do not have/need spaces so
> this might be a wrong move..

There are already other issues as well.  The strings also seem to be
assuming certain word orders.  For example:

> - on_what = _("HEAD detached at ");
> + on_what = _("HEAD detached at");

Both versions of this assume that the location at which HEAD was
detached should come at the end of the sentence, whereas in
rightward-headed languages such as Japanese it would be more natural to
put the location at the middle or beginning of the sentence (while it is
possible to rewrite the text a bit to force the location to come at the
end).

I think the best practice is to just have one long string per "message"
the program is supposed to display, and only display that string with %s
substituted for data, rather than printing that string and then printing
the data, or printing the data and then printing the string, or
whatever.  With printf() in C, the %s can even be reordered wrt to the
calling order with a special syntax, "%m$", if necessary, as I just
learned from `man 3 printf`...

-Keshav
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i18n: move the trailing space out of translatable strings

2013-12-07 Thread Duy Nguyen
On Sun, Dec 8, 2013 at 9:11 AM, Nguyễn Thái Ngọc Duy  wrote:
> I could fix vi.po alone, but it'd be better to avoid similar mistakes
> for all translations by moving the trailing space out of all
> translatable strings (*). This is inline with how we handle newlines
> (e.g. *printf_ln wrappers) but it's not widespread enough to make new
> *printf_space wrappers.

And I just realized spaces are more common in languages using latin
alphabet but are not a rule. CJK languages do not have/need spaces so
this might be a wrong move..
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] i18n: move the trailing space out of translatable strings

2013-12-07 Thread Nguyễn Thái Ngọc Duy
I've got this with Vietnamese translation

$ git status
HEAD được tách rời từorigin/master

One does not need to understand Vietnamese to see that a space is
missing before "origin/master" and this is because the original string
has a trailing space, but the translated one omits it.

I could fix vi.po alone, but it'd be better to avoid similar mistakes
for all translations by moving the trailing space out of all
translatable strings (*). This is inline with how we handle newlines
(e.g. *printf_ln wrappers) but it's not widespread enough to make new
*printf_space wrappers.

(*) the strings are detected by

make pot; msgcat --no-wrap po/git.pot|grep 'msgid.* "$'

and if you do it after this patch, you will see maybe 3 matched lines
from git-bisect.sh and git-am.sh that I didn't bother to fix.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 BTW "msgcat...|grep 'msgid.*\n"$'" gives 159 matches. Low hanging
 fruit..

 builtin/clean.c |  5 +++--
 builtin/clone.c |  4 ++--
 wt-status.c | 26 +-
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 615cd57..2b7e694 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -656,7 +656,7 @@ static int filter_by_patterns_cmd(void)
pretty_print_dels();
 
clean_print_color(CLEAN_COLOR_PROMPT);
-   printf(_("Input ignore patterns>> "));
+   printf("%s ", _("Input ignore patterns>>"));
clean_print_color(CLEAN_COLOR_RESET);
if (strbuf_getline(&confirm, stdin, '\n') != EOF)
strbuf_trim(&confirm);
@@ -754,7 +754,8 @@ static int ask_each_cmd(void)
/* Ctrl-D should stop removing files */
if (!eof) {
qname = quote_path_relative(item->string, NULL, &buf);
-   printf(_("remove %s? "), qname);
+   printf(_("remove %s?"), qname);
+   putchar(' ');
if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
strbuf_trim(&confirm);
} else {
diff --git a/builtin/clone.c b/builtin/clone.c
index 874e0fd..d48ccee 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,
 
if (check_connectivity) {
if (transport->progress)
-   fprintf(stderr, _("Checking connectivity... "));
+   fprintf(stderr, _("Checking connectivity..."));
if (check_everything_connected_with_transport(iterate_ref_map,
  0, &rm, 
transport))
die(_("remote did not send all necessary objects"));
if (transport->progress)
-   fprintf(stderr, _("done.\n"));
+   fprintf(stderr, " %s", _("done.\n"));
}
 
if (refs) {
diff --git a/wt-status.c b/wt-status.c
index 4625cdb..3637656 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -330,11 +330,11 @@ static void wt_status_print_change_data(struct wt_status 
*s,
if (d->new_submodule_commits || d->dirty_submodule) {
strbuf_addstr(&extra, " (");
if (d->new_submodule_commits)
-   strbuf_addf(&extra, _("new commits, "));
+   strbuf_addf(&extra, "%s, ", _("new commits"));
if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
-   strbuf_addf(&extra, _("modified content, "));
+   strbuf_addf(&extra, "%s, ", _("modified 
content"));
if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
-   strbuf_addf(&extra, _("untracked content, "));
+   strbuf_addf(&extra, "%s, ", _("untracked 
content"));
strbuf_setlen(&extra, extra.len - 2);
strbuf_addch(&extra, ')');
}
@@ -1244,30 +1244,30 @@ void wt_status_print(struct wt_status *s)
s->branch && !strcmp(s->branch, "HEAD"));
 
if (s->branch) {
-   const char *on_what = _("On branch ");
+   const char *on_what = _("On branch");
const char *branch_name = s->branch;
if (!prefixcmp(branch_name, "refs/heads/"))
branch_name += 11;
else if (!strcmp(branch_name, "HEAD")) {
branch_status_color = color(WT_STATUS_NOBRANCH, s);
if (state.rebase_in_progress || 
state.rebase_interactive_in_progress) {
-   on_what = _("rebase in progress; onto ");
+   on_what = _("rebase in progress; onto");
b