Re: [PATCH] wt-status: i18n of section labels
Sandy Carter writes: > I refered to the wrong lines, the ones I was refering to were: > >> +static int maxwidth(const char *(*label)(int), int minval, int maxval) >> +{ >> +int result = 0, i; >> + >> +for (i = minval; i <= maxval; i++) { >> +const char *s = label(i); >> +int len = s ? utf8_strwidth(s) : 0; > > Sorry about that Oh, yes, you are right. wt_status_diff_status_string() is meant to be asked with a bogus status character and expected to return NULL, so diagnosing anything it does not understand as a "bug" is indeed a bug I added. I think I fixed in my latest reroll ($gmane/243996). Thanks for catching that. -- 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] wt-status: i18n of section labels
Le 2014-03-12 16:12, Junio C Hamano a écrit : Sandy Carter writes: Le 2014-03-12 15:22, Junio C Hamano a écrit : static const char *wt_status_diff_status_string(int status) { switch (status) { case DIFF_STATUS_ADDED: - return _("new file"); + return _("new file:"); case DIFF_STATUS_COPIED: - return _("copied"); + return _("copied:"); case DIFF_STATUS_DELETED: - return _("deleted"); + return _("deleted:"); case DIFF_STATUS_MODIFIED: - return _("modified"); + return _("modified:"); case DIFF_STATUS_RENAMED: - return _("renamed"); + return _("renamed:"); case DIFF_STATUS_TYPE_CHANGED: - return _("typechange"); + return _("typechange:"); case DIFF_STATUS_UNKNOWN: - return _("unknown"); + return _("unknown:"); case DIFF_STATUS_UNMERGED: - return _("unmerged"); + return _("unmerged:"); default: - return NULL; + return _("bug"); + } +} I don't see why _("bug") is returned when, later down, When there is a bug in the caller. @@ -305,21 +346,16 @@ static void wt_status_print_change_data(struct wt_status *s, struct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT; struct strbuf extra = STRBUF_INIT; static char *padding; + static int label_width; const char *what; int len; if (!padding) { - int width = 0; - /* If DIFF_STATUS_* uses outside this range, we're in trouble */ - for (status = 'A'; status <= 'Z'; status++) { - what = wt_status_diff_status_string(status); - len = what ? strlen(what) : 0; checks for NULL. That extra NULL-ness check can go, I think. Thanks for double-checking. I refered to the wrong lines, the ones I was refering to were: > +static int maxwidth(const char *(*label)(int), int minval, int maxval) > +{ > + int result = 0, i; > + > + for (i = minval; i <= maxval; i++) { > + const char *s = label(i); > + int len = s ? utf8_strwidth(s) : 0; Sorry about that -- 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] wt-status: i18n of section labels
Sandy Carter writes: > Le 2014-03-12 15:22, Junio C Hamano a écrit : >> static const char *wt_status_diff_status_string(int status) >> { >> switch (status) { >> case DIFF_STATUS_ADDED: >> -return _("new file"); >> +return _("new file:"); >> case DIFF_STATUS_COPIED: >> -return _("copied"); >> +return _("copied:"); >> case DIFF_STATUS_DELETED: >> -return _("deleted"); >> +return _("deleted:"); >> case DIFF_STATUS_MODIFIED: >> -return _("modified"); >> +return _("modified:"); >> case DIFF_STATUS_RENAMED: >> -return _("renamed"); >> +return _("renamed:"); >> case DIFF_STATUS_TYPE_CHANGED: >> -return _("typechange"); >> +return _("typechange:"); >> case DIFF_STATUS_UNKNOWN: >> -return _("unknown"); >> +return _("unknown:"); >> case DIFF_STATUS_UNMERGED: >> -return _("unmerged"); >> +return _("unmerged:"); >> default: >> -return NULL; >> +return _("bug"); >> +} >> +} > > I don't see why _("bug") is returned when, later down, When there is a bug in the caller. > >> @@ -305,21 +346,16 @@ static void wt_status_print_change_data(struct >> wt_status *s, >> struct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT; >> struct strbuf extra = STRBUF_INIT; >> static char *padding; >> +static int label_width; >> const char *what; >> int len; >> >> if (!padding) { >> -int width = 0; >> -/* If DIFF_STATUS_* uses outside this range, we're in trouble */ >> -for (status = 'A'; status <= 'Z'; status++) { >> -what = wt_status_diff_status_string(status); >> -len = what ? strlen(what) : 0; > > checks for NULL. That extra NULL-ness check can go, I think. Thanks for double-checking. -- 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] wt-status: i18n of section labels
Le 2014-03-12 15:22, Junio C Hamano a écrit : static const char *wt_status_diff_status_string(int status) { switch (status) { case DIFF_STATUS_ADDED: - return _("new file"); + return _("new file:"); case DIFF_STATUS_COPIED: - return _("copied"); + return _("copied:"); case DIFF_STATUS_DELETED: - return _("deleted"); + return _("deleted:"); case DIFF_STATUS_MODIFIED: - return _("modified"); + return _("modified:"); case DIFF_STATUS_RENAMED: - return _("renamed"); + return _("renamed:"); case DIFF_STATUS_TYPE_CHANGED: - return _("typechange"); + return _("typechange:"); case DIFF_STATUS_UNKNOWN: - return _("unknown"); + return _("unknown:"); case DIFF_STATUS_UNMERGED: - return _("unmerged"); + return _("unmerged:"); default: - return NULL; + return _("bug"); + } +} I don't see why _("bug") is returned when, later down, @@ -305,21 +346,16 @@ static void wt_status_print_change_data(struct wt_status *s, struct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT; struct strbuf extra = STRBUF_INIT; static char *padding; + static int label_width; const char *what; int len; if (!padding) { - int width = 0; - /* If DIFF_STATUS_* uses outside this range, we're in trouble */ - for (status = 'A'; status <= 'Z'; status++) { - what = wt_status_diff_status_string(status); - len = what ? strlen(what) : 0; checks for NULL. -- 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] wt-status: i18n of section labels
From: Jonathan Nieder Date: Thu, 19 Dec 11:43:19 2013 -0800 The original code assumes that: (1) the number of bytes written is the width of a string, so they can line up; (2) the "how" string is always <= 19 bytes. Also a recent change to a similar codepath by 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05) adds this assumption: (3) the "colon" at the end of the label string does not have to be subject to l10n. None of which we should assume. Refactor the code to compute the label width for both codepaths into a helper function, make sure label strings have the trailing colon that can be localized, and use it when showing the section labels in the output. cf. http://bugs.debian.org/725777 Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- * Differences relative to Jonathan's original in $gmane/239537 are: - labels made by wt_status_diff_status_string() have trailing colon; the caller has been adjusted (please double check) - a static label_width avoids repeated strlen(padding); - unmerged status label has "at least 20 columns wide" reinstated, at least for now, to keep the existing tests from breaking. We may want to drop it in a separate follow-up patch. wt-status.c | 121 +++- 1 file changed, 78 insertions(+), 43 deletions(-) diff --git a/wt-status.c b/wt-status.c index 4e55810..a00861c 100644 --- a/wt-status.c +++ b/wt-status.c @@ -245,51 +245,92 @@ static void wt_status_print_trailer(struct wt_status *s) #define quote_path quote_path_relative -static void wt_status_print_unmerged_data(struct wt_status *s, - struct string_list_item *it) +static const char *wt_status_unmerged_status_string(int stagemask) { - const char *c = color(WT_STATUS_UNMERGED, s); - struct wt_status_change_data *d = it->util; - struct strbuf onebuf = STRBUF_INIT; - const char *one, *how = _("bug"); - - one = quote_path(it->string, s->prefix, &onebuf); - status_printf(s, color(WT_STATUS_HEADER, s), "\t"); - switch (d->stagemask) { - case 1: how = _("both deleted:"); break; - case 2: how = _("added by us:"); break; - case 3: how = _("deleted by them:"); break; - case 4: how = _("added by them:"); break; - case 5: how = _("deleted by us:"); break; - case 6: how = _("both added:"); break; - case 7: how = _("both modified:"); break; + switch (stagemask) { + case 1: + return _("both deleted:"); + case 2: + return _("added by us:"); + case 3: + return _("deleted by them:"); + case 4: + return _("added by them:"); + case 5: + return _("deleted by us:"); + case 6: + return _("both added:"); + case 7: + return _("both modified:"); + default: + return _("bug"); } - status_printf_more(s, c, "%-20s%s\n", how, one); - strbuf_release(&onebuf); } static const char *wt_status_diff_status_string(int status) { switch (status) { case DIFF_STATUS_ADDED: - return _("new file"); + return _("new file:"); case DIFF_STATUS_COPIED: - return _("copied"); + return _("copied:"); case DIFF_STATUS_DELETED: - return _("deleted"); + return _("deleted:"); case DIFF_STATUS_MODIFIED: - return _("modified"); + return _("modified:"); case DIFF_STATUS_RENAMED: - return _("renamed"); + return _("renamed:"); case DIFF_STATUS_TYPE_CHANGED: - return _("typechange"); + return _("typechange:"); case DIFF_STATUS_UNKNOWN: - return _("unknown"); + return _("unknown:"); case DIFF_STATUS_UNMERGED: - return _("unmerged"); + return _("unmerged:"); default: - return NULL; + return _("bug"); + } +} + +static int maxwidth(const char *(*label)(int), int minval, int maxval) +{ + int result = 0, i; + + for (i = minval; i <= maxval; i++) { + const char *s = label(i); + int len = s ? utf8_strwidth(s) : 0; + if (len > result) + result = len; + } + return result; +} + +static void wt_status_print_unmerged_data(struct wt_status *s, + struct string_list_item *it) +{ + const char *c = color(WT_STATUS_UNMERGED, s); + struct wt_status_change_data *d = it->util; + struct strbuf onebuf = STRBUF_INIT; + static char *padding; + static int label_width; + const char *one, *how; + int len; + + if (!padding) { + lab