* git status: minor output format error
@ 2013-11-03 17:17 Wolfgang Rohdewald
2013-11-04 1:46 ` Duy Nguyen
2013-11-04 3:08 ` [PATCH] wt-status: take the alignment burden off translators Nguyễn Thái Ngọc Duy
0 siblings, 2 replies; 6+ messages in thread
From: Wolfgang Rohdewald @ 2013-11-03 17:17 UTC (permalink / raw)
To: git
git version 1.8.3.2
git status in German says (extract)
# geändert: kajongg.py
# gelöscht: playfield.py
as you can see, there is one space too much before playfield.py
LANG=C git status is correct:
# modified: kajongg.py
# deleted: playfield.py
so it seems the spacing between the columns expects "deleted" to have the same number
of letters in all languages.
--
Wolfgang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: git status: minor output format error
2013-11-03 17:17 git status: minor output format error Wolfgang Rohdewald
@ 2013-11-04 1:46 ` Duy Nguyen
2013-11-04 7:53 ` Ralf Thielow
2013-11-04 3:08 ` [PATCH] wt-status: take the alignment burden off translators Nguyễn Thái Ngọc Duy
1 sibling, 1 reply; 6+ messages in thread
From: Duy Nguyen @ 2013-11-04 1:46 UTC (permalink / raw)
To: Wolfgang Rohdewald, Ralf Thielow; +Cc: Git Mailing List
On Mon, Nov 4, 2013 at 12:17 AM, Wolfgang Rohdewald
<wolfgang@rohdewald.de> wrote:
> git version 1.8.3.2
>
> git status in German says (extract)
>
> # geändert: kajongg.py
> # gelöscht: playfield.py
>
> as you can see, there is one space too much before playfield.py
>
> LANG=C git status is correct:
> # modified: kajongg.py
> # deleted: playfield.py
>
> so it seems the spacing between the columns expects "deleted" to have the same number
> of letters in all languages.
No, the translations control the number of columns in this case
(although it's not very intuitive for translators). Something like
this may fix it. I haven't tested because I don't have "de" locale
installed.
diff --git a/po/de.po b/po/de.po
index 35a44b9..d1846d2 100644
--- a/po/de.po
+++ b/po/de.po
@@ -1404,7 +1404,7 @@ msgstr "kopiert: %s -> %s"
#: wt-status.c:313
#, c-format
msgid "deleted: %s"
-msgstr "gelöscht: %s"
+msgstr "gelöscht: %s"
#: wt-status.c:316
#, c-format
--
Duy
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] wt-status: take the alignment burden off translators
2013-11-03 17:17 git status: minor output format error Wolfgang Rohdewald
2013-11-04 1:46 ` Duy Nguyen
@ 2013-11-04 3:08 ` Nguyễn Thái Ngọc Duy
2013-11-04 18:42 ` Junio C Hamano
2013-11-05 2:07 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
1 sibling, 2 replies; 6+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2013-11-04 3:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, wolfgang, vnwildman, ralf.thielow,
Nguyễn Thái Ngọc Duy
It's not easy for translators to see spaces in these strings have to
align, especially when there are no guarantees that these strings are
grouped together in .po files.
Refactor the code and do the alignment automatically. Let the
translator specify how big the alignment is, though, in case some
languages need more than 12 columns for the preceding text.
Noticed-by: Wolfgang Rohdewald <wolfgang@rohdewald.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Ralf and Quan, don't update your translations yet. If this change
gets merged it'll simplify your task a bit.
wt-status.c | 51 +++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..f9715d3 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -15,6 +15,7 @@
#include "submodule.h"
#include "column.h"
#include "strbuf.h"
+#include "utf8.h"
static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
@@ -276,6 +277,10 @@ static void wt_status_print_change_data(struct wt_status *s,
const char *one, *two;
struct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;
struct strbuf extra = STRBUF_INIT;
+ const char *what;
+ static const char *spaces = " ";
+ static int width = -1;
+ int len;
one_name = two_name = it->string;
switch (change_type) {
@@ -309,32 +314,62 @@ static void wt_status_print_change_data(struct wt_status *s,
status_printf(s, color(WT_STATUS_HEADER, s), "\t");
switch (status) {
case DIFF_STATUS_ADDED:
- status_printf_more(s, c, _("new file: %s"), one);
+ what = _("new file");
break;
case DIFF_STATUS_COPIED:
- status_printf_more(s, c, _("copied: %s -> %s"), one, two);
+ what = _("copied");
break;
case DIFF_STATUS_DELETED:
- status_printf_more(s, c, _("deleted: %s"), one);
+ what = _("deleted");
break;
case DIFF_STATUS_MODIFIED:
- status_printf_more(s, c, _("modified: %s"), one);
+ what = _("modified");
break;
case DIFF_STATUS_RENAMED:
- status_printf_more(s, c, _("renamed: %s -> %s"), one, two);
+ what = _("renamed");
break;
case DIFF_STATUS_TYPE_CHANGED:
- status_printf_more(s, c, _("typechange: %s"), one);
+ what = _("typechange");
break;
case DIFF_STATUS_UNKNOWN:
- status_printf_more(s, c, _("unknown: %s"), one);
+ what = _("unknown");
break;
case DIFF_STATUS_UNMERGED:
- status_printf_more(s, c, _("unmerged: %s"), one);
+ what = _("unmerged");
break;
default:
die(_("bug: unhandled diff status %c"), status);
}
+ if (width == -1) {
+ /*
+ * Translators: if you do translate this, replace it
+ * with a decimal number of how many columns needed
+ * to align file names in "git status":
+ *
+ * |<-columns->|
+ *
+ * unmerged: foo.c
+ * copied: foo.c -> bar.c
+ *
+ * The default value is 12. Normally you would not
+ * need to translate this at all unless the translated
+ * strings are longer than 12 columns and therefore
+ * break alignment.
+ */
+ width = atoi(_("<wt-status.c:width>"));
+ if (width <= 0 || width > 32)
+ width = 12;
+ }
+ if (width > utf8_strwidth(what) + 1)
+ len = width - utf8_strwidth(what) - 1;
+ else
+ len = 0;
+ if (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)
+ status_printf_more(s, c, "%s:%.*s%s -> %s",
+ what, len, spaces, one, two);
+ else
+ status_printf_more(s, c, "%s:%.*s%s",
+ what, len, spaces, one);
if (extra.len) {
status_printf_more(s, color(WT_STATUS_HEADER, s), "%s", extra.buf);
strbuf_release(&extra);
--
1.8.2.83.gc99314b
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: git status: minor output format error
2013-11-04 1:46 ` Duy Nguyen
@ 2013-11-04 7:53 ` Ralf Thielow
0 siblings, 0 replies; 6+ messages in thread
From: Ralf Thielow @ 2013-11-04 7:53 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Wolfgang Rohdewald, Git Mailing List
On Mon, Nov 4, 2013 at 2:46 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, Nov 4, 2013 at 12:17 AM, Wolfgang Rohdewald
> <wolfgang@rohdewald.de> wrote:
>> git version 1.8.3.2
>>
>> git status in German says (extract)
>>
>> # geändert: kajongg.py
>> # gelöscht: playfield.py
>>
>> as you can see, there is one space too much before playfield.py
>>
>> LANG=C git status is correct:
>> # modified: kajongg.py
>> # deleted: playfield.py
>>
>> so it seems the spacing between the columns expects "deleted" to have the same number
>> of letters in all languages.
>
> No, the translations control the number of columns in this case
> (although it's not very intuitive for translators). Something like
> this may fix it. I haven't tested because I don't have "de" locale
> installed.
>
> diff --git a/po/de.po b/po/de.po
> index 35a44b9..d1846d2 100644
> --- a/po/de.po
> +++ b/po/de.po
> @@ -1404,7 +1404,7 @@ msgstr "kopiert: %s -> %s"
> #: wt-status.c:313
> #, c-format
> msgid "deleted: %s"
> -msgstr "gelöscht: %s"
> +msgstr "gelöscht: %s"
>
The columns don't match in other related messages, either.
Let's see what happens with [1].
[1]
http://article.gmane.org/gmane.comp.version-control.git/237283
> #: wt-status.c:316
> #, c-format
> --
> Duy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wt-status: take the alignment burden off translators
2013-11-04 3:08 ` [PATCH] wt-status: take the alignment burden off translators Nguyễn Thái Ngọc Duy
@ 2013-11-04 18:42 ` Junio C Hamano
2013-11-05 2:07 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2013-11-04 18:42 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, wolfgang, vnwildman, ralf.thielow
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> + static const char *spaces = " ";
I do not see anything that limits "len" used to show only show the
prefix bytes in this array not to ask for more than you have spaces
here. It won't overrun the end of this array, but I suspect your
result will not align when that happens.
> + static int width = -1;
> + int len;
>
> one_name = two_name = it->string;
> switch (change_type) {
> @@ -309,32 +314,62 @@ static void wt_status_print_change_data(struct wt_status *s,
> status_printf(s, color(WT_STATUS_HEADER, s), "\t");
> switch (status) {
> case DIFF_STATUS_ADDED:
> - status_printf_more(s, c, _("new file: %s"), one);
> + what = _("new file");
> break;
> case DIFF_STATUS_COPIED:
> - status_printf_more(s, c, _("copied: %s -> %s"), one, two);
> + what = _("copied");
> break;
> case DIFF_STATUS_DELETED:
> - status_printf_more(s, c, _("deleted: %s"), one);
> + what = _("deleted");
> break;
> case DIFF_STATUS_MODIFIED:
> - status_printf_more(s, c, _("modified: %s"), one);
> + what = _("modified");
> break;
> case DIFF_STATUS_RENAMED:
> - status_printf_more(s, c, _("renamed: %s -> %s"), one, two);
> + what = _("renamed");
> break;
> case DIFF_STATUS_TYPE_CHANGED:
> - status_printf_more(s, c, _("typechange: %s"), one);
> + what = _("typechange");
> break;
> case DIFF_STATUS_UNKNOWN:
> - status_printf_more(s, c, _("unknown: %s"), one);
> + what = _("unknown");
> break;
> case DIFF_STATUS_UNMERGED:
> - status_printf_more(s, c, _("unmerged: %s"), one);
> + what = _("unmerged");
> break;
> default:
> die(_("bug: unhandled diff status %c"), status);
> }
> + if (width == -1) {
> + /*
> + * Translators: if you do translate this, replace it
> + * with a decimal number of how many columns needed
> + * to align file names in "git status":
> + *
> + * |<-columns->|
> + *
> + * unmerged: foo.c
> + * copied: foo.c -> bar.c
> + *
> + * The default value is 12. Normally you would not
> + * need to translate this at all unless the translated
> + * strings are longer than 12 columns and therefore
> + * break alignment.
> + */
> + width = atoi(_("<wt-status.c:width>"));
Somewhat ugly. Wouldn't it work better to keep the "what" strings
in a static array and compute "width" just once? Is the sparseness
of DIFF_STATUS_* enums what makes such an implementation cumbersome?
I dunno.
> + if (width <= 0 || width > 32)
> + width = 12;
> + }
> + if (width > utf8_strwidth(what) + 1)
> + len = width - utf8_strwidth(what) - 1;
Just a style, but this is far easier to read if you said:
if (width > utf8_strwidth(what) + 1)
len = width - (utf8_strwidth(what) + 1);
because you are essentially doing:
if (A > B)
x = A - B;
Or even
len = width - (utf8_strwidth(what) + 1);
if (len < 0)
len = 0;
> + else
> + len = 0;
> + if (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)
> + status_printf_more(s, c, "%s:%.*s%s -> %s",
> + what, len, spaces, one, two);
> + else
> + status_printf_more(s, c, "%s:%.*s%s",
> + what, len, spaces, one);
> if (extra.len) {
> status_printf_more(s, color(WT_STATUS_HEADER, s), "%s", extra.buf);
> strbuf_release(&extra);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] wt-status: take the alignment burden off translators
2013-11-04 3:08 ` [PATCH] wt-status: take the alignment burden off translators Nguyễn Thái Ngọc Duy
2013-11-04 18:42 ` Junio C Hamano
@ 2013-11-05 2:07 ` Nguyễn Thái Ngọc Duy
1 sibling, 0 replies; 6+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2013-11-05 2:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, wolfgang, vnwildman, ralf.thielow,
Nguyễn Thái Ngọc Duy
It's not easy for translators to see spaces in these strings have to
align, especially when there are no guarantees that these strings are
grouped together in .po files. Refactor the code and do the alignment
automatically.
Noticed-by: Wolfgang Rohdewald <wolfgang@rohdewald.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v2 calculates maximum length automatically too so the translator does
not have to test and specify that manually.
wt-status.c | 80 ++++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 53 insertions(+), 27 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..4625cdb 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -15,6 +15,7 @@
#include "submodule.h"
#include "column.h"
#include "strbuf.h"
+#include "utf8.h"
static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
@@ -264,6 +265,30 @@ static void wt_status_print_unmerged_data(struct wt_status *s,
strbuf_release(&onebuf);
}
+static const char *wt_status_diff_status_string(int status)
+{
+ switch (status) {
+ case DIFF_STATUS_ADDED:
+ return _("new file");
+ case DIFF_STATUS_COPIED:
+ return _("copied");
+ case DIFF_STATUS_DELETED:
+ return _("deleted");
+ case DIFF_STATUS_MODIFIED:
+ return _("modified");
+ case DIFF_STATUS_RENAMED:
+ return _("renamed");
+ case DIFF_STATUS_TYPE_CHANGED:
+ return _("typechange");
+ case DIFF_STATUS_UNKNOWN:
+ return _("unknown");
+ case DIFF_STATUS_UNMERGED:
+ return _("unmerged");
+ default:
+ return NULL;
+ }
+}
+
static void wt_status_print_change_data(struct wt_status *s,
int change_type,
struct string_list_item *it)
@@ -276,6 +301,23 @@ static void wt_status_print_change_data(struct wt_status *s,
const char *one, *two;
struct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;
struct strbuf extra = STRBUF_INIT;
+ static char *padding;
+ 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;
+ }
+ width += 2; /* colon and a space */
+ padding = xmallocz(width);
+ memset(padding, ' ', width);
+ }
one_name = two_name = it->string;
switch (change_type) {
@@ -307,34 +349,18 @@ static void wt_status_print_change_data(struct wt_status *s,
two = quote_path(two_name, s->prefix, &twobuf);
status_printf(s, color(WT_STATUS_HEADER, s), "\t");
- switch (status) {
- case DIFF_STATUS_ADDED:
- status_printf_more(s, c, _("new file: %s"), one);
- break;
- case DIFF_STATUS_COPIED:
- status_printf_more(s, c, _("copied: %s -> %s"), one, two);
- break;
- case DIFF_STATUS_DELETED:
- status_printf_more(s, c, _("deleted: %s"), one);
- break;
- case DIFF_STATUS_MODIFIED:
- status_printf_more(s, c, _("modified: %s"), one);
- break;
- case DIFF_STATUS_RENAMED:
- status_printf_more(s, c, _("renamed: %s -> %s"), one, two);
- break;
- case DIFF_STATUS_TYPE_CHANGED:
- status_printf_more(s, c, _("typechange: %s"), one);
- break;
- case DIFF_STATUS_UNKNOWN:
- status_printf_more(s, c, _("unknown: %s"), one);
- break;
- case DIFF_STATUS_UNMERGED:
- status_printf_more(s, c, _("unmerged: %s"), one);
- break;
- default:
+ 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);
+ assert(len >= 0);
+ if (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)
+ status_printf_more(s, c, "%s:%.*s%s -> %s",
+ what, len, padding, one, two);
+ else
+ 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);
strbuf_release(&extra);
--
1.8.2.82.gc24b958
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-05 2:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-03 17:17 git status: minor output format error Wolfgang Rohdewald
2013-11-04 1:46 ` Duy Nguyen
2013-11-04 7:53 ` Ralf Thielow
2013-11-04 3:08 ` [PATCH] wt-status: take the alignment burden off translators Nguyễn Thái Ngọc Duy
2013-11-04 18:42 ` Junio C Hamano
2013-11-05 2:07 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.