Re: [RFC/PATCH] grep: allow scripts to ignore configured pattern type
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Dec 26, 2021, 23:09 UTC
- Message-ID
- <211227.86czljez86.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <xmqqa6gp35a6.fsf_-_@gitster.g>
On Fri, Dec 24 2021, Junio C Hamano wrote:
Show 29 quoted lines
> 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.
Show 39 quoted lines
> 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;
> }