{"thread":{"id":"32876","subject":"[PATH/RFC] parse-options: report invalid UTF-8 switches","startedAt":"2013-02-11T13:34:47Z","lastAt":"2013-02-11T17:54:08Z","messageCount":13,"participants":["Erik Faye-Lund","Matthieu Moy","Torsten Bögershausen","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"209246","messageId":"1360589687-9233-1-git-send-email-kusmabite@gmail.com","threadId":"32876","inReplyTo":null,"subject":"[PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-02-11T13:34:47Z","receivedAt":"2013-02-11T13:34:47Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Even though parse-options doesn't support UTF-8 switches (which\nmakes sense; non-ascii switches would be difficult to enter on\nsome keyboard layouts), it can be useful to report incorrectly\nentered UTF-8 switches to make the output somewhat less ugly\nfor those of us with keyboard layouts with UTF-8 characters on\nit.\n\nMake the reporting code grok UTF-8 in the option sequence, and\nwrite a variable-width output sequence.\n\nSigned-off-by: Erik Faye-Lund \n---\nAs being both clumsy and Norwegian, I some times to enter the\nNorwegian bizarro-letters ('æ', 'ø' and 'å') instead of the\ncorrect ones when entering command-line options.\n\nHowever, since git only looks at one byte at the time for\nshort-options, it ends up reporting a partial UTF-8 sequence\nin such cases, leading to corruption of the output.\n\nThe \"real fix\" would probably be to add proper multi-byte\nsupport to the short-option parser, but this serves little\npurpose in Git; we don't internationalize the command-line\nswitches.\n\nSo perhaps this is a suitable band-aid instead?\n\n parse-options.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 67e98a6..20dc742 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@@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,\n \t\tif (ctx.argv[0][1] == '-') {\n \t\t\terror(\"unknown option `%s'\", ctx.argv[0] + 2);\n \t\t} else {\n-\t\t\terror(\"unknown switch `%c'\", *ctx.opt);\n+\t\t\tconst char *next = ctx.opt;\n+\t\t\tutf8_width(&next, NULL);\n+\t\t\terror(\"unknown switch `%.*s'\", (int)(next - ctx.opt), ctx.opt);\n \t\t}\n \t\tusage_with_options(usagestr, options);\n \t}\n-- \n1.7.11.7\n"},{"id":"209247","messageId":"vpqobfr9da7.fsf@grenoble-inp.fr","threadId":"32876","inReplyTo":"1360589687-9233-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-11T13:43:28Z","receivedAt":"2013-02-11T13:43:28Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Erik Faye-Lund writes:\n\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> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,\n> \t\tif (ctx.argv[0][1] == '-') {\n> \t\t\terror(\"unknown option `%s'\", ctx.argv[0] + 2);\n> \t\t} else {\n> -\t\t\terror(\"unknown switch `%c'\", *ctx.opt);\n> +\t\t\tconst char *next = ctx.opt;\n> +\t\t\tutf8_width(&next, NULL);\n> +\t\t\terror(\"unknown switch `%.*s'\", (int)(next - ctx.opt), ctx.opt);\n> \t\t}\n> \t\tusage_with_options(usagestr, options);\n> \t}\n\nYou should be careful with the case where the user has a non-UTF8\nenvironment, and entered a non-ascii sequence. I can see two cases:\n\n1) The non-ascii sequence is valid UTF-8, then I guess your patch would\n show two characters instead of one. Not really correct, but not really\n serious either.\n\n2) The non-ascii sequence is NOT valid UTF-8, then if I read correctly\n (I didn't test) utf8_width would set next to NULL, and then you are\n in big trouble.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"209248","messageId":"CABPQNSbYCrdSP5rWbfLX==u--bJpQo6A6sNE46a1RuU-fMDiWg@mail.gmail.com","threadId":"32876","inReplyTo":"vpqobfr9da7.fsf@grenoble-inp.fr","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-02-11T13:57:17Z","receivedAt":"2013-02-11T13:57:17Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 11, 2013 at 2:43 PM, Matthieu Moy\n wrote:\n> Erik Faye-Lund writes:\n>\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>> const char * const *usagestr,\n>> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,\n>> if (ctx.argv[0][1] == '-') {\n>> error(\"unknown option `%s'\", ctx.argv[0] + 2);\n>> } else {\n>> - error(\"unknown switch `%c'\", *ctx.opt);\n>> + const char *next = ctx.opt;\n>> + utf8_width(&next, NULL);\n>> + error(\"unknown switch `%.*s'\", (int)(next - ctx.opt), ctx.opt);\n>> }\n>> usage_with_options(usagestr, options);\n>> }\n>\n> You should be careful with the case where the user has a non-UTF8\n> environment, and entered a non-ascii sequence. I can see two cases:\n>\n> 1) The non-ascii sequence is valid UTF-8, then I guess your patch would\n> show two characters instead of one. Not really correct, but not really\n> serious either.\n\nHm. So we would end up trading some form of corruption for some other.\nNot the biggest problem in the world, but perhaps there's a way of\nfixing it?\n\nI'm not entirely sure how to correctly know what encoding stdin is\nsupposed to be. On Windows, that's easy; it's UTF-16, we re-encode it\nto UTF-8 on startup in Git for Windows. But on other platforms, I have\nno clue.\n\nBut isn't UTF-8 constructed to be very unlikely to clash with existing\nencodings? If so, I could add a case for non-ascii and non-UTF-8, that\nsimply writes the byte as a hex-tuple?\n\n> 2) The non-ascii sequence is NOT valid UTF-8, then if I read correctly\n> (I didn't test) utf8_width would set next to NULL, and then you are\n> in big trouble.\n\nOutch. Yeah, you are right; this is not good at all :)\n\nBut I guess the solution above should fix this as well, no?\n"},{"id":"209249","messageId":"vpqy5ev7xpp.fsf@grenoble-inp.fr","threadId":"32876","inReplyTo":"CABPQNSbYCrdSP5rWbfLX==u--bJpQo6A6sNE46a1RuU-fMDiWg@mail.gmail.com","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-11T14:05:06Z","receivedAt":"2013-02-11T14:05:06Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Erik Faye-Lund writes:\n\n> But isn't UTF-8 constructed to be very unlikely to clash with existing\n> encodings? If so, I could add a case for non-ascii and non-UTF-8, that\n> simply writes the byte as a hex-tuple?\n\nIf it's non-ascii and non-UTF-8, I think you'd want to display the byte\nas it is, because this is how it was entered. IOW, I'd say we should\nkeep the current behavior in this case.\n\n>> 2) The non-ascii sequence is NOT valid UTF-8, then if I read correctly\n>> (I didn't test) utf8_width would set next to NULL, and then you are\n>> in big trouble.\n>\n> Outch. Yeah, you are right; this is not good at all :)\n>\n> But I guess the solution above should fix this as well, no?\n\nIt should, yes.\n\nOf course, there's still the case where the user entered \"git -é\" as a\nà followed by a © in a latin-1 environment, but as you said, it's\nunlikely enough ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"209251","messageId":"CABPQNSaKt6XGwRBMNw1eHNCY-d0TY=63LwcnQsFKdnhA+JTL-w@mail.gmail.com","threadId":"32876","inReplyTo":"vpqy5ev7xpp.fsf@grenoble-inp.fr","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-02-11T14:27:19Z","receivedAt":"2013-02-11T14:27:19Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 11, 2013 at 3:05 PM, Matthieu Moy\n wrote:\n> Erik Faye-Lund writes:\n>\n>> But isn't UTF-8 constructed to be very unlikely to clash with existing\n>> encodings? If so, I could add a case for non-ascii and non-UTF-8, that\n>> simply writes the byte as a hex-tuple?\n>\n> If it's non-ascii and non-UTF-8, I think you'd want to display the byte\n> as it is, because this is how it was entered. IOW, I'd say we should\n> keep the current behavior in this case.\n>\n\nYes, you are of course right. We should detect UTF-8, and only in\nthose cases do anything special. Because the likely alternatives are\nother 8-byte encodings (which the terminal already should grok, since\nthe user was able to input it), or other multi-byte sequences (which\nalready is broken, and is tricky to handle). So at least we'd only\nbreak in very unlikely cases.\n\nBut, I wonder, could mbrlen be used to detect the length instead? It\nconsults LC_CTYPE to find out what encoding to use, which seems like\nit might give the correct answer in all non-corrupted cases... I'm far\nfrom an expert on UNIX-internationalization, though. And this approach\nis likely to break on Windows, but I suspect that we can perform some\nwell-placed hack for it, as we already know that we're doing UTF-8\nthere.\n"},{"id":"209262","messageId":"51191C33.8070402@web.de","threadId":"32876","inReplyTo":"1360589687-9233-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-02-11T16:28:35Z","receivedAt":"2013-02-11T16:28:35Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11.02.13 14:34, Erik Faye-Lund wrote:\n> Even though parse-options doesn't support UTF-8 switches (which\n> makes sense; non-ascii switches would be difficult to enter on\n> some keyboard layouts), it can be useful to report incorrectly\n> entered UTF-8 switches to make the output somewhat less ugly\n> for those of us with keyboard layouts with UTF-8 characters on\n> it.\n> \n> Make the reporting code grok UTF-8 in the option sequence, and\n> write a variable-width output sequence.\n> \n> Signed-off-by: Erik Faye-Lund \n> ---\n> As being both clumsy and Norwegian, I some times to enter the\n> Norwegian bizarro-letters ('æ', 'ø' and 'å') instead of the\n> correct ones when entering command-line options.\n> \n> However, since git only looks at one byte at the time for\n> short-options, it ends up reporting a partial UTF-8 sequence\n> in such cases, leading to corruption of the output.\n> \n> The \"real fix\" would probably be to add proper multi-byte\n> support to the short-option parser, but this serves little\n> purpose in Git; we don't internationalize the command-line\n> switches.\n> \n> So perhaps this is a suitable band-aid instead?\n> \n> parse-options.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n> \n> diff --git a/parse-options.c b/parse-options.c\n> index 67e98a6..20dc742 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> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,\n> \t\tif (ctx.argv[0][1] == '-') {\n> \t\t\terror(\"unknown option `%s'\", ctx.argv[0] + 2);\n> \t\t} else {\n> -\t\t\terror(\"unknown switch `%c'\", *ctx.opt);\n> +\t\t\tconst char *next = ctx.opt;\n> +\t\t\tutf8_width(&next, NULL);\n> +\t\t\terror(\"unknown switch `%.*s'\", (int)(next - ctx.opt), ctx.opt);\n> \t\t}\n> \t\tusage_with_options(usagestr, options);\n> \t}\n> \nWould the following do the trick?\n\ndiff --git a/parse-options.c b/parse-options.c\nindex c1c66bd..f800552 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -471,7 +471,7 @@ int parse_options(int argc, const char **argv, const char *prefix,\n if (ctx.argv[0][1] == '-') {\n error(\"unknown option `%s'\", ctx.argv[0] + 2);\n } else {\n- error(\"unknown switch `%c'\", *ctx.opt);\n+ error(\"unknown switch `%s'\", ctx.opt);\n }\n"},{"id":"209264","messageId":"CABPQNSYXk5_VdGP9QQttZKpcBnmohmNb+AOH=bMkOrXjbq1ZmQ@mail.gmail.com","threadId":"32876","inReplyTo":"51191C33.8070402@web.de","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-02-11T16:36:52Z","receivedAt":"2013-02-11T16:36:52Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 11, 2013 at 5:28 PM, Torsten Bögershausen wrote:\n> On 11.02.13 14:34, Erik Faye-Lund wrote:\n>> Even though parse-options doesn't support UTF-8 switches (which\n>> makes sense; non-ascii switches would be difficult to enter on\n>> some keyboard layouts), it can be useful to report incorrectly\n>> entered UTF-8 switches to make the output somewhat less ugly\n>> for those of us with keyboard layouts with UTF-8 characters on\n>> it.\n>>\n>> Make the reporting code grok UTF-8 in the option sequence, and\n>> write a variable-width output sequence.\n>>\n>> Signed-off-by: Erik Faye-Lund \n>> ---\n>> As being both clumsy and Norwegian, I some times to enter the\n>> Norwegian bizarro-letters ('æ', 'ø' and 'å') instead of the\n>> correct ones when entering command-line options.\n>>\n>> However, since git only looks at one byte at the time for\n>> short-options, it ends up reporting a partial UTF-8 sequence\n>> in such cases, leading to corruption of the output.\n>>\n>> The \"real fix\" would probably be to add proper multi-byte\n>> support to the short-option parser, but this serves little\n>> purpose in Git; we don't internationalize the command-line\n>> switches.\n>>\n>> So perhaps this is a suitable band-aid instead?\n>>\n>> parse-options.c | 5 ++++-\n>> 1 file changed, 4 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/parse-options.c b/parse-options.c\n>> index 67e98a6..20dc742 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>> const char * const *usagestr,\n>> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,\n>> if (ctx.argv[0][1] == '-') {\n>> error(\"unknown option `%s'\", ctx.argv[0] + 2);\n>> } else {\n>> - error(\"unknown switch `%c'\", *ctx.opt);\n>> + const char *next = ctx.opt;\n>> + utf8_width(&next, NULL);\n>> + error(\"unknown switch `%.*s'\", (int)(next - ctx.opt), ctx.opt);\n>> }\n>> usage_with_options(usagestr, options);\n>> }\n>>\n> Would the following do the trick?\n>\n> diff --git a/parse-options.c b/parse-options.c\n> index c1c66bd..f800552 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -471,7 +471,7 @@ int parse_options(int argc, const char **argv, const char *prefix,\n> if (ctx.argv[0][1] == '-') {\n> error(\"unknown option `%s'\", ctx.argv[0] + 2);\n> } else {\n> - error(\"unknown switch `%c'\", *ctx.opt);\n> + error(\"unknown switch `%s'\", ctx.opt);\n> }\n>\n>\n\nNope; that would print the rest of the option-string, in cases of \"git\n -abcd\".\n"},{"id":"209269","messageId":"511924A0.8030704@web.de","threadId":"32876","inReplyTo":"CABPQNSYXk5_VdGP9QQttZKpcBnmohmNb+AOH=bMkOrXjbq1ZmQ@mail.gmail.com","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-02-11T17:04:32Z","receivedAt":"2013-02-11T17:04:32Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11.02.13 17:36, Erik Faye-Lund wrote:\n> On Mon, Feb 11, 2013 at 5:28 PM, Torsten Bögershausen wrote:\n>> On 11.02.13 14:34, Erik Faye-Lund wrote:\n>>> Even though parse-options doesn't support UTF-8 switches (which\n>>> makes sense; non-ascii switches would be difficult to enter on\n>>> some keyboard layouts), it can be useful to report incorrectly\n>>> entered UTF-8 switches to make the output somewhat less ugly\n>>> for those of us with keyboard layouts with UTF-8 characters on\n>>> it.\n>>>\n>>> Make the reporting code grok UTF-8 in the option sequence, and\n>>> write a variable-width output sequence.\n>>>\n>>> Signed-off-by: Erik Faye-Lund \n>>> ---\n>>> As being both clumsy and Norwegian, I some times to enter the\n>>> Norwegian bizarro-letters ('æ', 'ø' and 'å') instead of the\n>>> correct ones when entering command-line options.\n>>>\n>>> However, since git only looks at one byte at the time for\n>>> short-options, it ends up reporting a partial UTF-8 sequence\n>>> in such cases, leading to corruption of the output.\n>>>\n>>> The \"real fix\" would probably be to add proper multi-byte\n>>> support to the short-option parser, but this serves little\n>>> purpose in Git; we don't internationalize the command-line\n>>> switches.\n>>>\n>>> So perhaps this is a suitable band-aid instead?\n>>>\n>>> parse-options.c | 5 ++++-\n>>> 1 file changed, 4 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/parse-options.c b/parse-options.c\n>>> index 67e98a6..20dc742 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>>> const char * const *usagestr,\n>>> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,\n>>> if (ctx.argv[0][1] == '-') {\n>>> error(\"unknown option `%s'\", ctx.argv[0] + 2);\n>>> } else {\n>>> - error(\"unknown switch `%c'\", *ctx.opt);\n>>> + const char *next = ctx.opt;\n>>> + utf8_width(&next, NULL);\n>>> + error(\"unknown switch `%.*s'\", (int)(next - ctx.opt), ctx.opt);\n>>> }\n>>> usage_with_options(usagestr, options);\n>>> }\n>>>\n>> Would the following do the trick?\n>>\n>> diff --git a/parse-options.c b/parse-options.c\n>> index c1c66bd..f800552 100644\n>> --- a/parse-options.c\n>> +++ b/parse-options.c\n>> @@ -471,7 +471,7 @@ int parse_options(int argc, const char **argv, const char *prefix,\n>> if (ctx.argv[0][1] == '-') {\n>> error(\"unknown option `%s'\", ctx.argv[0] + 2);\n>> } else {\n>> - error(\"unknown switch `%c'\", *ctx.opt);\n>> + error(\"unknown switch `%s'\", ctx.opt);\n>> }\n>>\n>>\n> Nope; that would print the rest of the option-string, in cases of \"git\n> -abcd\".\nOk, may be pick_one_utf8_char() is a better choice than simply assuming ASCII.\n\nWe can make a guess, if it is utf-8, we use it. If not, assume ASCII.\nJust thinking loud (the \"if\" could be written shorter using the \"?\" operator)\n\n } else {\n const char *start = ctx.opt;\n unsigned c = pick_one_utf8_char(&start, NULL);\n if (!c)\n c = *ctx.opt;\n error(\"unknown switch `%c'\", c);\n }\n\n \n"},{"id":"209270","messageId":"7v7gmeok2e.fsf@alter.siamese.dyndns.org","threadId":"32876","inReplyTo":"1360589687-9233-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-11T17:07:53Z","receivedAt":"2013-02-11T17:07:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> However, since git only looks at one byte at the time for\n> short-options, it ends up reporting a partial UTF-8 sequence\n> in such cases, leading to corruption of the output.\n\nIsn't it a workable, easier and more robust alternative to punt and\nuse the entire ctx.argv[0] as unrecognized?\n\n>\n> The \"real fix\" would probably be to add proper multi-byte\n> support to the short-option parser, but this serves little\n> purpose in Git; we don't internationalize the command-line\n> switches.\n>\n> So perhaps this is a suitable band-aid instead?\n>\n> parse-options.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/parse-options.c b/parse-options.c\n> index 67e98a6..20dc742 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> @@ -462,7 +463,9 @@ int parse_options(int argc, const char **argv, const char *prefix,\n> \t\tif (ctx.argv[0][1] == '-') {\n> \t\t\terror(\"unknown option `%s'\", ctx.argv[0] + 2);\n> \t\t} else {\n> -\t\t\terror(\"unknown switch `%c'\", *ctx.opt);\n> +\t\t\tconst char *next = ctx.opt;\n> +\t\t\tutf8_width(&next, NULL);\n> +\t\t\terror(\"unknown switch `%.*s'\", (int)(next - ctx.opt), ctx.opt);\n> \t\t}\n> \t\tusage_with_options(usagestr, options);\n> \t}\n"},{"id":"209273","messageId":"CABPQNSYoS6ik-29HSy14P69n7zJQORDT9Pa2O1BO=hL3wDuTeA@mail.gmail.com","threadId":"32876","inReplyTo":"7v7gmeok2e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-02-11T17:15:19Z","receivedAt":"2013-02-11T17:15:19Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 11, 2013 at 6:07 PM, Junio C Hamano wrote:\n> Erik Faye-Lund writes:\n>\n>> However, since git only looks at one byte at the time for\n>> short-options, it ends up reporting a partial UTF-8 sequence\n>> in such cases, leading to corruption of the output.\n>\n> Isn't it a workable, easier and more robust alternative to punt and\n> use the entire ctx.argv[0] as unrecognized?\n>\n\nPerhaps. It doesn't match the output of the usual GNU tools like we\ncurrently do, but even the GNU tools only report a single byte.\n\nHowever, I'm unsure if that totals to an improvement in the common\ncase. We stop telling the user exactly what option was problematic,\nmaking it slightly more annoying to read through the options.\n\nSo, we'd end up making the common-case worse, by making a special case\n(that only sometimes affects some users) more robust. Isn't that\nmaking the user interface worse?\n"},{"id":"209277","messageId":"20130211171957.GG16402@sigill.intra.peff.net","threadId":"32876","inReplyTo":"7v7gmeok2e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-11T17:19:57Z","receivedAt":"2013-02-11T17:19:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 11, 2013 at 09:07:53AM -0800, Junio C Hamano wrote:\n\n> Erik Faye-Lund writes:\n> \n> > However, since git only looks at one byte at the time for\n> > short-options, it ends up reporting a partial UTF-8 sequence\n> > in such cases, leading to corruption of the output.\n> \n> Isn't it a workable, easier and more robust alternative to punt and\n> use the entire ctx.argv[0] as unrecognized?\n\nYes, but it regresses the usability:\n\n [before]\n $ git foobar -qrxs\n unknown switch: x\n\n [after]\n $ git foobar -qrxs\n unknown switch: -qrxs\n\nOne is much more informative than the other, and you are punishing the\ncommon ascii case for the extremely uncommon case of utf-8. Maybe:\n\n if (isascii(*ctx.opt))\n error(\"unknown option `%c'\", *ctx.opt);\n else\n error(\"unknown multi-byte short option in string: `%s'\", ctx.argv[0]);\n\nwhich only kicks in in the uncommon case (and extends the error message\nto make it more clear why we are showing the whole string).\n\n-Peff\n"},{"id":"209279","messageId":"CABPQNSYtvXED_uj6m02N1-RWEdz3iMUUXUaNZquzy8GqQ2BnLw@mail.gmail.com","threadId":"32876","inReplyTo":"20130211171957.GG16402@sigill.intra.peff.net","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-02-11T17:21:58Z","receivedAt":"2013-02-11T17:21:58Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 11, 2013 at 6:19 PM, Jeff King wrote:\n> On Mon, Feb 11, 2013 at 09:07:53AM -0800, Junio C Hamano wrote:\n>\n>> Erik Faye-Lund writes:\n>>\n>> > However, since git only looks at one byte at the time for\n>> > short-options, it ends up reporting a partial UTF-8 sequence\n>> > in such cases, leading to corruption of the output.\n>>\n>> Isn't it a workable, easier and more robust alternative to punt and\n>> use the entire ctx.argv[0] as unrecognized?\n>\n> Yes, but it regresses the usability:\n>\n> [before]\n> $ git foobar -qrxs\n> unknown switch: x\n>\n> [after]\n> $ git foobar -qrxs\n> unknown switch: -qrxs\n>\n> One is much more informative than the other, and you are punishing the\n> common ascii case for the extremely uncommon case of utf-8. Maybe:\n>\n> if (isascii(*ctx.opt))\n> error(\"unknown option `%c'\", *ctx.opt);\n> else\n> error(\"unknown multi-byte short option in string: `%s'\", ctx.argv[0]);\n>\n> which only kicks in in the uncommon case (and extends the error message\n> to make it more clear why we are showing the whole string).\n\nYes. This is IMO a much better approach, and it doesn't involve trying\nto figure out what encoding the string is. Thanks!\n"},{"id":"209284","messageId":"7vy5eun3cv.fsf@alter.siamese.dyndns.org","threadId":"32876","inReplyTo":"20130211171957.GG16402@sigill.intra.peff.net","subject":"Re: [PATH/RFC] parse-options: report invalid UTF-8 switches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-11T17:54:08Z","receivedAt":"2013-02-11T17:54:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Feb 11, 2013 at 09:07:53AM -0800, Junio C Hamano wrote:\n>\n>> Erik Faye-Lund writes:\n>> \n>> > However, since git only looks at one byte at the time for\n>> > short-options, it ends up reporting a partial UTF-8 sequence\n>> > in such cases, leading to corruption of the output.\n>> \n>> Isn't it a workable, easier and more robust alternative to punt and\n>> use the entire ctx.argv[0] as unrecognized?\n>\n> Yes, but it regresses the usability:\n>\n> [before]\n> $ git foobar -qrxs\n> unknown switch: x\n>\n> [after]\n> $ git foobar -qrxs\n> unknown switch: -qrxs\n>\n> One is much more informative than the other, and you are punishing the\n> common ascii case for the extremely uncommon case of utf-8. Maybe:\n>\n> if (isascii(*ctx.opt))\n> error(\"unknown option `%c'\", *ctx.opt);\n> else\n> error(\"unknown multi-byte short option in string: `%s'\", ctx.argv[0]);\n>\n> which only kicks in in the uncommon case (and extends the error message\n> to make it more clear why we are showing the whole string).\n\nYup, that is what I meant.\n"}]}