Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > Change the interaction between "grep.patternType=default" and > "grep.extendedRegexp=true" to make setting "grep.extendedRegexp=true" > synonymous with setting "grep.patternType=extended". This description alone is not quite understandable. It is not saying much more than the single line title, and presense of it does not seem to improve the understanding by the readers. > When "grep.patternType" was introduced in 84befcd0a4a (grep: add a > grep.patternType configuration setting, 2012-08-03) we made two > seemingly contradictory promises: > > 1. You can set "grep.patternType", and "[setting it to] 'default' > will return to the default matching behavior". > > 2. Support the existing "grep.extendedRegexp" option, but ignore it > when the new "grep.patternType" is set, *except* "when the > `grep.patternType` option is set. to a value other than 'default'". OK, so setting grep.patternType=default makes grep.extendedRegexp to be taken into account. By grep.patternType to something else, the other one is ignored. 2. is a very explicit way to say so. Where did you get 1. from? If you have this paragraph in the log message in mind, I agree that it is less than ideally phrased, but ... Rather than adding an additional setting for grep.fooRegexp for current and future pattern matching options, add a grep.patternType setting that can accept appropriate values for modifying the default grep pattern matching behavior. The current values are "basic", "extended", "fixed", "perl" and "default" for setting -G, -E, -F, -P and the default behavior respectively. ... with the understanding of 2. (which is in what the commit adds to Documentation/config.txt), it is reasonable to understand that "the default behaviour" is "use BRE or ERE, depending on the setting of grep.extendedRegexp". Doesn't the code behave that way? I think the above is exactly how the commit wanted to make the code behave. > I think that 84befcd0a4a probably didn't intend this behavior, but > instead ended up conflating our internal "unspecified" state with a > user's explicit desire to set the configuration back to the > default. I am not sure where that comes from, but if I imagine somebody confuses between "default" and "basic" and considers "default" a synonym for "basic", I can sort-of understand it. Is it what is happening here? But it is not what the original .patternType patch wanted to do back then, and it is not what we want to see now. > I.e. a user would correctly expect this to keep working: > > # ERE grep > git -c grep.extendedRegexp=true grep <pattern> This makes sense. > 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> This makes sense, too. Do either of the above two not work as you expect (i.e. the first use ERE and the second use BRE)? What I have trouble with is that it is unclear if you are describing what should happen (in the above, I said "makes sense", to show my agreement, assuming that it is the case), or if you are describing what does happen that you disagree with. Another thing I have trouble with is your mention of "keep working". Are you proposing to deliberately break what is working as users correctly expect? Why? > But probably not for this to ignore the favored "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> I do not quite get your "Was X, now Y" label. What did you want to say with that? Also I am not sure what you exactly mean when you say "and wanted to use the 'default' value". There is no single "THE" default value. If patternType=default is the last patterntype (it may be set in many places, but the last one should win), the user is telling that the last-one-wins setting of extendedRegexp is to be honored. So, if grep.extendedRegexp in /etc/gitconfig is the last one defined, we would choose between BRE and ERE depending on that setting. Isn't that what is happening in the current code? Or are all the above explanation result of simple misunderstanding that setting to "default" means setting to "basic"?