git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] status: allow label strings to be translated
@ 2014-03-12 21:19 Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 1/4] wt-status: make full label string to be subject to l10n Junio C Hamano
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-03-12 21:19 UTC (permalink / raw)
  To: git

So here is my attempt to clean-up what Jonathan posted in
$gmane/239537 as "how about this?" patch.

The first one (full label string) fixes up 3651e45c (wt-status: take
the alignment burden off translators, 2013-11-05) to include colon
back to translatable string again, while retaining its label alignment
logic.

The second (extract the code) is taken from Jonathan's $gmane/239537
as a separate patch.

The third is essentially the remainder of Jonathan's $gmane/239537,
with one small fix s/strlen/utf8_width/; to teach the code that
shows unmerged paths the same label alignment logic Duy added in
3651e45c for the tracked paths, while retaining the "at least 20
columns" floor to avoid the churn to the tests.

And the last lifts the "at least 20 columns" floor.

Jonathan Nieder (2):
  wt-status: extract the code to compute width for labels
  wt-status: i18n of section labels

Junio C Hamano (2):
  wt-status: make full label string to be subject to l10n
  wt-status: lift the artificual "at least 20 columns" floor

 t/t7060-wtstatus.sh    |  14 +++---
 t/t7512-status-help.sh |  12 ++---
 wt-status.c            | 117 +++++++++++++++++++++++++++++++------------------
 3 files changed, 88 insertions(+), 55 deletions(-)

-- 
1.9.0-293-gd838d6f

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/4] wt-status: make full label string to be subject to l10n
  2014-03-12 21:19 [PATCH v2 0/4] status: allow label strings to be translated Junio C Hamano
@ 2014-03-12 21:19 ` Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 2/4] wt-status: extract the code to compute width for labels Junio C Hamano
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-03-12 21:19 UTC (permalink / raw)
  To: git

Earlier in 3651e45c (wt-status: take the alignment burden off
translators, 2013-11-05), we assumed that it is OK to make the
string before the colon in a label string we give as the section
header of various kinds of changes (e.g. "new file:") translatable.

This assumption apparently does not hold for some languages,
e.g. ones that want to have spaces around the colon.

Also introduce a static label_width to avoid having to run
strlen(padding) over and over.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 4e55810..9cf7028 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -272,21 +272,21 @@ 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;
 	}
@@ -305,21 +305,21 @@ 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;
-			if (len > width)
-				width = len;
+			if (len > label_width)
+				label_width = len;
 		}
-		width += 2;	/* colon and a space */
-		padding = xmallocz(width);
-		memset(padding, ' ', width);
+		label_width += strlen(" ");
+		padding = xmallocz(label_width);
+		memset(padding, ' ', label_width);
 	}
 
 	one_name = two_name = it->string;
@@ -355,14 +355,13 @@ static void wt_status_print_change_data(struct wt_status *s,
 	what = wt_status_diff_status_string(status);
 	if (!what)
 		die(_("bug: unhandled diff status %c"), status);
-	/* 1 for colon, which is not part of "what" */
-	len = strlen(padding) - (utf8_strwidth(what) + 1);
+	len = label_width - utf8_strwidth(what);
 	assert(len >= 0);
 	if (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)
-		status_printf_more(s, c, "%s:%.*s%s -> %s",
+		status_printf_more(s, c, "%s%.*s%s -> %s",
 				   what, len, padding, one, two);
 	else
-		status_printf_more(s, c, "%s:%.*s%s",
+		status_printf_more(s, c, "%s%.*s%s",
 				   what, len, padding, one);
 	if (extra.len) {
 		status_printf_more(s, color(WT_STATUS_HEADER, s), "%s", extra.buf);
-- 
1.9.0-293-gd838d6f

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/4] wt-status: extract the code to compute width for labels
  2014-03-12 21:19 [PATCH v2 0/4] status: allow label strings to be translated Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 1/4] wt-status: make full label string to be subject to l10n Junio C Hamano
@ 2014-03-12 21:19 ` Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 3/4] wt-status: i18n of section labels Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 4/4] wt-status: lift the artificual "at least 20 columns" floor Junio C Hamano
  3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-03-12 21:19 UTC (permalink / raw)
  To: git; +Cc: Jonathan Nieder

