{"thread":{"id":"31416","subject":"[PATCH] fetch: align new ref summary printout in UTF-8 locales","startedAt":"2012-09-03T11:10:09Z","lastAt":"2012-09-12T20:12:02Z","messageCount":14,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Johannes Sixt","Nguyen Thai Ngoc Duy","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"198244","messageId":"1346670609-19986-1-git-send-email-pclouds@gmail.com","threadId":"31416","inReplyTo":null,"subject":"[PATCH] fetch: align new ref summary printout in UTF-8 locales","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-03T11:10:09Z","receivedAt":"2012-09-03T11:10:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"fetch does printf(\"%-*s\", width, \"foo\") where \"foo\" can be an utf-8\nstring, but width is bytes, not letters. This results in misaligned\nref summary table.\n\nIntroduce gettext_length() function that returns the string length in\nletters. Make the code use TRANSPORT_SUMMARY(x) where the length is\ncompensated properly in utf-8 locales.\n---\n gettext_length() can be made to support other charsets too. But I'm\n on utf-8, it's not my itch.\n\n Grepping through '%-*s' does not reveal any other places that obviously\n need adjustment like this (apply and remote might, but pathnames and\n remote names are usually in ascii)\n\n builtin/fetch.c | 22 ++++++++++------------\n gettext.c       | 14 ++++++++++++--\n gettext.h       |  5 +++++\n transport.c     |  2 +-\n transport.h     |  1 +\n 5 files changed, 29 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex bb9a074..d7406d2 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -255,9 +255,8 @@ static int update_local_ref(struct ref *ref,\n \tif (!hashcmp(ref->old_sha1, ref->new_sha1)) {\n \t\tif (verbosity > 0)\n \t\t\tstrbuf_addf(display, \"= %-*s %-*s -> %s\",\n-\t\t\t\t    TRANSPORT_SUMMARY_WIDTH,\n-\t\t\t\t    _(\"[up to date]\"), REFCOL_WIDTH,\n-\t\t\t\t    remote, pretty_ref);\n+\t\t\t\t    TRANSPORT_SUMMARY(_(\"[up to date]\")),\n+\t\t\t\t    REFCOL_WIDTH, remote, pretty_ref);\n \t\treturn 0;\n \t}\n \n@@ -271,7 +270,7 @@ static int update_local_ref(struct ref *ref,\n \t\t */\n \t\tstrbuf_addf(display,\n \t\t\t    _(\"! %-*s %-*s -> %s  (can't fetch in current branch)\"),\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n+\t\t\t    TRANSPORT_SUMMARY(_(\"[rejected]\")),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref);\n \t\treturn 1;\n \t}\n@@ -282,7 +281,7 @@ static int update_local_ref(struct ref *ref,\n \t\tr = s_update_ref(\"updating tag\", ref, 0);\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t    r ? '!' : '-',\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, _(\"[tag update]\"),\n+\t\t\t    TRANSPORT_SUMMARY(_(\"[tag update]\")),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    r ? _(\"  (unable to update local ref)\") : \"\");\n \t\treturn r;\n@@ -317,7 +316,7 @@ static int update_local_ref(struct ref *ref,\n \t\tr = s_update_ref(msg, ref, 0);\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t    r ? '!' : '*',\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, what,\n+\t\t\t    TRANSPORT_SUMMARY(what),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    r ? _(\"  (unable to update local ref)\") : \"\");\n \t\treturn r;\n@@ -335,7 +334,7 @@ static int update_local_ref(struct ref *ref,\n \t\tr = s_update_ref(\"fast-forward\", ref, 1);\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t    r ? '!' : ' ',\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, quickref,\n+\t\t\t    TRANSPORT_SUMMARY(quickref),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    r ? _(\"  (unable to update local ref)\") : \"\");\n \t\treturn r;\n@@ -351,13 +350,13 @@ static int update_local_ref(struct ref *ref,\n \t\tr = s_update_ref(\"forced-update\", ref, 1);\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s  (%s)\",\n \t\t\t    r ? '!' : '+',\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, quickref,\n+\t\t\t    TRANSPORT_SUMMARY(quickref),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    r ? _(\"unable to update local ref\") : _(\"forced update\"));\n \t\treturn r;\n \t} else {\n \t\tstrbuf_addf(display, \"! %-*s %-*s -> %s  %s\",\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n+\t\t\t    TRANSPORT_SUMMARY(_(\"[rejected]\")),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    _(\"(non-fast-forward)\"));\n \t\treturn 1;\n@@ -479,8 +478,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\t\tfree(ref);\n \t\t\t} else\n \t\t\t\tstrbuf_addf(&note, \"* %-*s %-*s -> FETCH_HEAD\",\n-\t\t\t\t\t    TRANSPORT_SUMMARY_WIDTH,\n-\t\t\t\t\t    *kind ? kind : \"branch\",\n+\t\t\t\t\t    TRANSPORT_SUMMARY(*kind ? kind : \"branch\"),\n \t\t\t\t\t    REFCOL_WIDTH,\n \t\t\t\t\t    *what ? what : \"HEAD\");\n \t\t\tif (note.len) {\n@@ -554,7 +552,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n \t\t\tresult |= delete_ref(ref->name, NULL, 0);\n \t\tif (verbosity >= 0) {\n \t\t\tfprintf(stderr, \" x %-*s %-*s -> %s\\n\",\n-\t\t\t\tTRANSPORT_SUMMARY_WIDTH, _(\"[deleted]\"),\n+\t\t\t\tTRANSPORT_SUMMARY(_(\"[deleted]\")),\n \t\t\t\tREFCOL_WIDTH, _(\"(none)\"), prettify_refname(ref->name));\n \t\t\twarn_dangling_symref(stderr, dangling_msg, ref->name);\n \t\t}\ndiff --git a/gettext.c b/gettext.c\nindex f75bca7..e9f0f0d 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -4,6 +4,8 @@\n \n #include \"git-compat-util.h\"\n #include \"gettext.h\"\n+#include \"strbuf.h\"\n+#include \"utf8.h\"\n \n #ifndef NO_GETTEXT\n #\tinclude <locale.h>\n@@ -27,10 +29,9 @@ int use_gettext_poison(void)\n #endif\n \n #ifndef NO_GETTEXT\n+static const char *charset;\n static void init_gettext_charset(const char *domain)\n {\n-\tconst char *charset;\n-\n \t/*\n \t   This trick arranges for messages to be emitted in the user's\n \t   requested encoding, but avoids setting LC_CTYPE from the\n@@ -128,4 +129,13 @@ void git_setup_gettext(void)\n \tinit_gettext_charset(\"git\");\n \ttextdomain(\"git\");\n }\n+\n+int gettext_length(const char *s)\n+{\n+\tstatic int is_utf8 = -1;\n+\tif (is_utf8 == -1)\n+\t\tis_utf8 = !strcmp(charset, \"UTF-8\");\n+\n+\treturn is_utf8 ? utf8_strwidth(s) : strlen(s);\n+}\n #endif\ndiff --git a/gettext.h b/gettext.h\nindex 57ba8bb..7ea6db4 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -30,10 +30,15 @@\n \n #ifndef NO_GETTEXT\n extern void git_setup_gettext(void);\n+extern int gettext_length(const char *s);\n #else\n static inline void git_setup_gettext(void)\n {\n }\n+static inline int gettext_length(const char *s)\n+{\n+\treturn strlen(s);\n+}\n #endif\n \n #ifdef GETTEXT_POISON\ndiff --git a/transport.c b/transport.c\nindex 1811b50..863aaa6 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -629,7 +629,7 @@ static void print_ref_status(char flag, const char *summary, struct ref *to, str\n \t\telse\n \t\t\tfprintf(stdout, \"%s\\n\", summary);\n \t} else {\n-\t\tfprintf(stderr, \" %c %-*s \", flag, TRANSPORT_SUMMARY_WIDTH, summary);\n+\t\tfprintf(stderr, \" %c %-*s \", flag, TRANSPORT_SUMMARY(summary));\n \t\tif (from)\n \t\t\tfprintf(stderr, \"%s -> %s\", prettify_refname(from->name), prettify_refname(to->name));\n \t\telse\ndiff --git a/transport.h b/transport.h\nindex b866c12..1c95a8c 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -106,6 +106,7 @@ struct transport {\n #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n+#define TRANSPORT_SUMMARY(x) (TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_length(x)), (x)\n \n /* Returns a transport suitable for the url */\n struct transport *transport_get(struct remote *, const char *);\n-- \n1.7.12.rc2.18.g61b472e\n"},{"id":"198264","messageId":"7vwr0aewlf.fsf@alter.siamese.dyndns.org","threadId":"31416","inReplyTo":"1346670609-19986-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] fetch: align new ref summary printout in UTF-8 locales","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-03T19:26:52Z","receivedAt":"2012-09-03T19:26:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> fetch does printf(\"%-*s\", width, \"foo\") where \"foo\" can be an utf-8\n> string, but width is bytes, not letters. This results in misaligned\n> ref summary table.\n\n\"but width is bytes, not letters\" is a misleading statement.\n\nBe careful about three different quantities when talking about\naligning on terminal output with monospaced fonts:\n\n - How many bytes does the string occupy in memory?\n - How many unicode codepoints are in the string?\n - How many display columns does the string occupy on terminal?\n\nNote that some \"letters\" (e.g. Han) occupy two display columns, and\nyou want to measure the \"width\" and compensate that for \"bytes\".\nLetter count do not come into the picture for the purpose of aligning\nthe terminal output.\n\n> Introduce gettext_length() function that returns the string length in\n> letters. Make the code use TRANSPORT_SUMMARY(x) where the length is\n\nAgain, are you measuring \"string length in letters\"?  Or are you\ntrying to measure in terms of git_wcwidth()?  If the latter, please\nname this after \"width\", not \"length\" to make it more clear.\n\n> compensated properly in utf-8 locales.\n> ---\n\nIs it in vogue to omit Signed-off-by line these days or something?\n\n>  gettext_length() can be made to support other charsets too. But I'm\n>  on utf-8, it's not my itch.\n>\n>  Grepping through '%-*s' does not reveal any other places that obviously\n>  need adjustment like this (apply and remote might, but pathnames and\n>  remote names are usually in ascii)\n\nThanks.\n"},{"id":"198266","messageId":"50450C8D.7010301@kdbg.org","threadId":"31416","inReplyTo":"7vwr0aewlf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fetch: align new ref summary printout in UTF-8 locales","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-09-03T20:01:17Z","receivedAt":"2012-09-03T20:01:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.09.2012 21:26, schrieb Junio C Hamano:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n> \n>> fetch does printf(\"%-*s\", width, \"foo\") where \"foo\" can be an utf-8\n>> string, but width is bytes, not letters. This results in misaligned\n>> ref summary table.\n> \n> \"but width is bytes, not letters\" is a misleading statement.\n> \n> Be careful about three different quantities when talking about\n> aligning on terminal output with monospaced fonts:\n> \n>  - How many bytes does the string occupy in memory?\n>  - How many unicode codepoints are in the string?\n>  - How many display columns does the string occupy on terminal?\n> \n> Note that some \"letters\" (e.g. Han) occupy two display columns, and\n> you want to measure the \"width\" and compensate that for \"bytes\".\n> Letter count do not come into the picture for the purpose of aligning\n> the terminal output.\n\nIf I'm reading POSIX correctly, printf measures the width in %*s in bytes.\n\n-- Hannes\n"},{"id":"198269","messageId":"7vk3waetnf.fsf@alter.siamese.dyndns.org","threadId":"31416","inReplyTo":"50450C8D.7010301@kdbg.org","subject":"Re: [PATCH] fetch: align new ref summary printout in UTF-8 locales","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-03T20:30:28Z","receivedAt":"2012-09-03T20:30:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 03.09.2012 21:26, schrieb Junio C Hamano:\n>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>> \n>>> fetch does printf(\"%-*s\", width, \"foo\") where \"foo\" can be an utf-8\n>>> string, but width is bytes, not letters. This results in misaligned\n>>> ref summary table.\n>> \n>> \"but width is bytes, not letters\" is a misleading statement.\n>> \n>> Be careful about three different quantities when talking about\n>> aligning on terminal output with monospaced fonts:\n>> \n>>  - How many bytes does the string occupy in memory?\n>>  - How many unicode codepoints are in the string?\n>>  - How many display columns does the string occupy on terminal?\n>> \n>> Note that some \"letters\" (e.g. Han) occupy two display columns, and\n>> you want to measure the \"width\" and compensate that for \"bytes\".\n>> Letter count do not come into the picture for the purpose of aligning\n>> the terminal output.\n>\n> If I'm reading POSIX correctly, printf measures the width in %*s in bytes.\n\nYes, that is the problem.\n\nWhen we give allocated_width columns to display str, and call\n\n        printf(\"%.*s\", allocated_width, str)\n\nwe end up getting (allocated_width - strlen(str)) SPs as filler.  If\nthe str occupies less than strlen(str) columns (and it often is the\ncase for a string with non-ascii characters), insufficient number of\nfiller SPs will be given, and an item on the same line that comes\nafter this printf() will appear too close to str.  By measuring the\nrequired number of columns to display str, you can compensate. i.e.\n\n        printf(\"%.*s\", allocated_width +\n                (strlen(str) - display_width(str)), str)\n\nto give it enough filler.\n\nI think the patch uses utf8_strwidth() which measures the display\ncolumns needed for the string, not number of unicode codepoints in\nthe string.  I was merely pointing out that the wording of the\nproposed log message \"bytes, not letters\" and the function name\n\"foo_length\" was misleading, when we mean \"width\".\n"},{"id":"198272","messageId":"CACsJy8B_aUfg6Cs_23UyjoVwQGrkBtMUoqOBQLTSM2cC=KfR3g@mail.gmail.com","threadId":"31416","inReplyTo":"7vwr0aewlf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fetch: align new ref summary printout in UTF-8 locales","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-04T01:31:03Z","receivedAt":"2012-09-04T01:31:03Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Sep 4, 2012 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> fetch does printf(\"%-*s\", width, \"foo\") where \"foo\" can be an utf-8\n>> string, but width is bytes, not letters. This results in misaligned\n>> ref summary table.\n>\n> \"but width is bytes, not letters\" is a misleading statement.\n>\n> Be careful about three different quantities when talking about\n> aligning on terminal output with monospaced fonts:\n>\n>  - How many bytes does the string occupy in memory?\n>  - How many unicode codepoints are in the string?\n>  - How many display columns does the string occupy on terminal?\n>\n> Note that some \"letters\" (e.g. Han) occupy two display columns, and\n> you want to measure the \"width\" and compensate that for \"bytes\".\n> Letter count do not come into the picture for the purpose of aligning\n> the terminal output.\n\nHmm.. I did test with zh_CN.UTF-8 and it seemed to work. I'll check this again.\n-- \nDuy\n"},{"id":"198293","messageId":"1346755175-31468-1-git-send-email-pclouds@gmail.com","threadId":"31416","inReplyTo":"1346670609-19986-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-04T10:39:35Z","receivedAt":"2012-09-04T10:39:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"fetch does printf(\"%-*s\", width, \"foo\") where \"foo\" can be a utf-8\nstring, but width is in bytes, not columns. For ASCII it's fine as one\nbyte takes one column. For utf-8, this may result in misaligned ref\nsummary table.\n\nIntroduce gettext_width() function that returns the string length in\ncolumns (currently only supports utf-8 locales). Make the code use\nTRANSPORT_SUMMARY(x) where the length is compensated properly in\nnon-English locales.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n  - rename gettext_length() to gettext_width()\n  - use \"width\" instead of letters\n  - leave other \"%-*s\" places unchanged if they always take ascii\n    strings (i.e. no _() calls)\n  - note to self, may need to i18n-ize print_ref_status() in\n    transport.c, looks like it's used by git-push only\n\n builtin/fetch.c | 15 +++++++--------\n gettext.c       | 15 +++++++++++++--\n gettext.h       |  5 +++++\n transport.h     |  1 +\n 4 files changed, 26 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex bb9a074..85e291f 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -255,9 +255,8 @@ static int update_local_ref(struct ref *ref,\n \tif (!hashcmp(ref->old_sha1, ref->new_sha1)) {\n \t\tif (verbosity > 0)\n \t\t\tstrbuf_addf(display, \"= %-*s %-*s -> %s\",\n-\t\t\t\t    TRANSPORT_SUMMARY_WIDTH,\n-\t\t\t\t    _(\"[up to date]\"), REFCOL_WIDTH,\n-\t\t\t\t    remote, pretty_ref);\n+\t\t\t\t    TRANSPORT_SUMMARY(_(\"[up to date]\")),\n+\t\t\t\t    REFCOL_WIDTH, remote, pretty_ref);\n \t\treturn 0;\n \t}\n \n@@ -271,7 +270,7 @@ static int update_local_ref(struct ref *ref,\n \t\t */\n \t\tstrbuf_addf(display,\n \t\t\t    _(\"! %-*s %-*s -> %s  (can't fetch in current branch)\"),\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n+\t\t\t    TRANSPORT_SUMMARY(_(\"[rejected]\")),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref);\n \t\treturn 1;\n \t}\n@@ -282,7 +281,7 @@ static int update_local_ref(struct ref *ref,\n \t\tr = s_update_ref(\"updating tag\", ref, 0);\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t    r ? '!' : '-',\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, _(\"[tag update]\"),\n+\t\t\t    TRANSPORT_SUMMARY(_(\"[tag update]\")),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    r ? _(\"  (unable to update local ref)\") : \"\");\n \t\treturn r;\n@@ -317,7 +316,7 @@ static int update_local_ref(struct ref *ref,\n \t\tr = s_update_ref(msg, ref, 0);\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t    r ? '!' : '*',\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, what,\n+\t\t\t    TRANSPORT_SUMMARY(what),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    r ? _(\"  (unable to update local ref)\") : \"\");\n \t\treturn r;\n@@ -357,7 +356,7 @@ static int update_local_ref(struct ref *ref,\n \t\treturn r;\n \t} else {\n \t\tstrbuf_addf(display, \"! %-*s %-*s -> %s  %s\",\n-\t\t\t    TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n+\t\t\t    TRANSPORT_SUMMARY(_(\"[rejected]\")),\n \t\t\t    REFCOL_WIDTH, remote, pretty_ref,\n \t\t\t    _(\"(non-fast-forward)\"));\n \t\treturn 1;\n@@ -554,7 +553,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n \t\t\tresult |= delete_ref(ref->name, NULL, 0);\n \t\tif (verbosity >= 0) {\n \t\t\tfprintf(stderr, \" x %-*s %-*s -> %s\\n\",\n-\t\t\t\tTRANSPORT_SUMMARY_WIDTH, _(\"[deleted]\"),\n+\t\t\t\tTRANSPORT_SUMMARY(_(\"[deleted]\")),\n \t\t\t\tREFCOL_WIDTH, _(\"(none)\"), prettify_refname(ref->name));\n \t\t\twarn_dangling_symref(stderr, dangling_msg, ref->name);\n \t\t}\ndiff --git a/gettext.c b/gettext.c\nindex f75bca7..71e9545 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -4,6 +4,8 @@\n \n #include \"git-compat-util.h\"\n #include \"gettext.h\"\n+#include \"strbuf.h\"\n+#include \"utf8.h\"\n \n #ifndef NO_GETTEXT\n #\tinclude <locale.h>\n@@ -27,10 +29,9 @@ int use_gettext_poison(void)\n #endif\n \n #ifndef NO_GETTEXT\n+static const char *charset;\n static void init_gettext_charset(const char *domain)\n {\n-\tconst char *charset;\n-\n \t/*\n \t   This trick arranges for messages to be emitted in the user's\n \t   requested encoding, but avoids setting LC_CTYPE from the\n@@ -128,4 +129,14 @@ void git_setup_gettext(void)\n \tinit_gettext_charset(\"git\");\n \ttextdomain(\"git\");\n }\n+\n+/* return the number of columns of string 's' in current locale */\n+int gettext_width(const char *s)\n+{\n+\tstatic int is_utf8 = -1;\n+\tif (is_utf8 == -1)\n+\t\tis_utf8 = !strcmp(charset, \"UTF-8\");\n+\n+\treturn is_utf8 ? utf8_strwidth(s) : strlen(s);\n+}\n #endif\ndiff --git a/gettext.h b/gettext.h\nindex 57ba8bb..e44d8bc 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -30,10 +30,15 @@\n \n #ifndef NO_GETTEXT\n extern void git_setup_gettext(void);\n+extern int gettext_width(const char *s);\n #else\n static inline void git_setup_gettext(void)\n {\n }\n+static inline int gettext_width(const char *s)\n+{\n+\treturn strlen(s);\n+}\n #endif\n \n #ifdef GETTEXT_POISON\ndiff --git a/transport.h b/transport.h\nindex b866c12..a5d375e 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -106,6 +106,7 @@ struct transport {\n #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n+#define TRANSPORT_SUMMARY(x) (TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)\n \n /* Returns a transport suitable for the url */\n struct transport *transport_get(struct remote *, const char *);\n-- \n1.7.12.rc2.18.g61b472e\n"},{"id":"198397","messageId":"5047A602.1020802@web.de","threadId":"31416","inReplyTo":"504796A5.4070700@gmail.com","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-09-05T19:20:34Z","receivedAt":"2012-09-05T19:20:34Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 09/05/2012 08:15 PM, Torsten Bögershausen wrote:\n> On 04.09.12 12:39, Nguyễn Thái Ngọc Duy wrote:\n>> +/* return the number of columns of string 's' in current locale */\n>> +int gettext_width(const char *s)\n>> +{\n>> +\tstatic int is_utf8 = -1;\n>> +\tif (is_utf8 == -1)\n>> +\t\tis_utf8 = !strcmp(charset, \"UTF-8\");\n>> +\n>> +\treturn is_utf8 ? utf8_strwidth(s) : strlen(s);\n>\n> Will that work for non-ASCII encodings?\n> For ISO-8859-x we can say strlen() == strwidth(),\n> but for other encodings using multibytes that doesn't work, does it?\n(Sorry the message went out before completely written)\nSomething like that:\n\nint gettext_width(const char *s) {\n   static int is_utf8 = -1;\n\n   if (is_utf8 == -1)\n     is_utf8 = !strcmp(charset, \"UTF-8\");\n\n   if (is_utf8)\n     return utf8_strwidth(s);\n   else  {\n     char *s_utf = reencode_string(s, \"UTF-8\", charset);\n     if (s_utf) {\n       witdh = utf8_strwidth(s_utf);\n       free(s_utf);\n     } else\n       width = strlen(s);\n\n     return width;\n}\n"},{"id":"198433","messageId":"CACsJy8A60Os1pupMQdi44vnmO+E_ZqtvTELCOR1GeHSPdZeo5g@mail.gmail.com","threadId":"31416","inReplyTo":"5047A602.1020802@web.de","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-06T12:11:13Z","receivedAt":"2012-09-06T12:11:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Sep 6, 2012 at 2:20 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n> On 09/05/2012 08:15 PM, Torsten Bögershausen wrote:\n>>\n>> On 04.09.12 12:39, Nguyễn Thái Ngọc Duy wrote:\n>>>\n>>> +/* return the number of columns of string 's' in current locale */\n>>> +int gettext_width(const char *s)\n>>> +{\n>>> +       static int is_utf8 = -1;\n>>> +       if (is_utf8 == -1)\n>>> +               is_utf8 = !strcmp(charset, \"UTF-8\");\n>>> +\n>>> +       return is_utf8 ? utf8_strwidth(s) : strlen(s);\n>>\n>>\n>> Will that work for non-ASCII encodings?\n>> For ISO-8859-x we can say strlen() == strwidth(),\n>> but for other encodings using multibytes that doesn't work, does it?\n\nNo it does not. I think I mentioned that in the first version that I\nwas only interested in utf-8. Others can extend the function for their\nfavourite encodings.\n\n> (Sorry the message went out before completely written)\n> Something like that:\n>\n> int gettext_width(const char *s) {\n>\n>   static int is_utf8 = -1;\n>\n>   if (is_utf8 == -1)\n>\n>     is_utf8 = !strcmp(charset, \"UTF-8\");\n>\n>   if (is_utf8)\n>     return utf8_strwidth(s);\n>   else  {\n>     char *s_utf = reencode_string(s, \"UTF-8\", charset);\n>     if (s_utf) {\n>       witdh = utf8_strwidth(s_utf);\n>       free(s_utf);\n>     } else\n>       width = strlen(s);\n>\n>     return width;\n> }\n\nYes, something like that, assuming that column information is intact\nafter the conversion. Maybe you can make that a new function, int\nstrwidth(const char *str, const char *charset), and make\ngettext_strwidth() a thin wrapper:\n\nint gettext_strwidth(const char *s)\n{\n   return strwidth(s, charset);\n}\n-- \nDuy\n"},{"id":"198456","messageId":"CACsJy8D3VyFk+0PTeeXTN-Js9x7p+a+2vW+trry8-6q+9=m2xg@mail.gmail.com","threadId":"31416","inReplyTo":"5047A602.1020802@web.de","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-06T15:36:17Z","receivedAt":"2012-09-06T15:36:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Sep 6, 2012 at 2:20 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n>> Will that work for non-ASCII encodings?\n>> For ISO-8859-x we can say strlen() == strwidth(),\n>> but for other encodings using multibytes that doesn't work, does it?\n\nBTW if you are interested in supporting non-utf8 output, you may want\nto look at 1452bd6 (branch -v: align even when branch names are in\nUTF-8 - 2012-08-26), which assumes branches are in utf-8. So you have\nto convert them to output charset before printing.\n-- \nDuy\n"},{"id":"198457","messageId":"B268CEF3-88A6-49B0-8AE7-7C06EDAE7CEF@web.de","threadId":"31416","inReplyTo":"CACsJy8D3VyFk+0PTeeXTN-Js9x7p+a+2vW+trry8-6q+9=m2xg@mail.gmail.com","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-09-06T16:21:18Z","receivedAt":"2012-09-06T16:21:18Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\nAm 06.09.2012 um 17:36 schrieb Nguyen Thai Ngoc Duy:\n\n> On Thu, Sep 6, 2012 at 2:20 AM, Torsten Bögershausen <tboegi@web.de> wrote:\n>>> Will that work for non-ASCII encodings?\n>>> For ISO-8859-x we can say strlen() == strwidth(),\n>>> but for other encodings using multibytes that doesn't work, does it?\n> \n> BTW if you are interested in supporting non-utf8 output, you may want\n> to look at 1452bd6 (branch -v: align even when branch names are in\n> UTF-8 - 2012-08-26), which assumes branches are in utf-8. So you have\n> to convert them to output charset before printing.\n> -- \n> Duy\n\nThanks,\nI try to re-phrase my question:\n\nDo installations still exist which use e.g. BIG5 or any other\nmulti byte encoding which is not UTF-8?\n\nDo we want to support other encodings than ASCII or UTF-8?\n(Because then the screen width needs to be calculate different, I think)\n\n/Torsten\n"},{"id":"198468","messageId":"7v1uif3tyr.fsf@alter.siamese.dyndns.org","threadId":"31416","inReplyTo":"B268CEF3-88A6-49B0-8AE7-7C06EDAE7CEF@web.de","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-06T18:08:12Z","receivedAt":"2012-09-06T18:08:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> I try to re-phrase my question:\n>\n> Do installations still exist which use e.g. BIG5 or any other\n> multi byte encoding which is not UTF-8?\n\nYes.\n\n> Do we want to support other encodings than ASCII or UTF-8?\n> (Because then the screen width needs to be calculate different, I think)\n\nThat depends on who \"we\" are and what timeframe you have in mind.\n\nDo our developers care about these encodings so much that we would\nreject \"ASCCI/UTF-8 only\" patch and wait until we enhance it to do\nthe right thing for other encodings that we do not use ourselves?\nNo.  That does not make any sense, especially when we know we will\nnot have a good test coverage on the additional parts that we will\nnot be using ourselves.\n\n\"This change only helps people with ASCII or UTF-8 and does not help\nothers\" alone is never a valid reason to reject a change, but we\nstill try to be nicer to \"others\" that may come after we leave this\ntopic behind by doing a few things:\n\n - If the change will make things worse than it currently is for\n   \"others\", we would try to minimize the regression for them.\n\n - If the change will make the code harder to update later to\n   enhance with additional change to support \"others\", we would try\n   to anticipate what kind of changes are needed on top, and\n   structure the code in such a way that future changes can be made\n   cleanly.\n\nFor the first point, for multi-byte encodings (e.g. ISO-2022), the\ndisplay columns and byte length do not match and in general byte\nlength is longer than the display columns in the current code.  With\nthe current code that measures the required columns across elements\nby taking the maximum of byte length, they will see wrong number of\nfiller, so they are already getting a wrong alignment.  With the\n\"UTF-8 only\" change, the required columns and the filler will be\ncalculated by assuming that the string is in UTF-8, which may make\nthe computation off in a different way, and if we underestimate the\ndisplay columns for a string, they may see the strings truncated,\nwhich is bad.\n\nSo as long as gettext_width() punts and returns strlen() for a\nmalformed UTF-8, it would be OK [*1*].\n\nFor the second point, I think the API \"here is a string, give me the\nnumber of display columns it will occupy, as I am interested in\naligning them\" is a good abstraction that can be later enhanced to\nother encodings fairly easily, so I do not see a problem in the\npatch that goes in that direction.\n\n\n\n[Footnote]\n\n*1* If the patch used utf_strwidth() (which I didn't bother to go\nback and check, though), it should be OK.  The underlying\nutf8_width() will reject a malformed UTF-8 sequence and the code\nfalls back to strlen().\n"},{"id":"198836","messageId":"CACsJy8APtTU-7ZfTdcjg0nKoBSboNpP+7Va=akXcvn95Sd166A@mail.gmail.com","threadId":"31416","inReplyTo":"1346755175-31468-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-09-12T14:02:44Z","receivedAt":"2012-09-12T14:02:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Ping.. what happens to this patch? Do you want to support other\nencodings as well via iconv()? I can't test that though.\n\nOn Tue, Sep 4, 2012 at 5:39 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> fetch does printf(\"%-*s\", width, \"foo\") where \"foo\" can be a utf-8\n> string, but width is in bytes, not columns. For ASCII it's fine as one\n> byte takes one column. For utf-8, this may result in misaligned ref\n> summary table.\n>\n> Introduce gettext_width() function that returns the string length in\n> columns (currently only supports utf-8 locales). Make the code use\n> TRANSPORT_SUMMARY(x) where the length is compensated properly in\n> non-English locales.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>   - rename gettext_length() to gettext_width()\n>   - use \"width\" instead of letters\n>   - leave other \"%-*s\" places unchanged if they always take ascii\n>     strings (i.e. no _() calls)\n>   - note to self, may need to i18n-ize print_ref_status() in\n>     transport.c, looks like it's used by git-push only\n>\n>  builtin/fetch.c | 15 +++++++--------\n>  gettext.c       | 15 +++++++++++++--\n>  gettext.h       |  5 +++++\n>  transport.h     |  1 +\n>  4 files changed, 26 insertions(+), 10 deletions(-)\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index bb9a074..85e291f 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -255,9 +255,8 @@ static int update_local_ref(struct ref *ref,\n>         if (!hashcmp(ref->old_sha1, ref->new_sha1)) {\n>                 if (verbosity > 0)\n>                         strbuf_addf(display, \"= %-*s %-*s -> %s\",\n> -                                   TRANSPORT_SUMMARY_WIDTH,\n> -                                   _(\"[up to date]\"), REFCOL_WIDTH,\n> -                                   remote, pretty_ref);\n> +                                   TRANSPORT_SUMMARY(_(\"[up to date]\")),\n> +                                   REFCOL_WIDTH, remote, pretty_ref);\n>                 return 0;\n>         }\n>\n> @@ -271,7 +270,7 @@ static int update_local_ref(struct ref *ref,\n>                  */\n>                 strbuf_addf(display,\n>                             _(\"! %-*s %-*s -> %s  (can't fetch in current branch)\"),\n> -                           TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n> +                           TRANSPORT_SUMMARY(_(\"[rejected]\")),\n>                             REFCOL_WIDTH, remote, pretty_ref);\n>                 return 1;\n>         }\n> @@ -282,7 +281,7 @@ static int update_local_ref(struct ref *ref,\n>                 r = s_update_ref(\"updating tag\", ref, 0);\n>                 strbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n>                             r ? '!' : '-',\n> -                           TRANSPORT_SUMMARY_WIDTH, _(\"[tag update]\"),\n> +                           TRANSPORT_SUMMARY(_(\"[tag update]\")),\n>                             REFCOL_WIDTH, remote, pretty_ref,\n>                             r ? _(\"  (unable to update local ref)\") : \"\");\n>                 return r;\n> @@ -317,7 +316,7 @@ static int update_local_ref(struct ref *ref,\n>                 r = s_update_ref(msg, ref, 0);\n>                 strbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n>                             r ? '!' : '*',\n> -                           TRANSPORT_SUMMARY_WIDTH, what,\n> +                           TRANSPORT_SUMMARY(what),\n>                             REFCOL_WIDTH, remote, pretty_ref,\n>                             r ? _(\"  (unable to update local ref)\") : \"\");\n>                 return r;\n> @@ -357,7 +356,7 @@ static int update_local_ref(struct ref *ref,\n>                 return r;\n>         } else {\n>                 strbuf_addf(display, \"! %-*s %-*s -> %s  %s\",\n> -                           TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n> +                           TRANSPORT_SUMMARY(_(\"[rejected]\")),\n>                             REFCOL_WIDTH, remote, pretty_ref,\n>                             _(\"(non-fast-forward)\"));\n>                 return 1;\n> @@ -554,7 +553,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n>                         result |= delete_ref(ref->name, NULL, 0);\n>                 if (verbosity >= 0) {\n>                         fprintf(stderr, \" x %-*s %-*s -> %s\\n\",\n> -                               TRANSPORT_SUMMARY_WIDTH, _(\"[deleted]\"),\n> +                               TRANSPORT_SUMMARY(_(\"[deleted]\")),\n>                                 REFCOL_WIDTH, _(\"(none)\"), prettify_refname(ref->name));\n>                         warn_dangling_symref(stderr, dangling_msg, ref->name);\n>                 }\n> diff --git a/gettext.c b/gettext.c\n> index f75bca7..71e9545 100644\n> --- a/gettext.c\n> +++ b/gettext.c\n> @@ -4,6 +4,8 @@\n>\n>  #include \"git-compat-util.h\"\n>  #include \"gettext.h\"\n> +#include \"strbuf.h\"\n> +#include \"utf8.h\"\n>\n>  #ifndef NO_GETTEXT\n>  #      include <locale.h>\n> @@ -27,10 +29,9 @@ int use_gettext_poison(void)\n>  #endif\n>\n>  #ifndef NO_GETTEXT\n> +static const char *charset;\n>  static void init_gettext_charset(const char *domain)\n>  {\n> -       const char *charset;\n> -\n>         /*\n>            This trick arranges for messages to be emitted in the user's\n>            requested encoding, but avoids setting LC_CTYPE from the\n> @@ -128,4 +129,14 @@ void git_setup_gettext(void)\n>         init_gettext_charset(\"git\");\n>         textdomain(\"git\");\n>  }\n> +\n> +/* return the number of columns of string 's' in current locale */\n> +int gettext_width(const char *s)\n> +{\n> +       static int is_utf8 = -1;\n> +       if (is_utf8 == -1)\n> +               is_utf8 = !strcmp(charset, \"UTF-8\");\n> +\n> +       return is_utf8 ? utf8_strwidth(s) : strlen(s);\n> +}\n>  #endif\n> diff --git a/gettext.h b/gettext.h\n> index 57ba8bb..e44d8bc 100644\n> --- a/gettext.h\n> +++ b/gettext.h\n> @@ -30,10 +30,15 @@\n>\n>  #ifndef NO_GETTEXT\n>  extern void git_setup_gettext(void);\n> +extern int gettext_width(const char *s);\n>  #else\n>  static inline void git_setup_gettext(void)\n>  {\n>  }\n> +static inline int gettext_width(const char *s)\n> +{\n> +       return strlen(s);\n> +}\n>  #endif\n>\n>  #ifdef GETTEXT_POISON\n> diff --git a/transport.h b/transport.h\n> index b866c12..a5d375e 100644\n> --- a/transport.h\n> +++ b/transport.h\n> @@ -106,6 +106,7 @@ struct transport {\n>  #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256\n>\n>  #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n> +#define TRANSPORT_SUMMARY(x) (TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)\n>\n>  /* Returns a transport suitable for the url */\n>  struct transport *transport_get(struct remote *, const char *);\n> --\n> 1.7.12.rc2.18.g61b472e\n>\n\n\n\n-- \nDuy\n"},{"id":"198863","messageId":"7vpq5rce6y.fsf@alter.siamese.dyndns.org","threadId":"31416","inReplyTo":"CACsJy8APtTU-7ZfTdcjg0nKoBSboNpP+7Va=akXcvn95Sd166A@mail.gmail.com","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-12T18:02:13Z","receivedAt":"2012-09-12T18:02:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> Ping.. what happens to this patch? Do you want to support other\n> encodings as well via iconv()? I can't test that though.\n\nI thought I refuted a potential blocker, which was an implied\nobjection from Torsten, in $gmane/204912 already.  As long as we\nmake it clear that your patch helps only \"ASCII/UTF-8 only\" audience\nbut we still \"try to be nicer to 'others'\" by doing two things I\nsaid in the message, I think we should proceed without iconv() to\nkeep things simple.\n"},{"id":"198878","messageId":"5050EC92.6000103@web.de","threadId":"31416","inReplyTo":"7vpq5rce6y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] fetch: align new ref summary printout in UTF-8 locales","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-09-12T20:12:02Z","receivedAt":"2012-09-12T20:12:02Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 12.09.12 20:02, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n> \n>> Ping.. what happens to this patch? Do you want to support other\n>> encodings as well via iconv()? I can't test that though.\n> \n> I thought I refuted a potential blocker, which was an implied\n> objection from Torsten, in $gmane/204912 already.  As long as we\n> make it clear that your patch helps only \"ASCII/UTF-8 only\" audience\n> but we still \"try to be nicer to 'others'\" by doing two things I\n> said in the message, I think we should proceed without iconv() to\n> keep things simple.\n\nPlease unblock and proceed (as I can't test other encodings either)\n\nAnd even special thanks for the excellent lines from Junio,\nwhich explained the update philosophy in git so well, \nthat I take the freedom to re-post them here:\n\n>> I try to re-phrase my question:\n>>\n>> Do installations still exist which use e.g. BIG5 or any other\n>> multi byte encoding which is not UTF-8?\n>\n>Yes.\n>\n>> Do we want to support other encodings than ASCII or UTF-8?\n>> (Because then the screen width needs to be calculate different, I think)\n>\n>That depends on who \"we\" are and what timeframe you have in mind.\n>\n>Do our developers care about these encodings so much that we would\n>reject \"ASCCI/UTF-8 only\" patch and wait until we enhance it to do\n>the right thing for other encodings that we do not use ourselves?\n>No.  That does not make any sense, especially when we know we will\n>not have a good test coverage on the additional parts that we will\n>not be using ourselves.\n>\n>\"This change only helps people with ASCII or UTF-8 and does not help\n>others\" alone is never a valid reason to reject a change, but we\n>still try to be nicer to \"others\" that may come after we leave this\n>topic behind by doing a few things:\n>\n> - If the change will make things worse than it currently is for\n>   \"others\", we would try to minimize the regression for them.\n>\n> - If the change will make the code harder to update later to\n>   enhance with additional change to support \"others\", we would try\n>   to anticipate what kind of changes are needed on top, and\n>   structure the code in such a way that future changes can be made\n>   cleanly.\n>\n>For the first point, for multi-byte encodings (e.g. ISO-2022), the\n>display columns and byte length do not match and in general byte\n>length is longer than the display columns in the current code.  With\n>the current code that measures the required columns across elements\n>by taking the maximum of byte length, they will see wrong number of\n>filler, so they are already getting a wrong alignment.  With the\n>\"UTF-8 only\" change, the required columns and the filler will be\n>calculated by assuming that the string is in UTF-8, which may make\n>the computation off in a different way, and if we underestimate the\n>display columns for a string, they may see the strings truncated,\n>which is bad.\n>\n>So as long as gettext_width() punts and returns strlen() for a\n>malformed UTF-8, it would be OK [*1*].\n>\n>For the second point, I think the API \"here is a string, give me the\n>number of display columns it will occupy, as I am interested in\n>aligning them\" is a good abstraction that can be later enhanced to\n>other encodings fairly easily, so I do not see a problem in the\n>patch that goes in that direction.\n>\n>\n>\n>[Footnote]\n>\n>*1* If the patch used utf_strwidth() (which I didn't bother to go\n>back and check, though), it should be OK.  The underlying\n>utf8_width() will reject a malformed UTF-8 sequence and the code\n>falls back to strlen().\n\n \n"}]}