{"thread":{"id":"32881","subject":"[PATCHv2] parse-options: report uncorrupted multi-byte options","startedAt":"2013-02-11T23:13:48Z","lastAt":"2013-02-12T02:30:36Z","messageCount":6,"participants":["Erik Faye-Lund","Junio C Hamano","Jeff King","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"209319","messageId":"1360624428-4728-1-git-send-email-kusmabite@gmail.com","threadId":"32881","inReplyTo":null,"subject":"[PATCHv2] parse-options: report uncorrupted multi-byte options","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2013-02-11T23:13:48Z","receivedAt":"2013-02-11T23:13:48Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Because our command-line parser considers only one byte at the time\nfor short-options, we incorrectly report only the first byte when\nmulti-byte input was provided. This makes user-erros slightly\nawkward to diagnose for instance under UTF-8 locale and non-English\nkeyboard layouts.\n\nMake the reporting code report the whole argument-string when a\nnon-ASCII short-option is detected.\n\nSigned-off-by: Erik Faye-Lund \nImproved-by: Jeff King \n---\n\nHere's a second attempt at fixing error-reporting with UTF-8 encoded\ninput, this time without corrupting other non-ascii multi-byte\nencodings.\n\nI decided to change the text from what Jeff suggested; all we know is\nthat it's non-ASCII. It might be Latin-1 or some other non-ASCII,\nsingle byte encoding. And since we're trying not to care, let's also\ntry to not be overly specific :)\n\nI wasn't entirely sure who to attribute for the improvement, so I just\npicked Jeff; he provided some code. That decision might not be correct,\nfeel free to change it.\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..6a39446 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -461,8 +461,11 @@ int parse_options(int argc, const char **argv, const char *prefix,\n \tdefault: /* PARSE_OPT_UNKNOWN */\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} else if (isascii(*ctx.opt)) {\n \t\t\terror(\"unknown switch `%c'\", *ctx.opt);\n+\t\t} else {\n+\t\t\terror(\"unknown non-ascii option in string: `%s'\",\n+\t\t\t ctx.argv[0]);\n \t\t}\n \t\tusage_with_options(usagestr, options);\n \t}\n-- \n1.8.1.1\n"},{"id":"209320","messageId":"7vliaujto1.fsf@alter.siamese.dyndns.org","threadId":"32881","inReplyTo":"1360624428-4728-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCHv2] parse-options: report uncorrupted multi-byte options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-11T23:51:42Z","receivedAt":"2013-02-11T23:51:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"},{"id":"209323","messageId":"20130212010032.GA4895@sigill.intra.peff.net","threadId":"32881","inReplyTo":"1360624428-4728-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCHv2] parse-options: report uncorrupted multi-byte options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-12T01:00:32Z","receivedAt":"2013-02-12T01:00:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 12, 2013 at 12:13:48AM +0100, Erik Faye-Lund wrote:\n\n> I decided to change the text from what Jeff suggested; all we know is\n> that it's non-ASCII. It might be Latin-1 or some other non-ASCII,\n> single byte encoding. And since we're trying not to care, let's also\n> try to not be overly specific :)\n\nYeah, that makes more sense (I did not put too much thought into the\noriginal wording). Thanks.\n\n-Peff\n"},{"id":"209324","messageId":"CACsJy8BByNnEhhE3TieM_kOy65t75rmB45ZzjJJ8AtL2N4-UFA@mail.gmail.com","threadId":"32881","inReplyTo":"1360624428-4728-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCHv2] parse-options: report uncorrupted multi-byte options","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-12T01:21:24Z","receivedAt":"2013-02-12T01:21:24Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 12, 2013 at 6:13 AM, Erik Faye-Lund wrote:\n> Because our command-line parser considers only one byte at the time\n> for short-options, we incorrectly report only the first byte when\n> multi-byte input was provided. This makes user-erros slightly\n> awkward to diagnose for instance under UTF-8 locale and non-English\n> keyboard layouts.\n>\n> Make the reporting code report the whole argument-string when a\n> non-ASCII short-option is detected.\n\nSimilar cases:\n\nconfig.c:git_default_core_config() assumes core.commentchar is ascii.\nWe should catch and report non-ascii chars, or simply accept it as a\nstring.\n\nbuiltin/update-index.c:cmd_update_index(): error(\"unknown switch\n'%c'\", *ctx.opt);\n\nbuiltin/apply.c:apply_one_fragment(): error(_(\"invalid start of line:\n'%c'\"), first); where 'first' may be a part of utf-8 from a broken\npatch.\n-- \nDuy\n"},{"id":"209325","messageId":"7va9rajn99.fsf@alter.siamese.dyndns.org","threadId":"32881","inReplyTo":"CACsJy8BByNnEhhE3TieM_kOy65t75rmB45ZzjJJ8AtL2N4-UFA@mail.gmail.com","subject":"Re: [PATCHv2] parse-options: report uncorrupted multi-byte options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-12T02:10:10Z","receivedAt":"2013-02-12T02:10:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Feb 12, 2013 at 6:13 AM, Erik Faye-Lund wrote:\n>> Because our command-line parser considers only one byte at the time\n>> for short-options, we incorrectly report only the first byte when\n>> multi-byte input was provided. This makes user-erros slightly\n>> awkward to diagnose for instance under UTF-8 locale and non-English\n>> keyboard layouts.\n>>\n>> Make the reporting code report the whole argument-string when a\n>> non-ASCII short-option is detected.\n>\n> Similar cases:\n>\n> config.c:git_default_core_config() assumes core.commentchar is ascii.\n> We should catch and report non-ascii chars, or simply accept it as a\n> string.\n\nThat one is just an uninterpreted byte. core.commentString might be\na nice extension to the concept, but it is an entirely different\ncategory.\n\n> builtin/update-index.c:cmd_update_index(): error(\"unknown switch\n> '%c'\", *ctx.opt);\n\nThis one is in the same category as this topic.\n\n> builtin/apply.c:apply_one_fragment(): error(_(\"invalid start of line:\n> '%c'\"), first); where 'first' may be a part of utf-8 from a broken\n> patch.\n\nThis is where the patch is expected to have either \" \", \"-\" or \"+\",\nagain, anything else is an uninterpreted byte. It is more like\nreporting the file we found an error in, whose filename is not\nencoded in UTF-8 to the user's terminal.\n"},{"id":"209327","messageId":"CACsJy8DZy0_1RyeNtuobzABUD0rr6oE6RB=rXdqbG72SK4kNUw@mail.gmail.com","threadId":"32881","inReplyTo":"7va9rajn99.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2] parse-options: report uncorrupted multi-byte options","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-12T02:30:36Z","receivedAt":"2013-02-12T02:30:36Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 12, 2013 at 9:10 AM, Junio C Hamano wrote:\n>> Similar cases:\n>>\n>> config.c:git_default_core_config() assumes core.commentchar is ascii.\n>> We should catch and report non-ascii chars, or simply accept it as a\n>> string.\n>\n> That one is just an uninterpreted byte. core.commentString might be\n> a nice extension to the concept, but it is an entirely different\n> category.\n\nMy point is not to output broken utf-8 if we can. If someone\naccidentally puts a UTF-8 character in core.commentChar, it will\nproduce broken utf-8 templates that editors might react, but hard to\nsee by eye. Something like this may give sufficient protection:\n\ndiff --git a/config.c b/config.c\nindex aefd80b..b6f73e0 100644\n--- a/config.c\n+++ b/config.c\n@@ -726,8 +726,11 @@ static int git_default_core_config(const char\n*var, const char *value)\n if (!strcmp(var, \"core.commentchar\")) {\n const char *comment;\n int ret = git_config_string(&comment, var, value);\n- if (!ret)\n+ if (!ret) {\n+ if (comment[1])\n+ return error(\"core.commentchar must be\none ASCII character\");\n comment_line_char = comment[0];\n+ }\n return ret;\n }\n-- \nDuy\n"}]}