From: Jonathan Nieder <jrnieder@gmail.com>

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Thu, 19 Dec 2013 11:43:19 -0800

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 9cf7028..db98c52 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -292,6 +292,19 @@ static const char *wt_status_diff_status_string(int status)
 	}
 }
 
+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_change_data(struct wt_status *s,
 					int change_type,
 					struct string_list_item *it)
@@ -310,13 +323,8 @@ static void wt_status_print_change_data(struct wt_status *s,
 	int len;
 
 	if (!padding) {
-		/* 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 > label_width)
-				label_width = len;
-		}
+		/* If DIFF_STATUS_* uses outside the range [A..Z], we're in trouble */
+		label_width = maxwidth(wt_status_diff_status_string, 'A', 'Z');
 		label_width += strlen(" ");
 		padding = xmallocz(label_width);
 		memset(padding, ' ', label_width);
-- 
1.9.0-293-gd838d6f

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 3/4] wt-status: i18n of section labels
  2014-03-12 21:19 [PATCH v2 0/4] status: allow label strings to be translated Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 1/4] wt-status: make full label string to be subject to l10n Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 2/4] wt-status: extract the code to compute width for labels Junio C Hamano
@ 2014-03-12 21:19 ` Junio C Hamano
  2014-03-12 21:19 ` [PATCH v2 4/4] wt-status: lift the artificual "at least 20 columns" floor Junio C Hamano
  3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-03-12 21:19 UTC (permalink / raw)
  To: git; +Cc: Jonathan Nieder

From: Jonathan Nieder <jrnieder@gmail.com>

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Thu, 19 Dec 2013 11:43:19 -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.

Neither of which we should assume.

Using the same approach as the earlier 3651e45c (wt-status: take the
alignment burden off translators, 2013-11-05), compute the necessary
column width to hold the longest label and use that for alignment.

cf. http://bugs.debian.org/725777

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Sandy Carter
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c | 66 +++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index db98c52..b1b018e 100644
--- a/wt-status.c
+++ b/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:
+		die(_("bug: unhandled unmerged status %x"), stagemask);
 	}
-	status_printf_more(s, c, "%-20s%s\n", how, one);
-	strbuf_release(&onebuf);
 }
 
 static const char *wt_status_diff_status_string(int status)
@@ -305,6 +304,35 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)
 	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) {
+		label_width = maxwidth(wt_status_unmerged_status_string, 1, 7);
+		label_width += strlen(" ");
+		if (label_width < 20)
+			label_width = 20;
+		padding = xmallocz(label_width);
+		memset(padding, ' ', label_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);
+	len = label_width - 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)
-- 
1.9.0-293-gd838d6f

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 4/4] wt-status: lift the artificual "at least 20 columns" floor
  2014-03-12 21:19 [PATCH v2 0/4] status: allow label strings to be translated Junio C Hamano
                   ` (2 preceding siblings ...)
  2014-03-12 21:19 ` [PATCH v2 3/4] wt-status: i18n of section labels Junio C Hamano
