{"thread":{"id":"35553","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","startedAt":"2013-12-19T19:43:19Z","lastAt":"2014-03-13T18:01:52Z","messageCount":12,"participants":["Jonathan Nieder","Junio C Hamano","Duy Nguyen","Raphael Hertzog","Sandy Carter"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"232251","messageId":"20131219194319.GB20443@google.com","threadId":"35553","inReplyTo":"20131219104613.GA18379@x230-buxy.home.ouaza.com","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-12-19T19:43:19Z","receivedAt":"2013-12-19T19:43:19Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Raphaël Hertzog wrote[1]:\n\n> Here's an example of the problematic output:\n[...]\n> #\tmodifié des deux côtés :debian/control\n\nThanks for the ping, and sorry to leave this hanging before.\n\n[...]\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -255,7 +255,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s,\n>  \tcase 6: how = _(\"both added:\"); break;\n>  \tcase 7: how = _(\"both modified:\"); break;\n>  \t}\n> -\tstatus_printf_more(s, c, \"%-20s%s\\n\", how, one);\n> +\tstatus_printf_more(s, c, \"%-19s %s\\n\", how, one);\n\nIt looks like the original code is assuming that\n\n (1) the number of bytes written is the width of a string, so they can\n     line up\n\n (2) the \"how\" string is always <= 19 bytes\n\nBoth assumptions are problematic.\n\nPerhaps the value '20' should be dynamic (e.g.,\n\n\tmin(20,\n\t    1 + max(utf8_strwidth(all 'how' strings\n\t                          in the current UI language)))\n\n) to allow the text to always line up?  Ever since 3651e45c\n(wt-status: take the alignment burden off translators, 2013-11-05),\nwt_status_print_change_data() picks its width dynamically for the same\nreason.  So, something like the following (untested)?\n\nThis includes the colon in the translated string, to make it easier to\nremember to keep the non-breaking space before it.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n\n[1] http://bugs.debian.org/725777\n\ndiff --git i/wt-status.c w/wt-status.c\nindex 4e55810..7b0e5b8 100644\n--- i/wt-status.c\n+++ w/wt-status.c\n@@ -245,27 +245,26 @@ static void wt_status_print_trailer(struct wt_status *s)\n \n #define quote_path quote_path_relative\n \n-static void wt_status_print_unmerged_data(struct wt_status *s,\n-\t\t\t\t\t  struct string_list_item *it)\n+static const char *wt_status_unmerged_status_string(int stagemask)\n {\n-\tconst char *c = color(WT_STATUS_UNMERGED, s);\n-\tstruct wt_status_change_data *d = it->util;\n-\tstruct strbuf onebuf = STRBUF_INIT;\n-\tconst char *one, *how = _(\"bug\");\n-\n-\tone = quote_path(it->string, s->prefix, &onebuf);\n-\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n-\tswitch (d->stagemask) {\n-\tcase 1: how = _(\"both deleted:\"); break;\n-\tcase 2: how = _(\"added by us:\"); break;\n-\tcase 3: how = _(\"deleted by them:\"); break;\n-\tcase 4: how = _(\"added by them:\"); break;\n-\tcase 5: how = _(\"deleted by us:\"); break;\n-\tcase 6: how = _(\"both added:\"); break;\n-\tcase 7: how = _(\"both modified:\"); break;\n+\tswitch (stagemask) {\n+\tcase 1:\n+\t\treturn _(\"both deleted:\");\n+\tcase 2:\n+\t\treturn _(\"added by us:\");\n+\tcase 3:\n+\t\treturn _(\"deleted by them:\");\n+\tcase 4:\n+\t\treturn _(\"added by them:\");\n+\tcase 5:\n+\t\treturn _(\"deleted by us:\");\n+\tcase 6:\n+\t\treturn _(\"both added:\");\n+\tcase 7:\n+\t\treturn _(\"both modified:\");\n+\tdefault:\n+\t\treturn NULL;\n \t}\n-\tstatus_printf_more(s, c, \"%-20s%s\\n\", how, one);\n-\tstrbuf_release(&onebuf);\n }\n \n static const char *wt_status_diff_status_string(int status)\n@@ -292,6 +291,48 @@ static const char *wt_status_diff_status_string(int status)\n \t}\n }\n \n+static int maxwidth(const char *(*label)(int), int minval, int maxval)\n+{\n+\tconst char *s;\n+\tint result = 0, i;\n+\n+\tfor (i = minval; i <= maxval; i++) {\n+\t\tconst char *s = label(i);\n+\t\tint len = s ? strlen(s) : 0;\n+\t\tif (len > result)\n+\t\t\tresult = len;\n+\t}\n+\treturn result;\n+}\n+\n+static void wt_status_print_unmerged_data(struct wt_status *s,\n+\t\t\t\t\t  struct string_list_item *it)\n+{\n+\tconst char *c = color(WT_STATUS_UNMERGED, s);\n+\tstruct wt_status_change_data *d = it->util;\n+\tstruct strbuf onebuf = STRBUF_INIT;\n+\tstatic char *padding;\n+\tconst char *one, *how;\n+\tint len;\n+\n+\tif (!padding) {\n+\t\tint width = maxwidth(wt_status_unmerged_status_string, 1, 7);\n+\t\twidth += strlen(\" \");\n+\t\tpadding = xmallocz(width);\n+\t\tmemset(padding, ' ', width);\n+\t}\n+\n+\tone = quote_path(it->string, s->prefix, &onebuf);\n+\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n+\n+\thow = wt_status_unmerged_status_string(d->stagemask);\n+\tif (!how)\n+\t\thow = _(\"bug\");\n+\tlen = strlen(padding) - utf8_strwidth(how);\n+\tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n+\tstrbuf_release(&onebuf);\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@@ -309,14 +350,8 @@ static void wt_status_print_change_data(struct wt_status *s,\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\t/* If DIFF_STATUS_* uses outside the range [A..Z], we're in trouble */\n+\t\tint width = maxwidth(wt_status_diff_status_string, 'A', 'Z');\n \t\twidth += 2;\t/* colon and a space */\n \t\tpadding = xmallocz(width);\n \t\tmemset(padding, ' ', width);\n"},{"id":"232259","messageId":"xmqqk3f0efhe.fsf@gitster.dls.corp.google.com","threadId":"35553","inReplyTo":"20131219194319.GB20443@google.com","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-19T20:46:21Z","receivedAt":"2013-12-19T20:46:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> This includes the colon in the translated string, to make it easier to\n> remember to keep the non-breaking space before it.\n\nHmph, recent 3651e45c (wt-status: take the alignment burden off\ntranslators, 2013-11-05) seems to have gone in the different\ndirection when it updated similar code for the non-unmerged paths.\n\n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n>\n> [1] http://bugs.debian.org/725777\n>\n> diff --git i/wt-status.c w/wt-status.c\n> index 4e55810..7b0e5b8 100644\n> --- i/wt-status.c\n> +++ w/wt-status.c\n> @@ -245,27 +245,26 @@ static void wt_status_print_trailer(struct wt_status *s)\n>  \n>  #define quote_path quote_path_relative\n>  \n> -static void wt_status_print_unmerged_data(struct wt_status *s,\n> -\t\t\t\t\t  struct string_list_item *it)\n> +static const char *wt_status_unmerged_status_string(int stagemask)\n>  {\n> -\tconst char *c = color(WT_STATUS_UNMERGED, s);\n> -\tstruct wt_status_change_data *d = it->util;\n> -\tstruct strbuf onebuf = STRBUF_INIT;\n> -\tconst char *one, *how = _(\"bug\");\n> -\n> -\tone = quote_path(it->string, s->prefix, &onebuf);\n> -\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n> -\tswitch (d->stagemask) {\n> -\tcase 1: how = _(\"both deleted:\"); break;\n> -\tcase 2: how = _(\"added by us:\"); break;\n> -\tcase 3: how = _(\"deleted by them:\"); break;\n> -\tcase 4: how = _(\"added by them:\"); break;\n> -\tcase 5: how = _(\"deleted by us:\"); break;\n> -\tcase 6: how = _(\"both added:\"); break;\n> -\tcase 7: how = _(\"both modified:\"); break;\n> +\tswitch (stagemask) {\n> +\tcase 1:\n> +\t\treturn _(\"both deleted:\");\n> +\tcase 2:\n> +\t\treturn _(\"added by us:\");\n> +\tcase 3:\n> +\t\treturn _(\"deleted by them:\");\n> +\tcase 4:\n> +\t\treturn _(\"added by them:\");\n> +\tcase 5:\n> +\t\treturn _(\"deleted by us:\");\n> +\tcase 6:\n> +\t\treturn _(\"both added:\");\n> +\tcase 7:\n> +\t\treturn _(\"both modified:\");\n> +\tdefault:\n> +\t\treturn NULL;\n>  \t}\n> -\tstatus_printf_more(s, c, \"%-20s%s\\n\", how, one);\n> -\tstrbuf_release(&onebuf);\n>  }\n>  \n>  static const char *wt_status_diff_status_string(int status)\n> @@ -292,6 +291,48 @@ static const char *wt_status_diff_status_string(int status)\n>  \t}\n>  }\n>  \n> +static int maxwidth(const char *(*label)(int), int minval, int maxval)\n> +{\n> +\tconst char *s;\n> +\tint result = 0, i;\n> +\n> +\tfor (i = minval; i <= maxval; i++) {\n> +\t\tconst char *s = label(i);\n> +\t\tint len = s ? strlen(s) : 0;\n> +\t\tif (len > result)\n> +\t\t\tresult = len;\n> +\t}\n> +\treturn result;\n> +}\n> +\n> +static void wt_status_print_unmerged_data(struct wt_status *s,\n> +\t\t\t\t\t  struct string_list_item *it)\n> +{\n> +\tconst char *c = color(WT_STATUS_UNMERGED, s);\n> +\tstruct wt_status_change_data *d = it->util;\n> +\tstruct strbuf onebuf = STRBUF_INIT;\n> +\tstatic char *padding;\n> +\tconst char *one, *how;\n> +\tint len;\n> +\n> +\tif (!padding) {\n> +\t\tint width = maxwidth(wt_status_unmerged_status_string, 1, 7);\n> +\t\twidth += strlen(\" \");\n> +\t\tpadding = xmallocz(width);\n> +\t\tmemset(padding, ' ', width);\n> +\t}\n> +\n> +\tone = quote_path(it->string, s->prefix, &onebuf);\n> +\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n> +\n> +\thow = wt_status_unmerged_status_string(d->stagemask);\n> +\tif (!how)\n> +\t\thow = _(\"bug\");\n> +\tlen = strlen(padding) - utf8_strwidth(how);\n> +\tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n> +\tstrbuf_release(&onebuf);\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> @@ -309,14 +350,8 @@ static void wt_status_print_change_data(struct wt_status *s,\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\t/* If DIFF_STATUS_* uses outside the range [A..Z], we're in trouble */\n> +\t\tint width = maxwidth(wt_status_diff_status_string, 'A', 'Z');\n>  \t\twidth += 2;\t/* colon and a space */\n>  \t\tpadding = xmallocz(width);\n>  \t\tmemset(padding, ' ', width);\n"},{"id":"232260","messageId":"20131219205049.GE20443@google.com","threadId":"35553","inReplyTo":"xmqqk3f0efhe.fsf@gitster.dls.corp.google.com","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-12-19T20:50:49Z","receivedAt":"2013-12-19T20:50:49Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> This includes the colon in the translated string, to make it easier to\n>> remember to keep the non-breaking space before it.\n>\n> Hmph, recent 3651e45c (wt-status: take the alignment burden off\n> translators, 2013-11-05) seems to have gone in the different\n> direction when it updated similar code for the non-unmerged paths.\n\nYes, if this seems to go in the right direction, I'd add a follow-up\nfor that when rerolling.\n\nAlternatively if there is some library function to append a colon to a\nstring in a locale-appropriate way, that could work, too.  Pointers\nwelcome.\n\nThanks,\nJonathan\n"},{"id":"232274","messageId":"CACsJy8Cw+YpG3C8PFuBOddDWLTBZB=bHDFBtTwYzLO=1a15LuA@mail.gmail.com","threadId":"35553","inReplyTo":"20131219205049.GE20443@google.com","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-19T23:59:31Z","receivedAt":"2013-12-19T23:59:31Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Dec 20, 2013 at 3:50 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Junio C Hamano wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>>> This includes the colon in the translated string, to make it easier to\n>>> remember to keep the non-breaking space before it.\n>>\n>> Hmph, recent 3651e45c (wt-status: take the alignment burden off\n>> translators, 2013-11-05) seems to have gone in the different\n>> direction when it updated similar code for the non-unmerged paths.\n>\n> Yes, if this seems to go in the right direction, I'd add a follow-up\n> for that when rerolling.\n\ni'm fine either way.\n-- \nDuy\n"},{"id":"233231","messageId":"20140116220045.GA18773@x230-buxy.home.ouaza.com","threadId":"35553","inReplyTo":"CACsJy8Cw+YpG3C8PFuBOddDWLTBZB=bHDFBtTwYzLO=1a15LuA@mail.gmail.com","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","fromName":"Raphael Hertzog","fromEmail":"hertzog@debian.org","sentAt":"2014-01-16T22:00:45Z","receivedAt":"2014-01-16T22:00:45Z","isPatch":false,"sender":{"key":"hertzog@debian.org","avatar":"https://avatars.githubusercontent.com/u/1013915?v=4"},"body":"Hi,\n\nOn Fri, 20 Dec 2013, Duy Nguyen wrote:\n> On Fri, Dec 20, 2013 at 3:50 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> > Junio C Hamano wrote:\n> >> Jonathan Nieder <jrnieder@gmail.com> writes:\n> >\n> >>> This includes the colon in the translated string, to make it easier to\n> >>> remember to keep the non-breaking space before it.\n> >>\n> >> Hmph, recent 3651e45c (wt-status: take the alignment burden off\n> >> translators, 2013-11-05) seems to have gone in the different\n> >> direction when it updated similar code for the non-unmerged paths.\n> >\n> > Yes, if this seems to go in the right direction, I'd add a follow-up\n> > for that when rerolling.\n> \n> i'm fine either way.\n\nIn both cases, the alignment burden is taken off the translators. But I\nagree that it's best to keep the colon in the translatable string so that\ntranslators can add the space required by proper typography.\n\nSo I'd favor Jonathan's approach. I just tested his patch and it\nsolves the immediate problem for me. Without the patch I have this\n(French translation):\n\n| Chemins non fusionnés :\n|   (utilisez \"git add <fichier>...\" pour marquer comme résolu)\n| \n| \tmodifié des deux côtés :fichier\n\nAnd with it:\n\n| Chemins non fusionnés :\n|   (utilisez \"git add <fichier>...\" pour marquer comme résolu)\n| \n| \tmodifié des deux côtés :     fichier\n\nHowever it looks like the padding is calculated on bytes\nand not on (wide) characters, so we have 3 extra spaces before\nthe filename (one for ô, é, and the non-breaking space).\nThis can be problematic for translations where all characters\nrequire multiple bytes. :-)\n\nThe patch is also incomplete since it currently breaks\nthe test suite in multiple places (the unnecessary padding\nbefore the filename goes away), for example:\n\n--- expected\t2014-01-16 21:37:11.270535950 +0000\n+++ actual\t2014-01-16 21:37:11.270535950 +0000\n@@ -5,7 +5,7 @@\n Unmerged paths:\n   (use \"git add/rm <file>...\" as appropriate to mark resolution)\n \n-\tboth added:         conflict.txt\n-\tdeleted by them:    main.txt\n+\tboth added:      conflict.txt\n+\tdeleted by them: main.txt\n \n no changes added to commit (use \"git add\" and/or \"git commit -a\")\nnot ok 11 - status when conflicts with add and rm advice (deleted by them)\n\n\nThat was my two euros to help get this fixed because it annoys me\nquite often.\n\nCheers,\n\n-- \nRaphaël Hertzog ◈ Debian Developer\n\nDiscover the Debian Administrator's Handbook:\n→ http://debian-handbook.info/get/\n"},{"id":"234502","messageId":"CACsJy8AvnYsWCxwJbiSVtva7Evbn9Y4sPi3jgav1Ke2GirBQvQ@mail.gmail.com","threadId":"35553","inReplyTo":"20131219205049.GE20443@google.com","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-08T09:51:47Z","receivedAt":"2014-02-08T09:51:47Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Dec 20, 2013 at 3:50 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Junio C Hamano wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>>> This includes the colon in the translated string, to make it easier to\n>>> remember to keep the non-breaking space before it.\n>>\n>> Hmph, recent 3651e45c (wt-status: take the alignment burden off\n>> translators, 2013-11-05) seems to have gone in the different\n>> direction when it updated similar code for the non-unmerged paths.\n>\n> Yes, if this seems to go in the right direction, I'd add a follow-up\n> for that when rerolling.\n\nJust checking. Did you reroll it or did my filters move some mails to\nspam/trash folder again?\n-- \nDuy\n"},{"id":"236602","messageId":"xmqqa9cvmeod.fsf@gitster.dls.corp.google.com","threadId":"35553","inReplyTo":"20131219194319.GB20443@google.com","subject":"Re: git: problematic git status output with some translations (such as fr_FR)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-12T18:53:38Z","receivedAt":"2014-03-12T18:53:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> @@ -292,6 +291,48 @@ static const char *wt_status_diff_status_string(int status)\n>  \t}\n>  }\n>  \n> +static int maxwidth(const char *(*label)(int), int minval, int maxval)\n> +{\n> +\tconst char *s;\n> +\tint result = 0, i;\n> +\n> +\tfor (i = minval; i <= maxval; i++) {\n> +\t\tconst char *s = label(i);\n> +\t\tint len = s ? strlen(s) : 0;\n\nShouldn't this be a utf8_strwidth(), as the value is to count number\nof display columns to be used by the leading label part?\n\n> +\t\tif (len > result)\n> +\t\t\tresult = len;\n> +\t}\n> +\treturn result;\n> +}\n> +\n> +static void wt_status_print_unmerged_data(struct wt_status *s,\n> +\t\t\t\t\t  struct string_list_item *it)\n> +{\n> +\tconst char *c = color(WT_STATUS_UNMERGED, s);\n> +\tstruct wt_status_change_data *d = it->util;\n> +\tstruct strbuf onebuf = STRBUF_INIT;\n> +\tstatic char *padding;\n> +\tconst char *one, *how;\n> +\tint len;\n> +\n> +\tif (!padding) {\n> +\t\tint width = maxwidth(wt_status_unmerged_status_string, 1, 7);\n> +\t\twidth += strlen(\" \");\n> +\t\tpadding = xmallocz(width);\n> +\t\tmemset(padding, ' ', width);\n> +\t}\n> +\n> +\tone = quote_path(it->string, s->prefix, &onebuf);\n> +\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n> +\n> +\thow = wt_status_unmerged_status_string(d->stagemask);\n> +\tif (!how)\n> +\t\thow = _(\"bug\");\n\nI'd rather see the callee do this _(\"bug\") thing, not this\nparticular caller.\n\n> +\tlen = strlen(padding) - utf8_strwidth(how);\n> +\tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n> +\tstrbuf_release(&onebuf);\n> +}\n"},{"id":"236607","messageId":"xmqqtxb3kyr2.fsf_-_@gitster.dls.corp.google.com","threadId":"35553","inReplyTo":"xmqqa9cvmeod.fsf@gitster.dls.corp.google.com","subject":"[PATCH] wt-status: i18n of section labels","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-12T19:22:57Z","receivedAt":"2014-03-12T19:22:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Jonathan Nieder <jrnieder@gmail.com>\nDate: Thu, 19 Dec 11:43:19 2013 -0800\n\nThe original code assumes that:\n\n (1) the number of bytes written is the width of a string, so they\n     can line up;\n\n (2) the \"how\" string is always <= 19 bytes.\n\nAlso a recent change to a similar codepath by 3651e45c (wt-status:\ntake the alignment burden off translators, 2013-11-05) adds this\nassumption:\n\n (3) the \"colon\" at the end of the label string does not have to be\n     subject to l10n.\n\nNone of which we should assume.\n\nRefactor the code to compute the label width for both codepaths into\na helper function, make sure label strings have the trailing colon\nthat can be localized, and use it when showing the section labels in\nthe output.\n\ncf. http://bugs.debian.org/725777\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Differences relative to Jonathan's original in $gmane/239537 are:\n\n   - labels made by wt_status_diff_status_string() have trailing\n     colon; the caller has been adjusted (please double check)\n\n   - a static label_width avoids repeated strlen(padding);\n\n   - unmerged status label has \"at least 20 columns wide\"\n     reinstated, at least for now, to keep the existing tests from\n     breaking.  We may want to drop it in a separate follow-up\n     patch.\n\n wt-status.c | 121 +++++++++++++++++++++++++++++++++++++++---------------------\n 1 file changed, 78 insertions(+), 43 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 4e55810..a00861c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -245,51 +245,92 @@ static void wt_status_print_trailer(struct wt_status *s)\n \n #define quote_path quote_path_relative\n \n-static void wt_status_print_unmerged_data(struct wt_status *s,\n-\t\t\t\t\t  struct string_list_item *it)\n+static const char *wt_status_unmerged_status_string(int stagemask)\n {\n-\tconst char *c = color(WT_STATUS_UNMERGED, s);\n-\tstruct wt_status_change_data *d = it->util;\n-\tstruct strbuf onebuf = STRBUF_INIT;\n-\tconst char *one, *how = _(\"bug\");\n-\n-\tone = quote_path(it->string, s->prefix, &onebuf);\n-\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n-\tswitch (d->stagemask) {\n-\tcase 1: how = _(\"both deleted:\"); break;\n-\tcase 2: how = _(\"added by us:\"); break;\n-\tcase 3: how = _(\"deleted by them:\"); break;\n-\tcase 4: how = _(\"added by them:\"); break;\n-\tcase 5: how = _(\"deleted by us:\"); break;\n-\tcase 6: how = _(\"both added:\"); break;\n-\tcase 7: how = _(\"both modified:\"); break;\n+\tswitch (stagemask) {\n+\tcase 1:\n+\t\treturn _(\"both deleted:\");\n+\tcase 2:\n+\t\treturn _(\"added by us:\");\n+\tcase 3:\n+\t\treturn _(\"deleted by them:\");\n+\tcase 4:\n+\t\treturn _(\"added by them:\");\n+\tcase 5:\n+\t\treturn _(\"deleted by us:\");\n+\tcase 6:\n+\t\treturn _(\"both added:\");\n+\tcase 7:\n+\t\treturn _(\"both modified:\");\n+\tdefault:\n+\t\treturn _(\"bug\");\n \t}\n-\tstatus_printf_more(s, c, \"%-20s%s\\n\", how, one);\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+\t\treturn _(\"new file:\");\n \tcase DIFF_STATUS_COPIED:\n-\t\treturn _(\"copied\");\n+\t\treturn _(\"copied:\");\n \tcase DIFF_STATUS_DELETED:\n-\t\treturn _(\"deleted\");\n+\t\treturn _(\"deleted:\");\n \tcase DIFF_STATUS_MODIFIED:\n-\t\treturn _(\"modified\");\n+\t\treturn _(\"modified:\");\n \tcase DIFF_STATUS_RENAMED:\n-\t\treturn _(\"renamed\");\n+\t\treturn _(\"renamed:\");\n \tcase DIFF_STATUS_TYPE_CHANGED:\n-\t\treturn _(\"typechange\");\n+\t\treturn _(\"typechange:\");\n \tcase DIFF_STATUS_UNKNOWN:\n-\t\treturn _(\"unknown\");\n+\t\treturn _(\"unknown:\");\n \tcase DIFF_STATUS_UNMERGED:\n-\t\treturn _(\"unmerged\");\n+\t\treturn _(\"unmerged:\");\n \tdefault:\n-\t\treturn NULL;\n+\t\treturn _(\"bug\");\n+\t}\n+}\n+\n+static int maxwidth(const char *(*label)(int), int minval, int maxval)\n+{\n+\tint result = 0, i;\n+\n+\tfor (i = minval; i <= maxval; i++) {\n+\t\tconst char *s = label(i);\n+\t\tint len = s ? utf8_strwidth(s) : 0;\n+\t\tif (len > result)\n+\t\t\tresult = len;\n+\t}\n+\treturn result;\n+}\n+\n+static void wt_status_print_unmerged_data(struct wt_status *s,\n+\t\t\t\t\t  struct string_list_item *it)\n+{\n+\tconst char *c = color(WT_STATUS_UNMERGED, s);\n+\tstruct wt_status_change_data *d = it->util;\n+\tstruct strbuf onebuf = STRBUF_INIT;\n+\tstatic char *padding;\n+\tstatic int label_width;\n+\tconst char *one, *how;\n+\tint len;\n+\n+\tif (!padding) {\n+\t\tlabel_width = maxwidth(wt_status_unmerged_status_string, 1, 7);\n+\t\tlabel_width += strlen(\" \");\n+\t\tif (label_width < 20)\n+\t\t\tlabel_width = 20;\n+\t\tpadding = xmallocz(label_width);\n+\t\tmemset(padding, ' ', label_width);\n \t}\n+\n+\tone = quote_path(it->string, s->prefix, &onebuf);\n+\tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n+\n+\thow = wt_status_unmerged_status_string(d->stagemask);\n+\tlen = label_width - utf8_strwidth(how);\n+\tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n+\tstrbuf_release(&onebuf);\n }\n \n static void wt_status_print_change_data(struct wt_status *s,\n@@ -305,21 +346,16 @@ static void wt_status_print_change_data(struct wt_status *s,\n \tstruct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;\n \tstruct strbuf extra = STRBUF_INIT;\n \tstatic char *padding;\n+\tstatic int label_width;\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\t/* If DIFF_STATUS_* uses outside the range [A..Z], we're in trouble */\n+\t\tlabel_width = maxwidth(wt_status_diff_status_string, 'A', 'Z');\n+\t\tlabel_width += strlen(\" \");\n+\t\tpadding = xmallocz(label_width);\n+\t\tmemset(padding, ' ', label_width);\n \t}\n \n \tone_name = two_name = it->string;\n@@ -355,14 +391,13 @@ static void wt_status_print_change_data(struct wt_status *s,\n \twhat = wt_status_diff_status_string(status);\n \tif (!what)\n \t\tdie(_(\"bug: unhandled diff status %c\"), status);\n-\t/* 1 for colon, which is not part of \"what\" */\n-\tlen = strlen(padding) - (utf8_strwidth(what) + 1);\n+\tlen = label_width - utf8_strwidth(what);\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\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\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"},{"id":"236616","messageId":"5320BE1C.9020307@savoirfairelinux.com","threadId":"35553","inReplyTo":"xmqqtxb3kyr2.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [PATCH] wt-status: i18n of section labels","fromName":"Sandy Carter","fromEmail":"sandy.carter@savoirfairelinux.com","sentAt":"2014-03-12T20:05:48Z","receivedAt":"2014-03-12T20:05:48Z","isPatch":true,"sender":{"key":"sandy.carter@savoirfairelinux.com","avatar":"https://gravatar.com/avatar/f7200eeb1d898456afeba0045b3db9fc44992c1a1472c467883d32528048cd3e?d=mp&s=160"},"body":"\n\nLe 2014-03-12 15:22, Junio C Hamano a écrit :\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> +\t\treturn _(\"new file:\");\n>   \tcase DIFF_STATUS_COPIED:\n> -\t\treturn _(\"copied\");\n> +\t\treturn _(\"copied:\");\n>   \tcase DIFF_STATUS_DELETED:\n> -\t\treturn _(\"deleted\");\n> +\t\treturn _(\"deleted:\");\n>   \tcase DIFF_STATUS_MODIFIED:\n> -\t\treturn _(\"modified\");\n> +\t\treturn _(\"modified:\");\n>   \tcase DIFF_STATUS_RENAMED:\n> -\t\treturn _(\"renamed\");\n> +\t\treturn _(\"renamed:\");\n>   \tcase DIFF_STATUS_TYPE_CHANGED:\n> -\t\treturn _(\"typechange\");\n> +\t\treturn _(\"typechange:\");\n>   \tcase DIFF_STATUS_UNKNOWN:\n> -\t\treturn _(\"unknown\");\n> +\t\treturn _(\"unknown:\");\n>   \tcase DIFF_STATUS_UNMERGED:\n> -\t\treturn _(\"unmerged\");\n> +\t\treturn _(\"unmerged:\");\n>   \tdefault:\n> -\t\treturn NULL;\n> +\t\treturn _(\"bug\");\n> +\t}\n> +}\n\nI don't see why _(\"bug\") is returned when, later down,\n\n> @@ -305,21 +346,16 @@ static void wt_status_print_change_data(struct wt_status *s,\n>   \tstruct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;\n>   \tstruct strbuf extra = STRBUF_INIT;\n>   \tstatic char *padding;\n> +\tstatic int label_width;\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\nchecks for NULL.\n"},{"id":"236622","messageId":"xmqq1ty7kwfp.fsf@gitster.dls.corp.google.com","threadId":"35553","inReplyTo":"5320BE1C.9020307@savoirfairelinux.com","subject":"Re: [PATCH] wt-status: i18n of section labels","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-12T20:12:58Z","receivedAt":"2014-03-12T20:12:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sandy Carter <sandy.carter@savoirfairelinux.com> writes:\n\n> Le 2014-03-12 15:22, Junio C Hamano a écrit :\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>> +\t\treturn _(\"new file:\");\n>>   \tcase DIFF_STATUS_COPIED:\n>> -\t\treturn _(\"copied\");\n>> +\t\treturn _(\"copied:\");\n>>   \tcase DIFF_STATUS_DELETED:\n>> -\t\treturn _(\"deleted\");\n>> +\t\treturn _(\"deleted:\");\n>>   \tcase DIFF_STATUS_MODIFIED:\n>> -\t\treturn _(\"modified\");\n>> +\t\treturn _(\"modified:\");\n>>   \tcase DIFF_STATUS_RENAMED:\n>> -\t\treturn _(\"renamed\");\n>> +\t\treturn _(\"renamed:\");\n>>   \tcase DIFF_STATUS_TYPE_CHANGED:\n>> -\t\treturn _(\"typechange\");\n>> +\t\treturn _(\"typechange:\");\n>>   \tcase DIFF_STATUS_UNKNOWN:\n>> -\t\treturn _(\"unknown\");\n>> +\t\treturn _(\"unknown:\");\n>>   \tcase DIFF_STATUS_UNMERGED:\n>> -\t\treturn _(\"unmerged\");\n>> +\t\treturn _(\"unmerged:\");\n>>   \tdefault:\n>> -\t\treturn NULL;\n>> +\t\treturn _(\"bug\");\n>> +\t}\n>> +}\n>\n> I don't see why _(\"bug\") is returned when, later down,\n\nWhen there is a bug in the caller.\n\n>\n>> @@ -305,21 +346,16 @@ static void wt_status_print_change_data(struct wt_status *s,\n>>   \tstruct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;\n>>   \tstruct strbuf extra = STRBUF_INIT;\n>>   \tstatic char *padding;\n>> +\tstatic int label_width;\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>\n> checks for NULL.\n\nThat extra NULL-ness check can go, I think.  Thanks for\ndouble-checking.\n"},{"id":"236623","messageId":"5320C0DD.1090803@savoirfairelinux.com","threadId":"35553","inReplyTo":"xmqq1ty7kwfp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] wt-status: i18n of section labels","fromName":"Sandy Carter","fromEmail":"sandy.carter@savoirfairelinux.com","sentAt":"2014-03-12T20:17:33Z","receivedAt":"2014-03-12T20:17:33Z","isPatch":true,"sender":{"key":"sandy.carter@savoirfairelinux.com","avatar":"https://gravatar.com/avatar/f7200eeb1d898456afeba0045b3db9fc44992c1a1472c467883d32528048cd3e?d=mp&s=160"},"body":"Le 2014-03-12 16:12, Junio C Hamano a écrit :\n> Sandy Carter <sandy.carter@savoirfairelinux.com> writes:\n>\n>> Le 2014-03-12 15:22, Junio C Hamano a écrit :\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>>> +\t\treturn _(\"new file:\");\n>>>    \tcase DIFF_STATUS_COPIED:\n>>> -\t\treturn _(\"copied\");\n>>> +\t\treturn _(\"copied:\");\n>>>    \tcase DIFF_STATUS_DELETED:\n>>> -\t\treturn _(\"deleted\");\n>>> +\t\treturn _(\"deleted:\");\n>>>    \tcase DIFF_STATUS_MODIFIED:\n>>> -\t\treturn _(\"modified\");\n>>> +\t\treturn _(\"modified:\");\n>>>    \tcase DIFF_STATUS_RENAMED:\n>>> -\t\treturn _(\"renamed\");\n>>> +\t\treturn _(\"renamed:\");\n>>>    \tcase DIFF_STATUS_TYPE_CHANGED:\n>>> -\t\treturn _(\"typechange\");\n>>> +\t\treturn _(\"typechange:\");\n>>>    \tcase DIFF_STATUS_UNKNOWN:\n>>> -\t\treturn _(\"unknown\");\n>>> +\t\treturn _(\"unknown:\");\n>>>    \tcase DIFF_STATUS_UNMERGED:\n>>> -\t\treturn _(\"unmerged\");\n>>> +\t\treturn _(\"unmerged:\");\n>>>    \tdefault:\n>>> -\t\treturn NULL;\n>>> +\t\treturn _(\"bug\");\n>>> +\t}\n>>> +}\n>>\n>> I don't see why _(\"bug\") is returned when, later down,\n>\n> When there is a bug in the caller.\n>\n>>\n>>> @@ -305,21 +346,16 @@ static void wt_status_print_change_data(struct wt_status *s,\n>>>    \tstruct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;\n>>>    \tstruct strbuf extra = STRBUF_INIT;\n>>>    \tstatic char *padding;\n>>> +\tstatic int label_width;\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>>\n>> checks for NULL.\n>\n> That extra NULL-ness check can go, I think.  Thanks for\n> double-checking.\n>\n\nI refered to the wrong lines, the ones I was refering to were:\n\n > +static int maxwidth(const char *(*label)(int), int minval, int maxval)\n > +{\n > +\tint result = 0, i;\n > +\n > +\tfor (i = minval; i <= maxval; i++) {\n > +\t\tconst char *s = label(i);\n > +\t\tint len = s ? utf8_strwidth(s) : 0;\n\nSorry about that\n"},{"id":"236679","messageId":"xmqqvbviht9r.fsf@gitster.dls.corp.google.com","threadId":"35553","inReplyTo":"5320C0DD.1090803@savoirfairelinux.com","subject":"Re: [PATCH] wt-status: i18n of section labels","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-13T18:01:52Z","receivedAt":"2014-03-13T18:01:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sandy Carter <sandy.carter@savoirfairelinux.com> writes:\n\n> I refered to the wrong lines, the ones I was refering to were:\n>\n>> +static int maxwidth(const char *(*label)(int), int minval, int maxval)\n>> +{\n>> +\tint result = 0, i;\n>> +\n>> +\tfor (i = minval; i <= maxval; i++) {\n>> +\t\tconst char *s = label(i);\n>> +\t\tint len = s ? utf8_strwidth(s) : 0;\n>\n> Sorry about that\n\nOh, yes, you are right.  wt_status_diff_status_string() is meant to\nbe asked with a bogus status character and expected to return NULL,\nso diagnosing anything it does not understand as a \"bug\" is indeed a\nbug I added.\n\nI think I fixed in my latest reroll ($gmane/243996).\n\nThanks for catching that.\n"}]}