Re: [RFC/PATCH] grep: allow scripts to ignore configured pattern type

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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;
>  	}