| Message ID | xmqqa6gp35a6.fsf_-_@gitster.g (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC/PATCH] grep: allow scripts to ignore configured pattern type | expand |
On Fri, Dec 24 2021, Junio C Hamano wrote: > We made a mistake to add grep.extendedRegexp configuration variable > long time ago, and made things even worse by introducing an even > more generalized grep.patternType configuration variable. > > This was mostly because interactive users were lazy and wanted a way > to declare "I do not live in the ancient age, and my regexps are > always extended" and write "git grep" without having to type three > more letters " -E" on the command line. > > But this in turn forces script writers to always specify what kind > of patterns they are writing, because without such command line > override, the interpretation of the patterns they write in their > scripts will be affected by the configuration variables of the user > who is running their script. > > Introduce GIT_DISABLE_GREP_PATTERN_CONFIG environment variable that > script writers can set to "true" and export at the very beginning of > their script to defeat grep.extendedRegexp and grep.patternType > configuration variables. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > > * This is merely a weather balloon without proper documentation and > test. It might be even better idea to make such an environment > variable to _specify_ what kind of pattern the script uses, > instead of "we defeat end-user configuration and now we are > forced to write in basic or have to write -E/-P etc.", which is > what this patch does. You note the lack of documentation. I do think anything in this direction would do well to: * Specify what it is we're promising now exactly. The git-grep command is in "main porcelain" now, this change sounds like we're promising to make its output more plumbing-like. * As an aside I think a good follow-up to my series would be to just start warning() and eventually die()-ing on grep.extendedRegexp which would make this a bit simpler. * A "GIT_DISABLE_GREP_PATTERN_CONFIG" seems overly narrow. Just a few lines from the code being patched here we read the grep.lineNumber config, which is similarly annoying if you're parsing the "git grep" output, so at least a "GIT_DISABLE_GREP_CONFIG" would be handy. * But more generally we've had discussions on and off on-list about supporting a generic way to disable reading the config. Supporting e.g. "git --no-config" or a "GIT_NO_CONFIG" would be handy, even if all it did for now (and we could document it as such) would be to change the behavior of grep. > grep.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/grep.c b/grep.c > index fe847a0111..0cfb698b51 100644 > --- a/grep.c > +++ b/grep.c > @@ -77,10 +77,15 @@ int grep_config(const char *var, const char *value, void *cb) > { > struct grep_opt *opt = &grep_defaults; > const char *slot; > + static int disable_pattern_type_config = -1; > > if (userdiff_config(var, value) < 0) > return -1; > > + if (disable_pattern_type_config < 0) > + disable_pattern_type_config = > + git_env_bool("GIT_DISABLE_GREP_PATTERN_CONFIG", 0); > + > /* > * The instance of grep_opt that we set up here is copied by > * grep_init() to be used by each individual invocation. > @@ -90,12 +95,14 @@ int grep_config(const char *var, const char *value, void *cb) > */ > > if (!strcmp(var, "grep.extendedregexp")) { > - opt->extended_regexp_option = git_config_bool(var, value); > + if (!disable_pattern_type_config) > + opt->extended_regexp_option = git_config_bool(var, value); > return 0; > } > > if (!strcmp(var, "grep.patterntype")) { > - opt->pattern_type_option = parse_pattern_type_arg(var, value); > + if (!disable_pattern_type_config) > + opt->pattern_type_option = parse_pattern_type_arg(var, value); > return 0; > }
diff --git a/grep.c b/grep.c index fe847a0111..0cfb698b51 100644 --- a/grep.c +++ b/grep.c @@ -77,10 +77,15 @@ int grep_config(const char *var, const char *value, void *cb) { struct grep_opt *opt = &grep_defaults; const char *slot; + static int disable_pattern_type_config = -1; if (userdiff_config(var, value) < 0) return -1; + if (disable_pattern_type_config < 0) + disable_pattern_type_config = + git_env_bool("GIT_DISABLE_GREP_PATTERN_CONFIG", 0); + /* * The instance of grep_opt that we set up here is copied by * grep_init() to be used by each individual invocation. @@ -90,12 +95,14 @@ int grep_config(const char *var, const char *value, void *cb) */ if (!strcmp(var, "grep.extendedregexp")) { - opt->extended_regexp_option = git_config_bool(var, value); + if (!disable_pattern_type_config) + opt->extended_regexp_option = git_config_bool(var, value); return 0; } if (!strcmp(var, "grep.patterntype")) { - opt->pattern_type_option = parse_pattern_type_arg(var, value); + if (!disable_pattern_type_config) + opt->pattern_type_option = parse_pattern_type_arg(var, value); return 0; }
We made a mistake to add grep.extendedRegexp configuration variable long time ago, and made things even worse by introducing an even more generalized grep.patternType configuration variable. This was mostly because interactive users were lazy and wanted a way to declare "I do not live in the ancient age, and my regexps are always extended" and write "git grep" without having to type three more letters " -E" on the command line. But this in turn forces script writers to always specify what kind of patterns they are writing, because without such command line override, the interpretation of the patterns they write in their scripts will be affected by the configuration variables of the user who is running their script. Introduce GIT_DISABLE_GREP_PATTERN_CONFIG environment variable that script writers can set to "true" and export at the very beginning of their script to defeat grep.extendedRegexp and grep.patternType configuration variables. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * This is merely a weather balloon without proper documentation and test. It might be even better idea to make such an environment variable to _specify_ what kind of pattern the script uses, instead of "we defeat end-user configuration and now we are forced to write in basic or have to write -E/-P etc.", which is what this patch does. grep.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)