{"thread":{"id":"32831","subject":"[PATCH] Get correct column with for options in command usage","startedAt":"2013-02-05T07:40:32Z","lastAt":"2013-02-09T06:31:09Z","messageCount":20,"participants":["Jiang Xin","Duy Nguyen","Junio C Hamano","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"208690","messageId":"0b035132df6de2cac56ac59d66b04f30e90ad760.1360049671.git.worldhello.net@gmail.com","threadId":"32831","inReplyTo":null,"subject":"[PATCH] Get correct column with for options in command usage","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-05T07:40:32Z","receivedAt":"2013-02-05T07:40:32Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Command usage would not align well if command options are translated,\nespecially to CJK. Call utf8_strwidth in function usage_argh, so that\nthe caller will get correct column width.\n\nSigned-off-by: Jiang Xin <worldhello.net@gmail.com>\n---\n parse-options.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 67e98..ca0e6 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -473,7 +473,7 @@ int parse_options(int argc, const char **argv, const char *prefix,\n \n static int usage_argh(const struct option *opts, FILE *outfile)\n {\n-\tconst char *s;\n+\tconst char *s, *p;\n \tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n \tif (opts->flags & PARSE_OPT_OPTARG)\n \t\tif (opts->long_name)\n@@ -482,7 +482,9 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n \telse\n \t\ts = literal ? \" %s\" : \" <%s>\";\n-\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n+\tp = opts->argh ? _(opts->argh) : _(\"...\");\n+\tfprintf(outfile, s, p);\n+\treturn utf8_strwidth(p) + strlen(s) - 2;\n }\n \n #define USAGE_OPTS_WIDTH 24\n-- \n1.8.1.1.367.g57acac9.dirty\n"},{"id":"208722","messageId":"20130205121552.GA16601@lanh","threadId":"32831","inReplyTo":"0b035132df6de2cac56ac59d66b04f30e90ad760.1360049671.git.worldhello.net@gmail.com","subject":"Re: [PATCH] Get correct column with for options in command usage","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-05T12:15:52Z","receivedAt":"2013-02-05T12:15:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 05, 2013 at 03:40:32PM +0800, Jiang Xin wrote:\n> Command usage would not align well if command options are translated,\n> especially to CJK. Call utf8_strwidth in function usage_argh, so that\n> the caller will get correct column width.\n\nYeah, I just noticed a misalignment in Vietnamese translation (just\ntwo spaces to the left, hard to notice unless paying attention).\n\n>  static int usage_argh(const struct option *opts, FILE *outfile)\n>  {\n> -\tconst char *s;\n> +\tconst char *s, *p;\n>  \tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n>  \tif (opts->flags & PARSE_OPT_OPTARG)\n>  \t\tif (opts->long_name)\n> @@ -482,7 +482,9 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n>  \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n>  \telse\n>  \t\ts = literal ? \" %s\" : \" <%s>\";\n> -\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n> +\tp = opts->argh ? _(opts->argh) : _(\"...\");\n> +\tfprintf(outfile, s, p);\n> +\treturn utf8_strwidth(p) + strlen(s) - 2;\n>  }\n\nFirst of all, #include \"utf8.h\" is required for utf8_strwidth().\n\nThe \"strlen(s) - 2\" is quite sensitive to how \"s\" is written. How\nabout this? A bit longer but clearer. Your version is OK too with a\ncomment explaining \"strlen(s) - 2\".\n\n-- 8< --\ndiff --git a/parse-options.c b/parse-options.c\nindex 67e98a6..0f803bd 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -474,6 +474,8 @@ int parse_options(int argc, const char **argv, const char *prefix,\n static int usage_argh(const struct option *opts, FILE *outfile)\n {\n \tconst char *s;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint columns;\n \tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n \tif (opts->flags & PARSE_OPT_OPTARG)\n \t\tif (opts->long_name)\n@@ -482,7 +484,11 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n \telse\n \t\ts = literal ? \" %s\" : \" <%s>\";\n-\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n+\tstrbuf_addf(&sb, s, opts->argh ? _(opts->argh) : _(\"...\"));\n+\tfprintf(outfile, sb.buf);\n+\tcolumns = utf8_strwidth(sb.buf);\n+\tstrbuf_release(&sb);\n+\treturn columns;\n }\n \n #define USAGE_OPTS_WIDTH 24\n-- 8< --\n\nWhile looking at parse-options.c, I notice \"-NUM\" is not marked for\ntranslation. I think you might want to mark it so that you can\ntranslate \"NUM\" to a similar abbreviation in a local language. A\nsimilar patch like this is required so we get columns for \"-NUM\"\ntranslation instead of the number of bytes.\n--\nDuy\n"},{"id":"208723","messageId":"CACsJy8Ci05C1mKDGp_YzP0H6G0Y40K8MUUq+D6NzjtPGRNms0g@mail.gmail.com","threadId":"32831","inReplyTo":"20130205121552.GA16601@lanh","subject":"Re: [PATCH] Get correct column with for options in command usage","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-05T12:18:20Z","receivedAt":"2013-02-05T12:18:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 5, 2013 at 7:15 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> +       fprintf(outfile, sb.buf);\n\nUse fputs instead. I looked up fputs man page but somehow still left\nfprintf there.\n-- \nDuy\n"},{"id":"208737","messageId":"7v38xavj32.fsf@alter.siamese.dyndns.org","threadId":"32831","inReplyTo":"CACsJy8Ci05C1mKDGp_YzP0H6G0Y40K8MUUq+D6NzjtPGRNms0g@mail.gmail.com","subject":"Re: [PATCH] Get correct column with for options in command usage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T16:09:05Z","receivedAt":"2013-02-05T16:09:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Tue, Feb 5, 2013 at 7:15 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n>> +       fprintf(outfile, sb.buf);\n>\n> Use fputs instead. I looked up fputs man page but somehow still left\n> fprintf there.\n\nOnce the streams of \"oops that was wrong\" comments are done, I'd\nappreciate if one of you guys send a finished patch for application,\ninstead of forcing me to follow all the messages in the discussion\nand picking up pieces.\n\nThanks.\n"},{"id":"208740","messageId":"883efc2358a0deb48bee48134d45ddd528a732d3.1360080194.git.worldhello.net@gmail.com","threadId":"32831","inReplyTo":"20130205121552.GA16601@lanh","subject":"[PATCH v2 1/2] Get correct column with for options in command usage","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-05T16:16:55Z","receivedAt":"2013-02-05T16:16:55Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Command usage would not align well if command options are translated,\nespecially to CJK. Call utf8_strwidth in function usage_argh, so that\nthe caller will get correct column width.\n\nSigned-off-by: Jiang Xin <worldhello.net@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n parse-options.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 67e98..cd029f 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -3,6 +3,7 @@\n #include \"cache.h\"\n #include \"commit.h\"\n #include \"color.h\"\n+#include \"utf8.h\"\n \n static int parse_options_usage(struct parse_opt_ctx_t *ctx,\n \t\t\t       const char * const *usagestr,\n@@ -473,7 +474,7 @@ int parse_options(int argc, const char **argv, const char *prefix,\n \n static int usage_argh(const struct option *opts, FILE *outfile)\n {\n-\tconst char *s;\n+\tconst char *s, *p;\n \tint literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;\n \tif (opts->flags & PARSE_OPT_OPTARG)\n \t\tif (opts->long_name)\n@@ -482,7 +483,10 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n \telse\n \t\ts = literal ? \" %s\" : \" <%s>\";\n-\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n+\tp = opts->argh ? _(opts->argh) : _(\"...\");\n+\tfprintf(outfile, s, p);\n+\t/* Remove extra 2 chars (\"%s\" in s) to get column width of utf8 string */\n+\treturn utf8_strwidth(p) + strlen(s) - 2;\n }\n \n #define USAGE_OPTS_WIDTH 24\n-- \n1.8.1.1.368.g6034fad.dirty\n"},{"id":"208741","messageId":"8d6d4d869ea58e0a26b3bb6377fc102728948997.1360080194.git.worldhello.net@gmail.com","threadId":"32831","inReplyTo":"883efc2358a0deb48bee48134d45ddd528a732d3.1360080194.git.worldhello.net@gmail.com","subject":"[PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-05T16:16:56Z","receivedAt":"2013-02-05T16:16:56Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Signed-off-by: Jiang Xin <worldhello.net@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n parse-options.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex cd029f..be916 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -497,6 +497,8 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n \t\t\t\t       const struct option *opts, int full, int err)\n {\n \tFILE *outfile = err ? stderr : stdout;\n+\tconst char *opt_num_buff = _(\"-NUM\");\n+\tint opt_num_size = utf8_strwidth(opt_num_buff);\n \n \tif (!usagestr)\n \t\treturn PARSE_OPT_HELP;\n@@ -544,8 +546,10 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n \t\t\tpos += fprintf(outfile, \", \");\n \t\tif (opts->long_name)\n \t\t\tpos += fprintf(outfile, \"--%s\", opts->long_name);\n-\t\tif (opts->type == OPTION_NUMBER)\n-\t\t\tpos += fprintf(outfile, \"-NUM\");\n+\t\tif (opts->type == OPTION_NUMBER) {\n+\t\t\tfputs(opt_num_buff, outfile);\n+\t\t\tpos += opt_num_size;\n+\t\t}\n \n \t\tif ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n \t\t    !(opts->flags & PARSE_OPT_NOARG))\n-- \n1.8.1.1.368.g6034fad.dirty\n"},{"id":"208746","messageId":"7vd2weu1sq.fsf@alter.siamese.dyndns.org","threadId":"32831","inReplyTo":"8d6d4d869ea58e0a26b3bb6377fc102728948997.1360080194.git.worldhello.net@gmail.com","subject":"Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T17:07:49Z","receivedAt":"2013-02-05T17:07:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin <worldhello.net@gmail.com> writes:\n\n> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  parse-options.c | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/parse-options.c b/parse-options.c\n> index cd029f..be916 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -497,6 +497,8 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n>  \t\t\t\t       const struct option *opts, int full, int err)\n>  {\n>  \tFILE *outfile = err ? stderr : stdout;\n> +\tconst char *opt_num_buff = _(\"-NUM\");\n> +\tint opt_num_size = utf8_strwidth(opt_num_buff);\n>  \n>  \tif (!usagestr)\n>  \t\treturn PARSE_OPT_HELP;\n> @@ -544,8 +546,10 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n>  \t\t\tpos += fprintf(outfile, \", \");\n>  \t\tif (opts->long_name)\n>  \t\t\tpos += fprintf(outfile, \"--%s\", opts->long_name);\n> -\t\tif (opts->type == OPTION_NUMBER)\n> -\t\t\tpos += fprintf(outfile, \"-NUM\");\n> +\t\tif (opts->type == OPTION_NUMBER) {\n> +\t\t\tfputs(opt_num_buff, outfile);\n> +\t\t\tpos += opt_num_size;\n> +\t\t}\n\nI somehow suspect that this is going in a direction that makes this\npiece of code much less maintainable.\n\nLook at the entire function and see how many places you do fprintf\non strings that are marked with _().  short_name and long_name are\nnot likely to be translated, but everything else is, especially\nmultiple places that show _(opts->help) neither of these patches\ntouch.\n\nI wonder if it makes more sense to add a helper function that\nreturns the number of column positions (not bytes) with a signature\nsimilar to fprintf() and use that throughout the function instead.\n"},{"id":"208778","messageId":"CANYiYbF1cS=K9M0cwE5V0pUJMPEYGiJOjJwg5KQScCf8pjyTqw@mail.gmail.com","threadId":"32831","inReplyTo":"7vd2weu1sq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-06T00:15:33Z","receivedAt":"2013-02-06T00:15:33Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/2/6 Junio C Hamano <gitster@pobox.com>:\n> I somehow suspect that this is going in a direction that makes this\n> piece of code much less maintainable.\n>\n> Look at the entire function and see how many places you do fprintf\n> on strings that are marked with _().  short_name and long_name are\n> not likely to be translated, but everything else is, especially\n> multiple places that show _(opts->help) neither of these patches\n> touch.\n>\n> I wonder if it makes more sense to add a helper function that\n> returns the number of column positions (not bytes) with a signature\n> similar to fprintf() and use that throughout the function instead.\n\nI agree, a helper named 'utf8_fprintf' in utf8.c is better.\nI will send a patch latter.\n\n\n-- \nJiang Xin\n"},{"id":"208782","messageId":"00fcfef24710f03485186a23be82ca3c851d2a99.1360113349.git.worldhello.net@gmail.com","threadId":"32831","inReplyTo":"7vd2weu1sq.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3] Add utf8_fprintf helper which returns correct columns","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-06T01:16:37Z","receivedAt":"2013-02-06T01:16:37Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Since command usages can be translated, they may not align well especially\nwhen they are translated to CJK. A wrapper utf8_fprintf can help to return\nthe correct columns required.\n\nSigned-off-by: Jiang Xin <worldhello.net@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n parse-options.c |  5 +++--\n utf8.c          | 20 ++++++++++++++++++++\n utf8.h          |  1 +\n 3 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 67e98..a6ce9e 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -3,6 +3,7 @@\n #include \"cache.h\"\n #include \"commit.h\"\n #include \"color.h\"\n+#include \"utf8.h\"\n \n static int parse_options_usage(struct parse_opt_ctx_t *ctx,\n \t\t\t       const char * const *usagestr,\n@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n \telse\n \t\ts = literal ? \" %s\" : \" <%s>\";\n-\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n+\treturn utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n }\n \n #define USAGE_OPTS_WIDTH 24\n@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n \t\tif (opts->long_name)\n \t\t\tpos += fprintf(outfile, \"--%s\", opts->long_name);\n \t\tif (opts->type == OPTION_NUMBER)\n-\t\t\tpos += fprintf(outfile, \"-NUM\");\n+\t\t\tpos += utf8_fprintf(outfile, _(\"-NUM\"));\n \n \t\tif ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n \t\t    !(opts->flags & PARSE_OPT_NOARG))\ndiff --git a/utf8.c b/utf8.c\nindex a4ee6..52dbd 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -430,6 +430,26 @@ int same_encoding(const char *src, const char *dst)\n }\n \n /*\n+ * Wrapper for fprintf and returns the total number of columns required\n+ * for the printed string, assuming that the string is utf8.\n+ */\n+int utf8_fprintf(FILE *stream, const char *format, ...)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tva_list arg;\n+\tint columns;\n+\n+\tva_start (arg, format);\n+\tstrbuf_vaddf(&buf, format, arg);\n+\tva_end (arg);\n+\n+\tfputs(buf.buf, stream);\n+\tcolumns = utf8_strwidth(buf.buf);\n+\tstrbuf_release(&buf);\n+\treturn columns;\n+}\n+\n+/*\n  * Given a buffer and its encoding, return it re-encoded\n  * with iconv.  If the conversion fails, returns NULL.\n  */\ndiff --git a/utf8.h b/utf8.h\nindex a2142..501b2 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);\n int is_utf8(const char *text);\n int is_encoding_utf8(const char *name);\n int same_encoding(const char *, const char *);\n+int utf8_fprintf(FILE *, const char *, ...);\n \n void strbuf_add_wrapped_text(struct strbuf *buf,\n \t\tconst char *text, int indent, int indent2, int width);\n-- \n1.8.1.1.370.g47b6ee8.dirty\n"},{"id":"208783","messageId":"7vpq0enoui.fsf@alter.siamese.dyndns.org","threadId":"32831","inReplyTo":"CANYiYbF1cS=K9M0cwE5V0pUJMPEYGiJOjJwg5KQScCf8pjyTqw@mail.gmail.com","subject":"Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T02:44:05Z","receivedAt":"2013-02-06T02:44:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin <worldhello.net@gmail.com> writes:\n\n> 2013/2/6 Junio C Hamano <gitster@pobox.com>:\n>> I somehow suspect that this is going in a direction that makes this\n>> piece of code much less maintainable.\n>>\n>> Look at the entire function and see how many places you do fprintf\n>> on strings that are marked with _().  short_name and long_name are\n>> not likely to be translated, but everything else is, especially\n>> multiple places that show _(opts->help) neither of these patches\n>> touch.\n>>\n>> I wonder if it makes more sense to add a helper function that\n>> returns the number of column positions (not bytes) with a signature\n>> similar to fprintf() and use that throughout the function instead.\n>\n> I agree, a helper named 'utf8_fprintf' in utf8.c is better.\n> I will send a patch latter.\n\nYeah, the idea of a helper function I agree with; I am not thrilled\nwith the name utf8_fprintf() though.  People use the return value of\nfprintf() for error detection (negative return value means an error)\nmost of the time (even though non-negative value gives the number of\nbytes shown), but the primary use of the return value from the\nutf8_fprintf() function will be to get the display width, and the\nname does not quite capture that.\n"},{"id":"208784","messageId":"CANYiYbF8DCPxqGQ2AFFXpSm0nO+wFDg=qrn9C8uoZO6fj__NHA@mail.gmail.com","threadId":"32831","inReplyTo":"7vpq0enoui.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-06T03:02:27Z","receivedAt":"2013-02-06T03:02:27Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/2/6 Junio C Hamano <gitster@pobox.com>:\n> Jiang Xin <worldhello.net@gmail.com> writes:\n>> I agree, a helper named 'utf8_fprintf' in utf8.c is better.\n>> I will send a patch latter.\n>\n> Yeah, the idea of a helper function I agree with; I am not thrilled\n> with the name utf8_fprintf() though.  People use the return value of\n> fprintf() for error detection (negative return value means an error)\n> most of the time (even though non-negative value gives the number of\n> bytes shown), but the primary use of the return value from the\n> utf8_fprintf() function will be to get the display width, and the\n> name does not quite capture that.\n>\n\nHow about this since [PATCH v3]:\n\ndiff --git a/utf8.c b/utf8.c\nindex 52dbd..b893a 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)\n        strbuf_vaddf(&buf, format, arg);\n        va_end (arg);\n\n-       fputs(buf.buf, stream);\n-       columns = utf8_strwidth(buf.buf);\n+       columns = fputs(buf.buf, stream);\n+       /* If no error occurs, and really write something (columns > 0),\n+        * calculate really columns width with utf8_strwidth. */\n+       if (columns > 0)\n+               columns = utf8_strwidth(buf.buf);\n        strbuf_release(&buf);\n        return columns;\n }\n\n\n-- \n蒋鑫\n\n北京群英汇信息技术有限公司\n邮件: worldhello.net@gmail.com\n网址: http://www.ossxp.com/\n博客: http://www.worldhello.net/\n微博: http://weibo.com/gotgit/\n电话: 010-51262007, 18601196889\n"},{"id":"208785","messageId":"7vip66njpj.fsf@alter.siamese.dyndns.org","threadId":"32831","inReplyTo":"CANYiYbF8DCPxqGQ2AFFXpSm0nO+wFDg=qrn9C8uoZO6fj__NHA@mail.gmail.com","subject":"Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T04:35:04Z","receivedAt":"2013-02-06T04:35:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin <worldhello.net@gmail.com> writes:\n\n> 2013/2/6 Junio C Hamano <gitster@pobox.com>:\n>> Jiang Xin <worldhello.net@gmail.com> writes:\n>>> I agree, a helper named 'utf8_fprintf' in utf8.c is better.\n>>> I will send a patch latter.\n>>\n>> Yeah, the idea of a helper function I agree with; I am not thrilled\n>> with the name utf8_fprintf() though.  People use the return value of\n>> fprintf() for error detection (negative return value means an error)\n>> most of the time (even though non-negative value gives the number of\n>> bytes shown), but the primary use of the return value from the\n>> utf8_fprintf() function will be to get the display width, and the\n>> name does not quite capture that.\n>>\n>\n> How about this since [PATCH v3]:\n>\n> diff --git a/utf8.c b/utf8.c\n> index 52dbd..b893a 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)\n>         strbuf_vaddf(&buf, format, arg);\n>         va_end (arg);\n>\n> -       fputs(buf.buf, stream);\n> -       columns = utf8_strwidth(buf.buf);\n> +       columns = fputs(buf.buf, stream);\n> +       /* If no error occurs, and really write something (columns > 0),\n> +        * calculate really columns width with utf8_strwidth. */\n> +       if (columns > 0)\n> +               columns = utf8_strwidth(buf.buf);\n>         strbuf_release(&buf);\n>         return columns;\n>  }\n\nYeah, the error checking is necessary; it would make your intention\nmore clear to say:\n\n\tif (0 <= columns)\n\t\tcolumns = utf8_strwidth(buf.buf);\n\nthough, as buf.buf _may_ be an empty string, and with the \"if\"\nstatement you are saying \"we return the width only when output did\nnot result in an error\".\n\nThe above bugfix does not address my original concern about\nthe name, though.\n"},{"id":"208795","messageId":"CACsJy8DcXuFqjBtufQq1-0Vm3H4uxs03Crx+akY-kbrVgY8vUw@mail.gmail.com","threadId":"32831","inReplyTo":"7vip66njpj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-06T10:45:41Z","receivedAt":"2013-02-06T10:45:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 6, 2013 at 11:35 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> How about this since [PATCH v3]:\n>>\n>> diff --git a/utf8.c b/utf8.c\n>> index 52dbd..b893a 100644\n>> --- a/utf8.c\n>> +++ b/utf8.c\n>> @@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)\n>>         strbuf_vaddf(&buf, format, arg);\n>>         va_end (arg);\n>>\n>> -       fputs(buf.buf, stream);\n>> -       columns = utf8_strwidth(buf.buf);\n>> +       columns = fputs(buf.buf, stream);\n>> +       /* If no error occurs, and really write something (columns > 0),\n>> +        * calculate really columns width with utf8_strwidth. */\n>> +       if (columns > 0)\n>> +               columns = utf8_strwidth(buf.buf);\n>>         strbuf_release(&buf);\n>>         return columns;\n>>  }\n>\n> The above bugfix does not address my original concern about\n> the name, though.\n\nHow about utf8_fwprintf? wprintf() deals with wide characters and\nreturns the number of wide characters, I think the name fits. And we\ncould just drop utf8_ and use the existing name, because we don't use\nwchar_t* anyway.\n-- \nDuy\n"},{"id":"208812","messageId":"7va9rho350.fsf@alter.siamese.dyndns.org","threadId":"32831","inReplyTo":"CACsJy8DcXuFqjBtufQq1-0Vm3H4uxs03Crx+akY-kbrVgY8vUw@mail.gmail.com","subject":"Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T15:47:39Z","receivedAt":"2013-02-06T15:47:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> How about utf8_fwprintf? wprintf() deals with wide characters and\n> returns the number of wide characters, I think the name fits. And we\n> could just drop utf8_ and use the existing name, because we don't use\n> wchar_t* anyway.\n\nPlease, no.  That line of reasoning shows a horrible design taste\n(or lack of taste).  \"We don't use X right now\" (or \"We will promise\nnever to use X\", for that matter) is never a good reason to abuse a\nname that normal people would closely associate with X to something\nthat is completely different.  That leads to more confusion, not\nless.\n\nI guess utf8_fprintf() is not so bad after all.  fprintf() without\nthe utf8_ prefix is perfectly capable of showing a string encoded in\nUTF-8, and anybody can correctly guess that the magic utf8_ prefix\nwould introduce (i.e. the difference between utf8_fprintf and\nfprintf) can only be about the return value.  It can be reasonably\nexpected that everybody would then know that the display column\ncount can be the only sane return value that is different from what\nfprintf() would return.\n"},{"id":"208964","messageId":"4ea03e99bad13e2910b137fd3991951244fa23f1.1360289411.git.worldhello.net@gmail.com","threadId":"32831","inReplyTo":"7va9rho350.fsf@alter.siamese.dyndns.org","subject":"[PATCH v4] Add utf8_fprintf helper which returns correct columns","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-08T02:10:11Z","receivedAt":"2013-02-08T02:10:11Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Since command usages can be translated, they may not align well especially\nwhen they are translated to CJK. A wrapper utf8_fprintf can help to return\nthe correct columns required.\n\nSigned-off-by: Jiang Xin <worldhello.net@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n parse-options.c |  5 +++--\n utf8.c          | 22 ++++++++++++++++++++++\n utf8.h          |  1 +\n 3 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 67e98..a6ce9e 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -3,6 +3,7 @@\n #include \"cache.h\"\n #include \"commit.h\"\n #include \"color.h\"\n+#include \"utf8.h\"\n \n static int parse_options_usage(struct parse_opt_ctx_t *ctx,\n \t\t\t       const char * const *usagestr,\n@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n \telse\n \t\ts = literal ? \" %s\" : \" <%s>\";\n-\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n+\treturn utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n }\n \n #define USAGE_OPTS_WIDTH 24\n@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n \t\tif (opts->long_name)\n \t\t\tpos += fprintf(outfile, \"--%s\", opts->long_name);\n \t\tif (opts->type == OPTION_NUMBER)\n-\t\t\tpos += fprintf(outfile, \"-NUM\");\n+\t\t\tpos += utf8_fprintf(outfile, _(\"-NUM\"));\n \n \t\tif ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n \t\t    !(opts->flags & PARSE_OPT_NOARG))\ndiff --git a/utf8.c b/utf8.c\nindex a4ee6..05925 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -430,6 +430,28 @@ int same_encoding(const char *src, const char *dst)\n }\n \n /*\n+ * Wrapper for fprintf and returns the total number of columns required\n+ * for the printed string, assuming that the string is utf8.\n+ */\n+int utf8_fprintf(FILE *stream, const char *format, ...)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tva_list arg;\n+\tint columns;\n+\n+\tva_start (arg, format);\n+\tstrbuf_vaddf(&buf, format, arg);\n+\tva_end (arg);\n+\n+\tcolumns = fputs(buf.buf, stream);\n+\t/* If no error occurs, returns columns really required with utf8_strwidth. */\n+\tif (0 <= columns)\n+\t\tcolumns = utf8_strwidth(buf.buf);\n+\tstrbuf_release(&buf);\n+\treturn columns;\n+}\n+\n+/*\n  * Given a buffer and its encoding, return it re-encoded\n  * with iconv.  If the conversion fails, returns NULL.\n  */\ndiff --git a/utf8.h b/utf8.h\nindex a2142..501b2 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);\n int is_utf8(const char *text);\n int is_encoding_utf8(const char *name);\n int same_encoding(const char *, const char *);\n+int utf8_fprintf(FILE *, const char *, ...);\n \n void strbuf_add_wrapped_text(struct strbuf *buf,\n \t\tconst char *text, int indent, int indent2, int width);\n-- \n1.8.1.1.368.g917cd65\n"},{"id":"208974","messageId":"51149542.8060307@web.de","threadId":"32831","inReplyTo":"4ea03e99bad13e2910b137fd3991951244fa23f1.1360289411.git.worldhello.net@gmail.com","subject":"Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-02-08T06:03:46Z","receivedAt":"2013-02-08T06:03:46Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 08.02.13 03:10, Jiang Xin wrote:\n> Since command usages can be translated, they may not align well especially\n> when they are translated to CJK. A wrapper utf8_fprintf can help to return\n> the correct columns required.\n> \n> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  parse-options.c |  5 +++--\n>  utf8.c          | 22 ++++++++++++++++++++++\n>  utf8.h          |  1 +\n>  3 files changed, 26 insertions(+), 2 deletions(-)\n> \n> diff --git a/parse-options.c b/parse-options.c\n> index 67e98..a6ce9e 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -3,6 +3,7 @@\n>  #include \"cache.h\"\n>  #include \"commit.h\"\n>  #include \"color.h\"\n> +#include \"utf8.h\"\n>  \n>  static int parse_options_usage(struct parse_opt_ctx_t *ctx,\n>  \t\t\t       const char * const *usagestr,\n> @@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n>  \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n>  \telse\n>  \t\ts = literal ? \" %s\" : \" <%s>\";\n> -\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n> +\treturn utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n>  }\n>  \n>  #define USAGE_OPTS_WIDTH 24\n> @@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n>  \t\tif (opts->long_name)\n>  \t\t\tpos += fprintf(outfile, \"--%s\", opts->long_name);\n>  \t\tif (opts->type == OPTION_NUMBER)\n> -\t\t\tpos += fprintf(outfile, \"-NUM\");\n> +\t\t\tpos += utf8_fprintf(outfile, _(\"-NUM\"));\n>  \n>  \t\tif ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n>  \t\t    !(opts->flags & PARSE_OPT_NOARG))\n> diff --git a/utf8.c b/utf8.c\n> index a4ee6..05925 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -430,6 +430,28 @@ int same_encoding(const char *src, const char *dst)\n>  }\n>  \n>  /*\n> + * Wrapper for fprintf and returns the total number of columns required\n> + * for the printed string, assuming that the string is utf8.\n> + */\n> +int utf8_fprintf(FILE *stream, const char *format, ...)\n> +{\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tva_list arg;\n> +\tint columns;\n> +\n> +\tva_start (arg, format);\n> +\tstrbuf_vaddf(&buf, format, arg);\n> +\tva_end (arg);\n> +\n> +\tcolumns = fputs(buf.buf, stream);\n> +\t/* If no error occurs, returns columns really required with utf8_strwidth. */\n> +\tif (0 <= columns)\n> +\t\tcolumns = utf8_strwidth(buf.buf);\n> +\tstrbuf_release(&buf);\n> +\treturn columns;\n> +}\n> +\n\nI don't think we handle the return code from fputs() correctly.\n\nPlease dee below for specifications on fprintf(),\nsomething like the following could do:\n \nint utf8_fprintf(FILE *stream, const char *format, ...)\n{\n\tstruct strbuf buf = STRBUF_INIT;\n\tva_list arg;\n\tint columns = 0;\n\n\tva_start (arg, format);\n\tstrbuf_vaddf(&buf, format, arg);\n\tva_end (arg);\n\n\tif (EOF != fputs(buf.buf, stream))\n\t\tcolumns = utf8_strwidth(buf.buf);\n\tstrbuf_release(&buf);\n\treturn columns;\n}\n\nAnd as a side note: would fprintf_strwidth() be a better name?\n\n\nLinux:\nRETURN VALUE\n       fputc(), putc() and  putchar()  return  the  character  written  as  an\n       unsigned char cast to an int or EOF on error.\n\n       puts()  and  fputs()  return a nonnegative number on success, or EOF on\n       error.\n\nMac OS:\nCOMPATIBILITY\n     fputs() now returns a non-negative number (as opposed to 0) on successful\n     completion.  As a result, many tests (e.g., \"fputs() == 0\", \"fputs() !=\n     0\") do not give the desired result.  Use \"fputs() != EOF\" or \"fputs() ==\n     EOF\" to determine success or failure.\n\nPosix:\nRETURN VALUE\n       Upon successful completion, fputs() shall return a non-negative number.\n       Otherwise, it shall return EOF, set an error indicator for the  stream,\n       and set errno to indicate the error.\n"},{"id":"208977","messageId":"51149770.5070303@web.de","threadId":"32831","inReplyTo":"51149542.8060307@web.de","subject":"Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-02-08T06:13:04Z","receivedAt":"2013-02-08T06:13:04Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"(Sorry for confusing: I should have written:)\n\nPlease see below for specifications on fputs()\n\nLinux:\nRETURN VALUE\nfputc(), putc() and putchar() return the character written as an unsigned char cast to an int or EOF on error.\nputs() and fputs() return a nonnegative number on success, or EOF on error.\n\nMac OS:\nCOMPATIBILITY\nfputs() now returns a non-negative number (as opposed to 0) on successful completion. As a result, many tests (e.g., \"fputs() == 0\", \"fputs() != 0\") do not give the desired result.\nUse \"fputs() != EOF\" or \"fputs() == EOF\" to determine success or failure.\n\nPosix:\nRETURN VALUE\nUpon successful completion, fputs() shall return a non-negative number. Otherwise, it shall return EOF, set an error indicator for the stream, and set errno to indicate the error.\n"},{"id":"208984","messageId":"CANYiYbGZEJXDjzBJa_qbQCgw9tcTMfOChNr6ANwaLvo=fB=bcQ@mail.gmail.com","threadId":"32831","inReplyTo":"51149542.8060307@web.de","subject":"Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-08T07:20:26Z","receivedAt":"2013-02-08T07:20:26Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/2/8 Torsten Bögershausen <tboegi@web.de>:\n> On 08.02.13 03:10, Jiang Xin wrote:\n>> +     /* If no error occurs, returns columns really required with utf8_strwidth. */\n>> +     if (0 <= columns)\n>> +             columns = utf8_strwidth(buf.buf);\n>> +     strbuf_release(&buf);\n>> +     return columns;\n>> +}\n>> +\n>\n> I don't think we handle the return code from fputs() correctly.\n>\n> Please dee below for specifications on fprintf(),\n> something like the following could do:\n>\n> int utf8_fprintf(FILE *stream, const char *format, ...)\n> {\n>         struct strbuf buf = STRBUF_INIT;\n>         va_list arg;\n>         int columns = 0;\n>\n>         va_start (arg, format);\n>         strbuf_vaddf(&buf, format, arg);\n>         va_end (arg);\n>\n>         if (EOF != fputs(buf.buf, stream))\n>                 columns = utf8_strwidth(buf.buf);\n>         strbuf_release(&buf);\n>         return columns;\n> }\n\nAs fputs() returns a non-negative number (as opposed to 0) on\nsuccessful completion,\nTest fputs() return value as \"fputs() >=0\" is correct, while \"fputs()\n== 0\", \"fputs() != 0\"\nare wrong. I think it's OK, must I send a new re-roll for this?\n\nEOF is defined as (-1) in stdio.h:\n\n    #define EOF     (-1)\n\n> And as a side note: would fprintf_strwidth() be a better name?\n\nThis is a nice candidate.\n\n\n-- \nJiang Xin\n"},{"id":"208998","messageId":"51152575.8090507@web.de","threadId":"32831","inReplyTo":"CANYiYbGZEJXDjzBJa_qbQCgw9tcTMfOChNr6ANwaLvo=fB=bcQ@mail.gmail.com","subject":"Re: [PATCH v4] Add utf8_fprintf helper which returns correct columns","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-02-08T16:19:01Z","receivedAt":"2013-02-08T16:19:01Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 08.02.13 08:20, Jiang Xin wrote:\n> 2013/2/8 Torsten Bögershausen <tboegi@web.de>:\n>> On 08.02.13 03:10, Jiang Xin wrote:\n>>> +     /* If no error occurs, returns columns really required with utf8_strwidth. */\n>>> +     if (0 <= columns)\n>>> +             columns = utf8_strwidth(buf.buf);\n>>> +     strbuf_release(&buf);\n>>> +     return columns;\n>>> +}\n>>> +\n>>\n>> I don't think we handle the return code from fputs() correctly.\n>>\n>> Please dee below for specifications on fprintf(),\n>> something like the following could do:\n>>\n>> int utf8_fprintf(FILE *stream, const char *format, ...)\n>> {\n>>         struct strbuf buf = STRBUF_INIT;\n>>         va_list arg;\n>>         int columns = 0;\n>>\n>>         va_start (arg, format);\n>>         strbuf_vaddf(&buf, format, arg);\n>>         va_end (arg);\n>>\n>>         if (EOF != fputs(buf.buf, stream))\n>>                 columns = utf8_strwidth(buf.buf);\n>>         strbuf_release(&buf);\n>>         return columns;\n>> }\n> \n> As fputs() returns a non-negative number (as opposed to 0) on\n> successful completion,\n> Test fputs() return value as \"fputs() >=0\" is correct, while \"fputs()\n> == 0\", \"fputs() != 0\"\n> are wrong. I think it's OK, must I send a new re-roll for this?\n> \n> EOF is defined as (-1) in stdio.h:\n> \n>     #define EOF     (-1)\n> \n>> And as a side note: would fprintf_strwidth() be a better name?\n> \n> This is a nice candidate.\n> \n> \n\nDoing a slurp(coffe) followed by a re-read,\nI think we are save with the existng code.\n\nHowever, I feel that the commit message can be improved, \nas it fixes problems not only for CJK but e.g. european languages\n(and all systems using utf-8)\n\n>Since command usages can be translated, they may not align well especially\n>when they are translated to CJK. A wrapper utf8_fprintf can help to return\n>the correct columns required.\n \nSince command usages can be translated, they may include utf-8 encoded strings,\nand strlen() is different from strwidth().\nA wrapper utf8_fprintf() helps to return the correct columns required.\n\n\nThanks for working on this.\n"},{"id":"209071","messageId":"618461e2e8e98c48279759138ff2e850629d0e76.1360391274.git.worldhello.net@gmail.com","threadId":"32831","inReplyTo":"51152575.8090507@web.de","subject":"[PATCH v5] Add utf8_fprintf helper that returns correct columns","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-02-09T06:31:09Z","receivedAt":"2013-02-09T06:31:09Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Since command usages can be translated, they may include utf-8 encoded\nstrings, and the output in console may not align well any more. This is\nbecause strlen() is different from strwidth() on utf-8 strings. A wrapper\nutf8_fprintf() can help to return the correct columns required.\n\nSigned-off-by: Jiang Xin <worldhello.net@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nReviewed-by: Torsten Bögershausen <tboegi@web.de>\n---\n parse-options.c |  5 +++--\n utf8.c          | 21 +++++++++++++++++++++\n utf8.h          |  1 +\n 3 files changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 67e98..a6ce9e 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -3,6 +3,7 @@\n #include \"cache.h\"\n #include \"commit.h\"\n #include \"color.h\"\n+#include \"utf8.h\"\n \n static int parse_options_usage(struct parse_opt_ctx_t *ctx,\n \t\t\t       const char * const *usagestr,\n@@ -482,7 +483,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)\n \t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n \telse\n \t\ts = literal ? \" %s\" : \" <%s>\";\n-\treturn fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n+\treturn utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _(\"...\"));\n }\n \n #define USAGE_OPTS_WIDTH 24\n@@ -541,7 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n \t\tif (opts->long_name)\n \t\t\tpos += fprintf(outfile, \"--%s\", opts->long_name);\n \t\tif (opts->type == OPTION_NUMBER)\n-\t\t\tpos += fprintf(outfile, \"-NUM\");\n+\t\t\tpos += utf8_fprintf(outfile, _(\"-NUM\"));\n \n \t\tif ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||\n \t\t    !(opts->flags & PARSE_OPT_NOARG))\ndiff --git a/utf8.c b/utf8.c\nindex a4ee6..c09dc 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -430,6 +430,27 @@ int same_encoding(const char *src, const char *dst)\n }\n \n /*\n+ * Wrapper for fprintf and returns the total number of columns required\n+ * for the printed string, assuming that the string is utf8.\n+ */\n+int utf8_fprintf(FILE *stream, const char *format, ...)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tva_list arg;\n+\tint columns;\n+\n+\tva_start (arg, format);\n+\tstrbuf_vaddf(&buf, format, arg);\n+\tva_end (arg);\n+\n+\tcolumns = fputs(buf.buf, stream);\n+\tif (0 <= columns) /* keep the error from the I/O */\n+\t\tcolumns = utf8_strwidth(buf.buf);\n+\tstrbuf_release(&buf);\n+\treturn columns;\n+}\n+\n+/*\n  * Given a buffer and its encoding, return it re-encoded\n  * with iconv.  If the conversion fails, returns NULL.\n  */\ndiff --git a/utf8.h b/utf8.h\nindex a2142..501b2 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -8,6 +8,7 @@ int utf8_strwidth(const char *string);\n int is_utf8(const char *text);\n int is_encoding_utf8(const char *name);\n int same_encoding(const char *, const char *);\n+int utf8_fprintf(FILE *, const char *, ...);\n \n void strbuf_add_wrapped_text(struct strbuf *buf,\n \t\tconst char *text, int indent, int indent2, int width);\n-- \n1.8.1.1.370.gf39fc7e\n"}]}