{"thread":{"id":"35268","subject":"git status: minor output format error","startedAt":"2013-11-03T17:17:39Z","lastAt":"2013-11-05T02:07:29Z","messageCount":6,"participants":["Wolfgang Rohdewald","Duy Nguyen","Nguyễn Thái Ngọc Duy","Ralf Thielow","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"230062","messageId":"31465687.35grSWXxkx@i5","threadId":"35268","inReplyTo":null,"subject":"git status: minor output format error","fromName":"Wolfgang Rohdewald","fromEmail":"wolfgang@rohdewald.de","sentAt":"2013-11-03T17:17:39Z","receivedAt":"2013-11-03T17:17:39Z","isPatch":false,"sender":{"key":"wolfgang@rohdewald.de","avatar":null},"body":"git version 1.8.3.2\n\ngit status in German says (extract)\n\n#       geändert:   kajongg.py\n#       gelöscht:    playfield.py\n\nas you can see, there is one space too much before playfield.py\n\nLANG=C git status is correct:\n#       modified:   kajongg.py\n#       deleted:    playfield.py\n\nso it seems the spacing between the columns expects \"deleted\" to have the same number\nof letters in all languages.\n\n-- \nWolfgang\n"},{"id":"230073","messageId":"CACsJy8DyX-cxJNYe3HXeb8e96O36h4aEJrBo+P5035b-vuz0hw@mail.gmail.com","threadId":"35268","inReplyTo":"31465687.35grSWXxkx@i5","subject":"Re: git status: minor output format error","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-04T01:46:01Z","receivedAt":"2013-11-04T01:46:01Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Nov 4, 2013 at 12:17 AM, Wolfgang Rohdewald\n<wolfgang@rohdewald.de> wrote:\n> git version 1.8.3.2\n>\n> git status in German says (extract)\n>\n> #       geändert:   kajongg.py\n> #       gelöscht:    playfield.py\n>\n> as you can see, there is one space too much before playfield.py\n>\n> LANG=C git status is correct:\n> #       modified:   kajongg.py\n> #       deleted:    playfield.py\n>\n> so it seems the spacing between the columns expects \"deleted\" to have the same number\n> of letters in all languages.\n\nNo, the translations control the number of columns in this case\n(although it's not very intuitive for translators). Something like\nthis may fix it. I haven't tested because I don't have \"de\" locale\ninstalled.\n\ndiff --git a/po/de.po b/po/de.po\nindex 35a44b9..d1846d2 100644\n--- a/po/de.po\n+++ b/po/de.po\n@@ -1404,7 +1404,7 @@ msgstr \"kopiert:     %s -> %s\"\n #: wt-status.c:313\n #, c-format\n msgid \"deleted:    %s\"\n-msgstr \"gelöscht:    %s\"\n+msgstr \"gelöscht:   %s\"\n\n #: wt-status.c:316\n #, c-format\n-- \nDuy\n"},{"id":"230074","messageId":"1383534531-5153-1-git-send-email-pclouds@gmail.com","threadId":"35268","inReplyTo":"31465687.35grSWXxkx@i5","subject":"[PATCH] wt-status: take the alignment burden off translators","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-04T03:08:51Z","receivedAt":"2013-11-04T03:08:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It's not easy for translators to see spaces in these strings have to\nalign, especially when there are no guarantees that these strings are\ngrouped together in .po files.\n\nRefactor the code and do the alignment automatically. Let the\ntranslator specify how big the alignment is, though, in case some\nlanguages need more than 12 columns for the preceding text.\n\nNoticed-by: Wolfgang Rohdewald <wolfgang@rohdewald.de>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Ralf and Quan, don't update your translations yet. If this change\n gets merged it'll simplify your task a bit.\n\n wt-status.c | 51 +++++++++++++++++++++++++++++++++++++++++++--------\n 1 file changed, 43 insertions(+), 8 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex b4e44ba..f9715d3 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -15,6 +15,7 @@\n #include \"submodule.h\"\n #include \"column.h\"\n #include \"strbuf.h\"\n+#include \"utf8.h\"\n \n static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n@@ -276,6 +277,10 @@ static void wt_status_print_change_data(struct wt_status *s,\n \tconst char *one, *two;\n \tstruct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;\n \tstruct strbuf extra = STRBUF_INIT;\n+\tconst char *what;\n+\tstatic const char *spaces = \"                                \";\n+\tstatic int width = -1;\n+\tint len;\n \n \tone_name = two_name = it->string;\n \tswitch (change_type) {\n@@ -309,32 +314,62 @@ static void wt_status_print_change_data(struct wt_status *s,\n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \tswitch (status) {\n \tcase DIFF_STATUS_ADDED:\n-\t\tstatus_printf_more(s, c, _(\"new file:   %s\"), one);\n+\t\twhat = _(\"new file\");\n \t\tbreak;\n \tcase DIFF_STATUS_COPIED:\n-\t\tstatus_printf_more(s, c, _(\"copied:     %s -> %s\"), one, two);\n+\t\twhat = _(\"copied\");\n \t\tbreak;\n \tcase DIFF_STATUS_DELETED:\n-\t\tstatus_printf_more(s, c, _(\"deleted:    %s\"), one);\n+\t\twhat = _(\"deleted\");\n \t\tbreak;\n \tcase DIFF_STATUS_MODIFIED:\n-\t\tstatus_printf_more(s, c, _(\"modified:   %s\"), one);\n+\t\twhat = _(\"modified\");\n \t\tbreak;\n \tcase DIFF_STATUS_RENAMED:\n-\t\tstatus_printf_more(s, c, _(\"renamed:    %s -> %s\"), one, two);\n+\t\twhat = _(\"renamed\");\n \t\tbreak;\n \tcase DIFF_STATUS_TYPE_CHANGED:\n-\t\tstatus_printf_more(s, c, _(\"typechange: %s\"), one);\n+\t\twhat = _(\"typechange\");\n \t\tbreak;\n \tcase DIFF_STATUS_UNKNOWN:\n-\t\tstatus_printf_more(s, c, _(\"unknown:    %s\"), one);\n+\t\twhat = _(\"unknown\");\n \t\tbreak;\n \tcase DIFF_STATUS_UNMERGED:\n-\t\tstatus_printf_more(s, c, _(\"unmerged:   %s\"), one);\n+\t\twhat = _(\"unmerged\");\n \t\tbreak;\n \tdefault:\n \t\tdie(_(\"bug: unhandled diff status %c\"), status);\n \t}\n+\tif (width == -1) {\n+\t\t/*\n+\t\t * Translators: if you do translate this, replace it\n+\t\t * with a decimal number of how many columns needed\n+\t\t * to align file names in \"git status\":\n+\t\t *\n+\t\t * |<-columns->|\n+\t\t *\n+\t\t *   unmerged: foo.c\n+\t\t *   copied:   foo.c -> bar.c\n+\t\t *\n+\t\t * The default value is 12. Normally you would not\n+\t\t * need to translate this at all unless the translated\n+\t\t * strings are longer than 12 columns and therefore\n+\t\t * break alignment.\n+\t\t */\n+\t\twidth = atoi(_(\"<wt-status.c:width>\"));\n+\t\tif (width <= 0 || width > 32)\n+\t\t\twidth = 12;\n+\t}\n+\tif (width > utf8_strwidth(what) + 1)\n+\t\tlen = width - utf8_strwidth(what) - 1;\n+\telse\n+\t\tlen = 0;\n+\tif (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)\n+\t\tstatus_printf_more(s, c, \"%s:%.*s%s -> %s\",\n+\t\t\t\t   what, len, spaces, one, two);\n+\telse\n+\t\tstatus_printf_more(s, c, \"%s:%.*s%s\",\n+\t\t\t\t   what, len, spaces, one);\n \tif (extra.len) {\n \t\tstatus_printf_more(s, color(WT_STATUS_HEADER, s), \"%s\", extra.buf);\n \t\tstrbuf_release(&extra);\n-- \n1.8.2.83.gc99314b\n"},{"id":"230076","messageId":"CAN0XMOK3NqcAyk5c246+CCDZETxGfsZ_AxbBEmUC0RX9aXW8mw@mail.gmail.com","threadId":"35268","inReplyTo":"CACsJy8DyX-cxJNYe3HXeb8e96O36h4aEJrBo+P5035b-vuz0hw@mail.gmail.com","subject":"Re: git status: minor output format error","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2013-11-04T07:53:00Z","receivedAt":"2013-11-04T07:53:00Z","isPatch":false,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"On Mon, Nov 4, 2013 at 2:46 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Mon, Nov 4, 2013 at 12:17 AM, Wolfgang Rohdewald\n> <wolfgang@rohdewald.de> wrote:\n>> git version 1.8.3.2\n>>\n>> git status in German says (extract)\n>>\n>> #       geändert:   kajongg.py\n>> #       gelöscht:    playfield.py\n>>\n>> as you can see, there is one space too much before playfield.py\n>>\n>> LANG=C git status is correct:\n>> #       modified:   kajongg.py\n>> #       deleted:    playfield.py\n>>\n>> so it seems the spacing between the columns expects \"deleted\" to have the same number\n>> of letters in all languages.\n>\n> No, the translations control the number of columns in this case\n> (although it's not very intuitive for translators). Something like\n> this may fix it. I haven't tested because I don't have \"de\" locale\n> installed.\n>\n> diff --git a/po/de.po b/po/de.po\n> index 35a44b9..d1846d2 100644\n> --- a/po/de.po\n> +++ b/po/de.po\n> @@ -1404,7 +1404,7 @@ msgstr \"kopiert:     %s -> %s\"\n>  #: wt-status.c:313\n>  #, c-format\n>  msgid \"deleted:    %s\"\n> -msgstr \"gelöscht:    %s\"\n> +msgstr \"gelöscht:   %s\"\n>\n\nThe columns don't match in other related messages, either.\nLet's see what happens with [1].\n\n[1]\nhttp://article.gmane.org/gmane.comp.version-control.git/237283\n\n>  #: wt-status.c:316\n>  #, c-format\n> --\n> Duy\n"},{"id":"230083","messageId":"xmqqppqgypnd.fsf@gitster.dls.corp.google.com","threadId":"35268","inReplyTo":"1383534531-5153-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] wt-status: take the alignment burden off translators","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-04T18:42:46Z","receivedAt":"2013-11-04T18:42:46Z","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> +\tstatic const char *spaces = \"                                \";\n\nI do not see anything that limits \"len\" used to show only show the\nprefix bytes in this array not to ask for more than you have spaces\nhere.  It won't overrun the end of this array, but I suspect your\nresult will not align when that happens.\n\n> +\tstatic int width = -1;\n> +\tint len;\n>  \n>  \tone_name = two_name = it->string;\n>  \tswitch (change_type) {\n> @@ -309,32 +314,62 @@ static void wt_status_print_change_data(struct wt_status *s,\n>  \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n>  \tswitch (status) {\n>  \tcase DIFF_STATUS_ADDED:\n> -\t\tstatus_printf_more(s, c, _(\"new file:   %s\"), one);\n> +\t\twhat = _(\"new file\");\n>  \t\tbreak;\n>  \tcase DIFF_STATUS_COPIED:\n> -\t\tstatus_printf_more(s, c, _(\"copied:     %s -> %s\"), one, two);\n> +\t\twhat = _(\"copied\");\n>  \t\tbreak;\n>  \tcase DIFF_STATUS_DELETED:\n> -\t\tstatus_printf_more(s, c, _(\"deleted:    %s\"), one);\n> +\t\twhat = _(\"deleted\");\n>  \t\tbreak;\n>  \tcase DIFF_STATUS_MODIFIED:\n> -\t\tstatus_printf_more(s, c, _(\"modified:   %s\"), one);\n> +\t\twhat = _(\"modified\");\n>  \t\tbreak;\n>  \tcase DIFF_STATUS_RENAMED:\n> -\t\tstatus_printf_more(s, c, _(\"renamed:    %s -> %s\"), one, two);\n> +\t\twhat = _(\"renamed\");\n>  \t\tbreak;\n>  \tcase DIFF_STATUS_TYPE_CHANGED:\n> -\t\tstatus_printf_more(s, c, _(\"typechange: %s\"), one);\n> +\t\twhat = _(\"typechange\");\n>  \t\tbreak;\n>  \tcase DIFF_STATUS_UNKNOWN:\n> -\t\tstatus_printf_more(s, c, _(\"unknown:    %s\"), one);\n> +\t\twhat = _(\"unknown\");\n>  \t\tbreak;\n>  \tcase DIFF_STATUS_UNMERGED:\n> -\t\tstatus_printf_more(s, c, _(\"unmerged:   %s\"), one);\n> +\t\twhat = _(\"unmerged\");\n>  \t\tbreak;\n>  \tdefault:\n>  \t\tdie(_(\"bug: unhandled diff status %c\"), status);\n>  \t}\n> +\tif (width == -1) {\n> +\t\t/*\n> +\t\t * Translators: if you do translate this, replace it\n> +\t\t * with a decimal number of how many columns needed\n> +\t\t * to align file names in \"git status\":\n> +\t\t *\n> +\t\t * |<-columns->|\n> +\t\t *\n> +\t\t *   unmerged: foo.c\n> +\t\t *   copied:   foo.c -> bar.c\n> +\t\t *\n> +\t\t * The default value is 12. Normally you would not\n> +\t\t * need to translate this at all unless the translated\n> +\t\t * strings are longer than 12 columns and therefore\n> +\t\t * break alignment.\n> +\t\t */\n> +\t\twidth = atoi(_(\"<wt-status.c:width>\"));\n\nSomewhat ugly.  Wouldn't it work better to keep the \"what\" strings\nin a static array and compute \"width\" just once?  Is the sparseness\nof DIFF_STATUS_* enums what makes such an implementation cumbersome?\n\nI dunno.\n\n> +\t\tif (width <= 0 || width > 32)\n> +\t\t\twidth = 12;\n> +\t}\n> +\tif (width > utf8_strwidth(what) + 1)\n> +\t\tlen = width - utf8_strwidth(what) - 1;\n\nJust a style, but this is far easier to read if you said:\n\n\tif (width > utf8_strwidth(what) + 1)\n\t\tlen = width - (utf8_strwidth(what) + 1);\n\nbecause you are essentially doing:\n\n\tif (A > B)\n        \tx = A - B;\n\nOr even\n\n\tlen = width - (utf8_strwidth(what) + 1);\n        if (len < 0)\n        \tlen = 0;\n\n> +\telse\n> +\t\tlen = 0;\n> +\tif (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)\n> +\t\tstatus_printf_more(s, c, \"%s:%.*s%s -> %s\",\n> +\t\t\t\t   what, len, spaces, one, two);\n> +\telse\n> +\t\tstatus_printf_more(s, c, \"%s:%.*s%s\",\n> +\t\t\t\t   what, len, spaces, one);\n>  \tif (extra.len) {\n>  \t\tstatus_printf_more(s, color(WT_STATUS_HEADER, s), \"%s\", extra.buf);\n>  \t\tstrbuf_release(&extra);\n"},{"id":"230107","messageId":"1383617249-13360-1-git-send-email-pclouds@gmail.com","threadId":"35268","inReplyTo":"1383534531-5153-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] wt-status: take the alignment burden off translators","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-05T02:07:29Z","receivedAt":"2013-11-05T02:07:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It's not easy for translators to see spaces in these strings have to\nalign, especially when there are no guarantees that these strings are\ngrouped together in .po files. Refactor the code and do the alignment\nautomatically.\n\nNoticed-by: Wolfgang Rohdewald <wolfgang@rohdewald.de>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n v2 calculates maximum length automatically too so the translator does\n not have to test and specify that manually.\n\n wt-status.c | 80 ++++++++++++++++++++++++++++++++++++++++---------------------\n 1 file changed, 53 insertions(+), 27 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex b4e44ba..4625cdb 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -15,6 +15,7 @@\n #include \"submodule.h\"\n #include \"column.h\"\n #include \"strbuf.h\"\n+#include \"utf8.h\"\n \n static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n@@ -264,6 +265,30 @@ static void wt_status_print_unmerged_data(struct wt_status *s,\n \tstrbuf_release(&onebuf);\n }\n \n+static const char *wt_status_diff_status_string(int status)\n+{\n+\tswitch (status) {\n+\tcase DIFF_STATUS_ADDED:\n+\t\treturn _(\"new file\");\n+\tcase DIFF_STATUS_COPIED:\n+\t\treturn _(\"copied\");\n+\tcase DIFF_STATUS_DELETED:\n+\t\treturn _(\"deleted\");\n+\tcase DIFF_STATUS_MODIFIED:\n+\t\treturn _(\"modified\");\n+\tcase DIFF_STATUS_RENAMED:\n+\t\treturn _(\"renamed\");\n+\tcase DIFF_STATUS_TYPE_CHANGED:\n+\t\treturn _(\"typechange\");\n+\tcase DIFF_STATUS_UNKNOWN:\n+\t\treturn _(\"unknown\");\n+\tcase DIFF_STATUS_UNMERGED:\n+\t\treturn _(\"unmerged\");\n+\tdefault:\n+\t\treturn NULL;\n+\t}\n+}\n+\n static void wt_status_print_change_data(struct wt_status *s,\n \t\t\t\t\tint change_type,\n \t\t\t\t\tstruct string_list_item *it)\n@@ -276,6 +301,23 @@ static void wt_status_print_change_data(struct wt_status *s,\n \tconst char *one, *two;\n \tstruct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;\n \tstruct strbuf extra = STRBUF_INIT;\n+\tstatic char *padding;\n+\tconst char *what;\n+\tint len;\n+\n+\tif (!padding) {\n+\t\tint width = 0;\n+\t\t/* If DIFF_STATUS_* uses outside this range, we're in trouble */\n+\t\tfor (status = 'A'; status <= 'Z'; status++) {\n+\t\t\twhat = wt_status_diff_status_string(status);\n+\t\t\tlen = what ? strlen(what) : 0;\n+\t\t\tif (len > width)\n+\t\t\t\twidth = len;\n+\t\t}\n+\t\twidth += 2;\t/* colon and a space */\n+\t\tpadding = xmallocz(width);\n+\t\tmemset(padding, ' ', width);\n+\t}\n \n \tone_name = two_name = it->string;\n \tswitch (change_type) {\n@@ -307,34 +349,18 @@ static void wt_status_print_change_data(struct wt_status *s,\n \ttwo = quote_path(two_name, s->prefix, &twobuf);\n \n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n-\tswitch (status) {\n-\tcase DIFF_STATUS_ADDED:\n-\t\tstatus_printf_more(s, c, _(\"new file:   %s\"), one);\n-\t\tbreak;\n-\tcase DIFF_STATUS_COPIED:\n-\t\tstatus_printf_more(s, c, _(\"copied:     %s -> %s\"), one, two);\n-\t\tbreak;\n-\tcase DIFF_STATUS_DELETED:\n-\t\tstatus_printf_more(s, c, _(\"deleted:    %s\"), one);\n-\t\tbreak;\n-\tcase DIFF_STATUS_MODIFIED:\n-\t\tstatus_printf_more(s, c, _(\"modified:   %s\"), one);\n-\t\tbreak;\n-\tcase DIFF_STATUS_RENAMED:\n-\t\tstatus_printf_more(s, c, _(\"renamed:    %s -> %s\"), one, two);\n-\t\tbreak;\n-\tcase DIFF_STATUS_TYPE_CHANGED:\n-\t\tstatus_printf_more(s, c, _(\"typechange: %s\"), one);\n-\t\tbreak;\n-\tcase DIFF_STATUS_UNKNOWN:\n-\t\tstatus_printf_more(s, c, _(\"unknown:    %s\"), one);\n-\t\tbreak;\n-\tcase DIFF_STATUS_UNMERGED:\n-\t\tstatus_printf_more(s, c, _(\"unmerged:   %s\"), one);\n-\t\tbreak;\n-\tdefault:\n+\twhat = wt_status_diff_status_string(status);\n+\tif (!what)\n \t\tdie(_(\"bug: unhandled diff status %c\"), status);\n-\t}\n+\t/* 1 for colon, which is not part of \"what\" */\n+\tlen = strlen(padding) - (utf8_strwidth(what) + 1);\n+\tassert(len >= 0);\n+\tif (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)\n+\t\tstatus_printf_more(s, c, \"%s:%.*s%s -> %s\",\n+\t\t\t\t   what, len, padding, one, two);\n+\telse\n+\t\tstatus_printf_more(s, c, \"%s:%.*s%s\",\n+\t\t\t\t   what, len, padding, one);\n \tif (extra.len) {\n \t\tstatus_printf_more(s, color(WT_STATUS_HEADER, s), \"%s\", extra.buf);\n \t\tstrbuf_release(&extra);\n-- \n1.8.2.82.gc24b958\n"}]}