Re: bug report: symbolic-ref --short command echos the wrong text while use Chinese language

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Feb 14, 2023 at 11:01 AM Jeff King <peff@peff.net> wrote:
> On Tue, Feb 14, 2023 at 01:55:32AM -0500, Eric Sunshine wrote:
> > On Tue, Feb 14, 2023 at 1:45 AM Junio C Hamano <gitster@pobox.com> wrote:
> > > Eric Sunshine <sunshine@sunshineco.com> writes:
> > > > Using (presumably) valid LANG codes results in the buggy truncated
> > > > output, but "LANG=C" produces the correct result:
> > > >
> > > >   $ for i in C en_US fr_FR de_DE ru_RU zh_CN; do printf "$i: " &&
> > > > LANG=$i.UTF-8 git symbolic-ref --short HEAD; done
> > > >   C: 测试-加-增加-加-增加
> > > >   en_US: 测试-?

Imadummy. The example loop incorrectly uses LANG=C.UTF-8 rather than
LANG=C for the first item. However, LANG=C does indeed result in the
correct output:

  $ LANG=C git symbolic-ref --short HEAD
  测试-加-增加-加-增加

> >   $ LANG=bogus git symbolic-ref --short HEAD
> >   测试-加-增加-加-增加
>
> Oof. So it is some weird locale thing that scanf is doing. I don't even
> want to think about what the details could be. ;)
>
> Since scanf is such a bad and error-prone interface in the first place
> (and I'd actually like to put it on the banned list), what about just
> parsing manually here? We are already implicitly assuming that each
> rev-parse rule has a single "%.*s" in it. Armed with that knowledge,
> it's not too hard to match using skip_prefix() and strip_suffix(). Or
> with a little bit more custom code, we can avoid the step to pre-process
> the rule strings completely. Something like:
>
> +/*
> + * Check that the string refname matches a rule of the form
> + * "{prefix}%.*s{suffix}". So "foo/bar/baz" would match the rule
> + * "foo/%.*s/baz", and return the string "bar".
> + */
> +static char *match_parse_rule(const char *refname, const char *rule)

Yes, this works nicely and fixes the reported problem:

  % $GIT_DIR/bin-wrappers/git symbolic-ref --short HEAD
  测试-加-增加-加-增加

I'm all for this approach. Fewer scanf()'s, the better.

The new code itself looks correct; I think it properly covers all the
edge-cases (at least those that came to my mind).