This series changes the behavior of an obscure interaction between "grep.extendedRegexp=true" and "grep.patternType=default". See 7-8/8. Along the way we can delete a lot of code that was needed to support the previous behavior. For v1 and a more extensive summary see [1]. Thanks a lot Taylor for the detailed review on v1! Hopefully this v1 addresses all the feedback on one way or another, it's still 8 patches, but much of the early part of v1 is squashed together & re-done as suggested. Then there's a mid-series de-duplication of the grep config documentation. I ended up keeping the change to not needlessly pass "cb" around in grep_cmd_config(), but that's now also in its own patch. 1. https://lore.kernel.org/git/cover-0.8-00000000000-20211106T210711Z-avarab@xxxxxxxxx/ Ævar Arnfjörð Bjarmason (8): grep.h: remove unused "regex_t regexp" from grep_opt built-ins: trust the "prefix" from run_builtin() log tests: check if grep_config() is called by "log"-like cmds grep docs: de-duplicate configuration sections grep.c: don't pass along NULL callback value grep API: call grep_config() after grep_init() grep: simplify config parsing, change grep.<rx config> interaction grep: make "extendedRegexp=true" the same as "patternType=extended" Documentation/config/grep.txt | 11 ++-- Documentation/git-grep.txt | 30 +-------- builtin/grep.c | 27 ++++---- builtin/log.c | 13 +++- builtin/ls-tree.c | 9 ++- git.c | 4 +- grep.c | 118 ++++------------------------------ grep.h | 35 ++++++---- revision.c | 4 +- t/t4202-log.sh | 16 +++++ t/t7810-grep.sh | 4 +- 11 files changed, 97 insertions(+), 174 deletions(-) Range-diff against v1: 1: 412b8b65266 = 1: 1435db727ef grep.h: remove unused "regex_t regexp" from grep_opt 2: 244715e3497 < -: ----------- git.c & grep.c: assert that "prefix" is NULL or non-zero string 3: 3338cc95b81 < -: ----------- grep: remove unused "prefix_length" member 4: 78298657d69 < -: ----------- grep.c: move "prefix" out of "struct grep_opt" -: ----------- > 2: 63cf2fe266d built-ins: trust the "prefix" from run_builtin() 5: ba9be0b9283 = 3: 41e38ebb32c log tests: check if grep_config() is called by "log"-like cmds -: ----------- > 4: efe95397d72 grep docs: de-duplicate configuration sections -: ----------- > 5: d0f0ac6c7ae grep.c: don't pass along NULL callback value 6: 933ac853bca ! 6: 917944f79a5 grep API: call grep_config() after grep_init() @@ Commit message didn't do that, but now that it can't be a concern anymore let's remove those comments. + This does not change the behavior of any of the configuration + variables or options. That would have been the case if we didn't move + around the grep_config() call in "builtin/log.c". But now that we call + "grep_config" after "git_log_config" and "git_format_config" we'll + need to pass in the already initialized "struct grep_opt *". + See 6ba9bb76e02 (grep: copy struct in one fell swoop, 2020-11-29) and 7687a0541e0 (grep: move the configuration parsing logic to grep.[ch], 2012-10-09) for the commits that added the comments. + The memcpy() pattern here will be optimized away and follows the + convention of other *_init() functions. See 5726a6b4012 (*.c *_init(): + define in terms of corresponding *_INIT macro, 2021-07-01). + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> ## builtin/grep.c ## @@ builtin/grep.c: static int wait_all(void) + static int grep_cmd_config(const char *var, const char *value, void *cb) { - int st = grep_config(var, value, cb); -- if (git_color_default_config(var, value, cb) < 0) -+ if (git_color_default_config(var, value, NULL) < 0) +- int st = grep_config(var, value, NULL); ++ int st = grep_config(var, value, cb); + if (git_color_default_config(var, value, NULL) < 0) st = -1; - if (!strcmp(var, "grep.threads")) { @@ builtin/grep.c: int cmd_grep(int argc, const char **argv, const char *prefix) - OPT_END() }; + grep_prefix = prefix; - git_config(grep_cmd_config, NULL); grep_init(&opt, the_repository); + git_config(grep_cmd_config, &opt); - opt.caller_priv = &opt_cmd; /* + * If there is no -- then the paths must exist in the working ## builtin/log.c ## @@ builtin/log.c: static int git_log_config(const char *var, const char *value, void *cb) @@ grep.c: int grep_config(const char *var, const char *value, void *cb) ## grep.h ## @@ grep.h: struct grep_opt { + int show_hunk_mark; + int file_break; + int heading; ++ void *caller_priv; + void *priv; + + void (*output)(struct grep_opt *opt, const void *data, size_t size); void *output_priv; }; 7: 677a8f8520f ! 7: 140a7416223 grep: simplify config parsing, change grep.<rx config> interaction @@ Commit message but in a way that's consistent with how we parse other configuration. - Pedantically speaking we're probably breaking past promises here, but - I doubt that this will impact anyone in practice. The reduction in - complexity and resulting consistency with other default config - behavior is worth it. + We are breaking past promises here, but I doubt that this will impact + anyone in practice. The reduction in complexity and resulting + consistency with other default config behavior is worth it. When "grep.patternType" was introduced in 84befcd0a4a (grep: add a grep.patternType configuration setting, 2012-08-03) we made two @@ Commit message # 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 + 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): @@ Documentation/config/grep.txt: grep.patternType:: If set to true, enable `--extended-regexp` option by default. This - option is ignored when the `grep.patternType` option is set to a value - other than 'default'. -+ option is ignored when the `grep.patternType` option is set. - - grep.threads:: - Number of grep worker threads to use. - - ## Documentation/git-grep.txt ## -@@ Documentation/git-grep.txt: grep.patternType:: - - grep.extendedRegexp:: - If set to true, enable `--extended-regexp` option by default. This -- option is ignored when the `grep.patternType` option is set to a value -- other than 'default'. + option is ignored when the `grep.patternType` option is set. grep.threads:: 8: dadd5dff77a ! 8: cc904d93b26 grep: make "extendedRegexp=true" the same as "patternType=extended" @@ Documentation/config/grep.txt: grep.patternType:: grep.extendedRegexp:: - If set to true, enable `--extended-regexp` option by default. This - option is ignored when the `grep.patternType` option is set. -+ Deprecated synonym for 'grep.patternType=extended`. - - grep.threads:: - Number of grep worker threads to use. - - ## Documentation/git-grep.txt ## -@@ Documentation/git-grep.txt: grep.patternType:: - value 'default' will return to the default matching behavior. - - grep.extendedRegexp:: -- If set to true, enable `--extended-regexp` option by default. This -- option is ignored when the `grep.patternType` option is set. + Deprecated synonym for 'grep.patternType=extended`. grep.threads:: @@ grep.h: struct grep_opt { .colors = { \ [GREP_COLOR_CONTEXT] = "", \ [GREP_COLOR_FILENAME] = "", \ -@@ grep.h: struct grep_opt { - [GREP_COLOR_SELECTED] = "", \ - [GREP_COLOR_SEP] = GIT_COLOR_CYAN, \ - }, \ -- .only_matching = 0, \ - .color = -1, \ - .output = std_output, \ - } ## t/t7810-grep.sh ## @@ t/t7810-grep.sh: do -- 2.34.0.rc1.741.gab7bfd97031
