Re: [PATCH 7/8] grep: simplify config parsing, change grep.<rx config> interaction

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 9, 2021, 02:01 UTC
Message-ID: <211109.86ee7qdqz0.gmgdl@evledraar.gmail.com>
In-Reply-To: <YYms4czKVnQFWYQ+@nand.local>

On Mon, Nov 08 2021, Taylor Blau wrote:

Show 37 quoted lines

> On Sat, Nov 06, 2021 at 10:10:53PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> I.e. a user would correctly expect this to keep working:
>>
>>     # ERE grep
>>     git -c grep.extendedRegexp=true grep <pattern>
>>
>> And likewise for "grep.patternType=default" to take precedence over
>> the disfavored "grep.extendedRegexp" option, i.e. the usual "last set
>> wins" semantics.
>>
>>     # BRE grep
>>     git -c grep.extendedRegexp=true -c grep.patternType=basic grep <pattern>
>>
>> But probably not for this to ignore the new "grep.patternType" option
>> entirely, say if /etc/gitconfig was still setting
>> "grep.extendedRegexp", but "~/.gitconfig" used the new
>> "grep.patternType" (and wanted to use the "default" value):
>>
>>     # Was ERE, now BRE
>>     git -c grep.extendedRegexp=true grep.patternType=default grep <pattern>
>
> OK, so this is the case that we'd be "breaking". And I think that the
> new behavior you're outlining here (where a higher-precedence
> grep.patternType=default overrides a lower-precedence
> grep.extendedRegexp=true, resulting in using BRE over ERE) makes more
> sense.
>
> At least, it makes more sense if your expectation of "default" is "the
> default matching behavior", not "fallthrough to grep.extendedRegexp".
>
> In any case, I am sensitive to breaking existing user workflows, but
> this seems so obscure to me that I have a hard time expecting that
> m(any?) users will even notice this at all.
>
> The situation I'm most concerned about is having grep.extendedRegexp set
> in, say, /etc/gitconfig and grep.patternType=default set at a
> higher-precedence level.

*nod*, but the only user who'd end up with that is someone who's trying to override grep.extendedRegexp but failing to do it, so this would help.

Or someone who'd read the docs, understood that we promised that would do nothing, and inserted that just to test us, but that seems unlikely :)

Or, I suppose someone who's entirely confused, and will continue being even more confused now that behavior changes on a git upgrade from ERE to BRE.

I'm hoping the last two paragraphs describe no-one & that this is safe to do.

Show 8 quoted lines

>> ---
>>  Documentation/config/grep.txt |  3 +-
>>  Documentation/git-grep.txt    |  3 +-
>
> Not the fault of your patch, but these two are annoyingly (and subtly)
> different from one another. Could we clean this up and put everything in
> Documentation/config/grep.txt (and then include that in the
> CONFIGURATION section of Documentation/git-grep.txt)?

I've got a large series to do that for all of these, but opted to skip that particular digression here (even for just grep.txt it's a bit distracting).