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

From: Jonathan Nieder <[email-masked]>
Date: Thu, 19 Dec 2013 11:43:19 -0800

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 <[email-masked]>

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