git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Raphael Hertzog <hertzog@debian.org>
Cc: git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: git: problematic git status output with some translations (such as fr_FR)
Date: Thu, 19 Dec 2013 11:43:19 -0800	[thread overview]
Message-ID: <20131219194319.GB20443@google.com> (raw)
In-Reply-To: <20131219104613.GA18379@x230-buxy.home.ouaza.com>

Raphaël Hertzog wrote[1]:

> Here's an example of the problematic output:
[...]
> #	modifié des deux côtés :debian/control

Thanks for the ping, and sorry to leave this hanging before.

[...]
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -255,7 +255,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s,
>  	case 6: how = _("both added:"); break;
>  	case 7: how = _("both modified:"); break;
>  	}
> -	status_printf_more(s, c, "%-20s%s\n", how, one);
> +	status_printf_more(s, c, "%-19s %s\n", how, one);

It looks like the original code is assuming 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

Both assumptions are problematic.

Perhaps the value '20' should be dynamic (e.g.,

	min(20,
	    1 + max(utf8_strwidth(all 'how' strings
	                          in the current UI language)))

) to allow the text to always line up?  Ever since 3651e45c
(wt-status: take the alignment burden off translators, 2013-11-05),
wt_status_print_change_data() picks its width dynamically for the same
reason.  So, something like the following (untested)?

This includes the colon in the translated string, to make it easier to
remember to keep the non-breaking space before it.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

[1] http://bugs.debian.org/725777

diff --git i/wt-status.c w/wt-status.c
index 4e55810..7b0e5b8 100644
--- i/wt-status.c
+++ w/wt-status.c
@@ -245,27 +245,26 @@ 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 NULL;
 	}
-	status_printf_more(s, c, "%-20s%s\n", how, one);
-	strbuf_release(&onebuf);
 }
 
 static const char *wt_status_diff_status_string(int status)
@@ -292,6 +291,48 @@ static const char *wt_status_diff_status_string(int status)
 	}
 }
 
+static int maxwidth(const char *(*label)(int), int minval, int maxval)
+{
+	const char *s;
+	int result = 0, i;
+
+	for (i = minval; i <= maxval; i++) {
+		const char *s = label(i);
+		int len = s ? strlen(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;
+	const char *one, *how;
+	int len;
+
+	if (!padding) {
+		int width = maxwidth(wt_status_unmerged_status_string, 1, 7);
+		width += strlen(" ");
+		padding = xmallocz(width);
+		memset(padding, ' ', width);
+	}
+
+	one = quote_path(it->string, s->prefix, &onebuf);
+	status_printf(s, color(WT_STATUS_HEADER, s), "\t");
+
+	how = wt_status_unmerged_status_string(d->stagemask);
+	if (!how)
+		how = _("bug");
+	len = strlen(padding) - utf8_strwidth(how);
+	status_printf_more(s, c, "%s%.*s%s\n", how, len, padding, one);
+	strbuf_release(&onebuf);
+}
+
 static void wt_status_print_change_data(struct wt_status *s,
 					int change_type,
 					struct string_list_item *it)
@@ -309,14 +350,8 @@ static void wt_status_print_change_data(struct wt_status *s,
 	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;
-			if (len > width)
-				width = len;
-		}
+		/* If DIFF_STATUS_* uses outside the range [A..Z], we're in trouble */
+		int width = maxwidth(wt_status_diff_status_string, 'A', 'Z');
 		width += 2;	/* colon and a space */
 		padding = xmallocz(width);
 		memset(padding, ' ', width);

       reply	other threads:[~2013-12-19 19:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20131008085036.11434.25160.reportbug@x230-buxy.home.ouaza.com>
     [not found] ` <20131219104613.GA18379@x230-buxy.home.ouaza.com>
2013-12-19 19:43   ` Jonathan Nieder [this message]
2013-12-19 20:46     ` git: problematic git status output with some translations (such as fr_FR) Junio C Hamano
2013-12-19 20:50       ` Jonathan Nieder
2013-12-19 23:59         ` Duy Nguyen
2014-01-16 22:00           ` Raphael Hertzog
2014-02-08  9:51         ` Duy Nguyen
2014-03-12 18:53     ` Junio C Hamano
2014-03-12 19:22       ` [PATCH] wt-status: i18n of section labels Junio C Hamano
2014-03-12 20:05         ` Sandy Carter
2014-03-12 20:12           ` Junio C Hamano
2014-03-12 20:17             ` Sandy Carter
2014-03-13 18:01               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131219194319.GB20443@google.com \
    --to=jrnieder@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=hertzog@debian.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).