From: Junio C Hamano <hidden> Date: 2016-06-15 22:56:07
Erik Faye-Lund [off-list ref] writes:
However, since git only looks at one byte at the time for
short-options, it ends up reporting a partial UTF-8 sequence
in such cases, leading to corruption of the output.
Isn't it a workable, easier and more robust alternative to punt and
use the entire ctx.argv[0] as unrecognized?
quoted hunk
The "real fix" would probably be to add proper multi-byte
support to the short-option parser, but this serves little
purpose in Git; we don't internationalize the command-line
switches.
So perhaps this is a suitable band-aid instead?
parse-options.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:56:07
On Mon, Feb 11, 2013 at 6:07 PM, Junio C Hamano [off-list ref] wrote:
Erik Faye-Lund [off-list ref] writes:
quoted
However, since git only looks at one byte at the time for
short-options, it ends up reporting a partial UTF-8 sequence
in such cases, leading to corruption of the output.
Isn't it a workable, easier and more robust alternative to punt and
use the entire ctx.argv[0] as unrecognized?
Perhaps. It doesn't match the output of the usual GNU tools like we
currently do, but even the GNU tools only report a single byte.
However, I'm unsure if that totals to an improvement in the common
case. We stop telling the user exactly what option was problematic,
making it slightly more annoying to read through the options.
So, we'd end up making the common-case worse, by making a special case
(that only sometimes affects some users) more robust. Isn't that
making the user interface worse?
From: Jeff King <hidden> Date: 2016-06-15 22:56:07
On Mon, Feb 11, 2013 at 09:07:53AM -0800, Junio C Hamano wrote:
Erik Faye-Lund [off-list ref] writes:
quoted
However, since git only looks at one byte at the time for
short-options, it ends up reporting a partial UTF-8 sequence
in such cases, leading to corruption of the output.
Isn't it a workable, easier and more robust alternative to punt and
use the entire ctx.argv[0] as unrecognized?
Yes, but it regresses the usability:
[before]
$ git foobar -qrxs
unknown switch: x
[after]
$ git foobar -qrxs
unknown switch: -qrxs
One is much more informative than the other, and you are punishing the
common ascii case for the extremely uncommon case of utf-8. Maybe:
if (isascii(*ctx.opt))
error("unknown option `%c'", *ctx.opt);
else
error("unknown multi-byte short option in string: `%s'", ctx.argv[0]);
which only kicks in in the uncommon case (and extends the error message
to make it more clear why we are showing the whole string).
-Peff
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:56:07
On Mon, Feb 11, 2013 at 6:19 PM, Jeff King [off-list ref] wrote:
On Mon, Feb 11, 2013 at 09:07:53AM -0800, Junio C Hamano wrote:
quoted
Erik Faye-Lund [off-list ref] writes:
quoted
However, since git only looks at one byte at the time for
short-options, it ends up reporting a partial UTF-8 sequence
in such cases, leading to corruption of the output.
Isn't it a workable, easier and more robust alternative to punt and
use the entire ctx.argv[0] as unrecognized?
Yes, but it regresses the usability:
[before]
$ git foobar -qrxs
unknown switch: x
[after]
$ git foobar -qrxs
unknown switch: -qrxs
One is much more informative than the other, and you are punishing the
common ascii case for the extremely uncommon case of utf-8. Maybe:
if (isascii(*ctx.opt))
error("unknown option `%c'", *ctx.opt);
else
error("unknown multi-byte short option in string: `%s'", ctx.argv[0]);
which only kicks in in the uncommon case (and extends the error message
to make it more clear why we are showing the whole string).
Yes. This is IMO a much better approach, and it doesn't involve trying
to figure out what encoding the string is. Thanks!