Subject: Re: [PATCH v2 3/3] shorten_unambiguous_ref(): avoid sscanf()

From: Junio C Hamano <[email-masked]>
Date: Thu, 16 Feb 2023 15:36:15 -0800

Jeff King <[email-masked]> writes:

> I do kind of wonder if this test is even doing much. It is nice to
> verify the fix (and hopefully somebody with macOS did indeed verify that
> it fails before the fix!). But it does not seem all that likely that we
> are going to regress in this area. I think it's reasonable to include
> it, but if the LC_ALL bit starts creating any portability problems, my
> first instinct would be to drop the test.

That was my thought as well.  I do not see this particular test to
be about protecting against regression.  With the greediness of its
patterns, it is not likely that our future update will reintroduce
use of sscanf(3) and suffer from the same platform bug.  It is about
documenting the historical bug, i.e. the value of the test part of
the patch primarily is that macOS folks can apply it (and revert the
code fix) and demonstrate the presence of a problem.

The refs/remotes/%s/HEAD test does have value---it is to keep us
away from the temptation to go back to the sscanf(3) based solution.
So do the other two to lessor degree, as they cover corner cases
that a similar/rejected implementation would have failed to handle.

Thanks.