Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > Simplify the parsing of "grep.patternType" and > "grep.extendedRegexp". This changes no behavior, but gets rid of > complex parsing logic that isn't needed anymore. > > When "grep.patternType" was introduced in 84befcd0a4a (grep: add a > grep.patternType configuration setting, 2012-08-03) we promised that: > > 1. You can set "grep.patternType", and "[setting it to] 'default' > will return to the default matching behavior". You need to call it into readers' attention that back then the author of that commit meant by "the default" to mean "whatever the configuration system specified before the patch applied, i.e. with grep.extendedRegexp". > 2. We'd support the existing "grep.extendedRegexp" option, but ignore > it when the new "grep.patternType" option is set. We said we'd > only ignore the older "grep.extendedRegexp" option "when the > `grep.patternType` option is set. to a value other than > 'default'". Yes, in short, you can think of it this way. - We use grep.patternType as the source of truth. It is a usual last-one-wins variable, with values like fixed, basic, pcre, etc. - There however is a special value 'default', which may mean 'basic' or 'extended', depending on what grep.extendedRegexp is set to. > In a preceding commit we changed grep_config() to be called after > grep_init(), which means that much of the complexity here can go > away. > > Now as before when we only understand a "grep.extendedRegexp" setting > of "true", and if "grep.patterntype=default" is set we'll interpret it > as "grep.patterntype=basic", Is that a typo? If extendedRegexp is set to 'true', then the 'default' would mean 'extended', so I would expect that we'd see it as the same as setting it 'grep.patternType=extended'. > except if we previously saw a > "grep.extendedRegexp", then it's interpreted as > "grep.patterntype=extended". I am not sure what this means. grep.extendedRegexp is also a usual last-one-wins variable. If you had this series: git \ -c grep.extendedRegexp = false \ -c grep.extendedRegexp = true \ -c grep.patternType = default \ some-command-that-take-regexp the last grep.extendedRegexp is true, and the last grep.patternType is default, so the command sould work on extended. It is also true if you had git \ -c grep.extendedRegexp = false \ -c grep.patternType = default \ -c grep.extendedRegexp = true \ some-command-that-take-regexp That is why it is important to remember the fact that patternType was give as "default" before you finish reading the configuration and you are sure you know the last value of grep.extendedRegexp. Only after that, you can resolve what that "default" means between "basic" and "extended". Trying to interpret "default" as soon as you see it in grep.patternType and trying to make it into either "basic" or "extended" will not work unless you know you have the final value of grep.extendedRegexp. > We don't need grep_commit_pattern_type() anymore,... And I think that is what this function wanted to say: "we now have seen all necessary, so we can commit what pattern type we are going to use; before this point, we couldn't tell what 'default' meant". So I am not sure how any change that says we do not need the "commit" phase can be correct.