[PATCHv2] parse-options: report uncorrupted multi-byte options

Subsystems: the rest

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCHv2] parse-options: report uncorrupted multi-byte options

From: Erik Faye-Lund <hidden>
Date: 2016-06-15 22:56:08

Because our command-line parser considers only one byte at the time
for short-options, we incorrectly report only the first byte when
multi-byte input was provided. This makes user-erros slightly
awkward to diagnose for instance under UTF-8 locale and non-English
keyboard layouts.

Make the reporting code report the whole argument-string when a
non-ASCII short-option is detected.

Signed-off-by: Erik Faye-Lund <redacted>
Improved-by: Jeff King [off-list ref]
---

Here's a second attempt at fixing error-reporting with UTF-8 encoded
input, this time without corrupting other non-ascii multi-byte
encodings.

I decided to change the text from what Jeff suggested; all we know is
that it's non-ASCII. It might be Latin-1 or some other non-ASCII,
single byte encoding. And since we're trying not to care, let's also
try to not be overly specific :)

I wasn't entirely sure who to attribute for the improvement, so I just
picked Jeff; he provided some code. That decision might not be correct,
feel free to change it.

 parse-options.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/parse-options.c b/parse-options.c
index 67e98a6..6a39446 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -461,8 +461,11 @@ int parse_options(int argc, const char **argv, const char *prefix,
 	default: /* PARSE_OPT_UNKNOWN */
 		if (ctx.argv[0][1] == '-') {
 			error("unknown option `%s'", ctx.argv[0] + 2);
-		} else {
+		} else if (isascii(*ctx.opt)) {
 			error("unknown switch `%c'", *ctx.opt);
+		} else {
+			error("unknown non-ascii option in string: `%s'",
+			    ctx.argv[0]);
 		}
 		usage_with_options(usagestr, options);
 	}
-- 
1.8.1.1

Re: [PATCHv2] parse-options: report uncorrupted multi-byte options

From: Jeff King <hidden>
Date: 2016-06-15 22:56:08

On Tue, Feb 12, 2013 at 12:13:48AM +0100, Erik Faye-Lund wrote:
I decided to change the text from what Jeff suggested; all we know is
that it's non-ASCII. It might be Latin-1 or some other non-ASCII,
single byte encoding. And since we're trying not to care, let's also
try to not be overly specific :)
Yeah, that makes more sense (I did not put too much thought into the
original wording). Thanks.

-Peff

Re: [PATCHv2] parse-options: report uncorrupted multi-byte options

From: Duy Nguyen <hidden>
Date: 2016-06-15 22:56:08

On Tue, Feb 12, 2013 at 6:13 AM, Erik Faye-Lund [off-list ref] wrote:
Because our command-line parser considers only one byte at the time
for short-options, we incorrectly report only the first byte when
multi-byte input was provided. This makes user-erros slightly
awkward to diagnose for instance under UTF-8 locale and non-English
keyboard layouts.

Make the reporting code report the whole argument-string when a
non-ASCII short-option is detected.
Similar cases:

config.c:git_default_core_config() assumes core.commentchar is ascii.
We should catch and report non-ascii chars, or simply accept it as a
string.

builtin/update-index.c:cmd_update_index(): error("unknown switch
'%c'", *ctx.opt);

builtin/apply.c:apply_one_fragment(): error(_("invalid start of line:
'%c'"), first); where 'first' may be a part of utf-8 from a broken
patch.
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help