@ 2014-03-12 21:19 ` Junio C Hamano
  3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-03-12 21:19 UTC (permalink / raw)
  To: git

When we show unmerged paths, we had an artificial 20 columns floor
for the width of labels (e.g. "both deleted:") shown next to the
pathnames.  Depending on the locale, this may result in a label that
is too wide when all the label strings are way shorter than 20
columns, or no-op when a label string is longer than 20 columns.

Just drop the artificial floor.  The screen real estate is better
utilized this way when all the strings are shorter.

Adjust the tests to this change.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7060-wtstatus.sh    | 14 +++++++-------
 t/t7512-status-help.sh | 12 ++++++------
 wt-status.c            |  2 --
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 7d467c0..741ec08 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -38,7 +38,7 @@ You have unmerged paths.
 Unmerged paths:
   (use "git add/rm <file>..." as appropriate to mark resolution)
 
-	deleted by us:      foo
+	deleted by us:   foo
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
@@ -142,8 +142,8 @@ You have unmerged paths.
 Unmerged paths:
   (use "git add/rm <file>..." as appropriate to mark resolution)
 
-	both added:         conflict.txt
-	deleted by them:    main.txt
+	both added:      conflict.txt
+	deleted by them: main.txt
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
@@ -175,9 +175,9 @@ You have unmerged paths.
 Unmerged paths:
   (use "git add/rm <file>..." as appropriate to mark resolution)
 
-	both deleted:       main.txt
-	added by them:      sub_master.txt
-	added by us:        sub_second.txt
+	both deleted:    main.txt
+	added by them:   sub_master.txt
+	added by us:     sub_second.txt
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
@@ -203,7 +203,7 @@ Changes to be committed:
 Unmerged paths:
   (use "git rm <file>..." to mark resolution)
 
-	both deleted:       main.txt
+	both deleted:    main.txt
 
 Untracked files not listed (use -u option to show untracked files)
 EOF
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 3cec57a..68ad2d7 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -33,7 +33,7 @@ You have unmerged paths.
 Unmerged paths:
   (use "git add <file>..." to mark resolution)
 
-	both modified:      main.txt
+	both modified:   main.txt
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
@@ -87,7 +87,7 @@ Unmerged paths:
   (use "git reset HEAD <file>..." to unstage)
   (use "git add <file>..." to mark resolution)
 
-	both modified:      main.txt
+	both modified:   main.txt
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
@@ -146,7 +146,7 @@ Unmerged paths:
   (use "git reset HEAD <file>..." to unstage)
   (use "git add <file>..." to mark resolution)
 
-	both modified:      main.txt
+	both modified:   main.txt
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
@@ -602,7 +602,7 @@ rebase in progress; onto $ONTO
 You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
 
 Unmerged paths:
-	both modified:      main.txt
+	both modified:   main.txt
 
 no changes added to commit
 EOF
@@ -636,7 +636,7 @@ You are currently cherry-picking commit $TO_CHERRY_PICK.
 Unmerged paths:
   (use "git add <file>..." to mark resolution)
 
-	both modified:      main.txt
+	both modified:   main.txt
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
@@ -707,7 +707,7 @@ Unmerged paths:
   (use "git reset HEAD <file>..." to unstage)
   (use "git add <file>..." to mark resolution)
 
-	both modified:      to-revert.txt
+	both modified:   to-revert.txt
 
 no changes added to commit (use "git add" and/or "git commit -a")
 EOF
diff --git a/wt-status.c b/wt-status.c
index b1b018e..6f3ed67 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -318,8 +318,6 @@ static void wt_status_print_unmerged_data(struct wt_status *s,
 	if (!padding) {
 		label_width = maxwidth(wt_status_unmerged_status_string, 1, 7);
 		label_width += strlen(" ");
-		if (label_width < 20)
-			label_width = 20;
 		padding = xmallocz(label_width);
 		memset(padding, ' ', label_width);
 	}
-- 
1.9.0-293-gd838d6f

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-03-12 21:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-12 21:19 [PATCH v2 0/4] status: allow label strings to be translated Junio C Hamano
2014-03-12 21:19 ` [PATCH v2 1/4] wt-status: make full label string to be subject to l10n Junio C Hamano
2014-03-12 21:19 ` [PATCH v2 2/4] wt-status: extract the code to compute width for labels Junio C Hamano
2014-03-12 21:19 ` [PATCH v2 3/4] wt-status: i18n of section labels Junio C Hamano
2014-03-12 21:19 ` [PATCH v2 4/4] wt-status: lift the artificual "at least 20 columns" floor Junio C Hamano

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