Re: git: problematic git status output with some translations (such as fr_FR)

From

Jonathan Nieder <jrnieder@gmail.com>

Date

Dec 19, 2013, 19:43 UTC

Message-ID

<20131219194319.GB20443@google.com>

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)))

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);