Re: [PATCH v3] ci: disallow directional formatting

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 9, 2021, 13:34 UTC
Message-ID: <211109.86bl2tjvns.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqo86utnpi.fsf@gitster.g>

On Mon, Nov 08 2021, Junio C Hamano wrote:

Show 24 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> So you are comparing
>>>
>>>  * requiring bash and C.UTF-8 locale to be available
>>>
>>> vs
>>>
>>>  * requiring git built with PCRE
>>>
>>> assuming that "Dscho says doesn't work with PCRE and you say it
>>> works with PCRE" is resolved?  They seem roughly the same
>>> difficulty to me.
>>
>> We can hard depend on a git built with PCRE, since the point of this
>> thing is to run in GitHub CI, Ubuntu builds git with PCRE, and that's
>> unlikely to ever change.
>
> Yes, so is the availability of bash and C.UTF-8 for the same reason:
> we are talking about controlled environment.  That is what I meant
> by "roughly the same difficulty to me".
>
> FWIW, I am OK with either approach, as I find the patch in question
> is just as readable as any rewrite that would use "grep -P", so...

To each his own I guess :) I do find the simple regex of:

    '[\N{U+202a}-\N{U+202e}\N{U+2066}-\N{U+2069}]'

Much easier to understand than something using printf, shell interpolation, and needing to switch around LC_CTYPE to two different values twice on one line.

But anyway, that's a matter of taste.

What isn't is the issue I noted at the bottom of [1], i.e. if we're relying on '(attr:binary)' we should probably start with an assertion that our idea of "binary" matches reality, or perhaps go back to the -I heuristic.

Because now we've got at least one binary non-attribute-marked file, and if files like that ever get updated they might start matching this pattern. Maybe not a big deal, but someone updating those might confusingly trip over this otherwise...

1. https://lore.kernel.org/git/211103.86zgqlhzvz.gmgdl@evledraar.gmail.com/