[PATCH 8/8] grep: make "extendedRegexp=true" the same as "patternType=extended"

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"J Smith" <dark.panda@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 8/8] grep: make "extendedRegexp=true" the same as "patternType=extended"
Date: Sat,  6 Nov 2021 22:10:54 +0100	[thread overview]
Message-ID: <patch-8.8-dadd5dff77a-20211106T210711Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-0.8-00000000000-20211106T210711Z-avarab@gmail.com>

In the preceding commit we changed how a "grep.patternType=default"
set after "grep.extendedRegexp=true" would be handled so that the last
set would win, but a "grep.extendedRegexp=true" would only be used if
"grep.patternType" was set to a value other than "default".

Thus a user who had old config and set "grep.extendedRegexp=true" in
their ~/.gitconfig expecting ERE behavior would be opted-in to say
"perl" regexes if a system "/etc/gitconfig" started setting
"grep.patternType=perl".

These funny semantics of only paying attention to a set if another key
is not set to a given value aren't how we treat other config keys, so
let's do away with this caveat for consistency.

The new semantics are simple, a "grep.extendedRegexp=true" is an exact
synonym for specifying "grep.patternType=extended" in the
config. We'll keep ignoring ""grep.extendedRegexp=false", although
arguably we could treat it as a "grep.patternType=basic".

As argued in the preceding commit I think this behavior came about
because we were conflating the state of our code's own internal
"default" value with what we found in explicit user config. See
84befcd0a4a (grep: add a grep.patternType configuration setting,
2012-08-03) for that past behavior.

Let's further change the documentation to note that
"grep.extendedRegexp" is a deprecated synonym, perhaps we'll be able
to remove it at some point in the future and do away with this
special-case entirely.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config/grep.txt | 3 +--
 Documentation/git-grep.txt    | 3 +--
 grep.c                        | 8 +++-----
 grep.h                        | 5 +----
 t/t7810-grep.sh               | 2 +-
 5 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/Documentation/config/grep.txt b/Documentation/config/grep.txt
index 2669b1757d3..9868ea8a061 100644
--- a/Documentation/config/grep.txt
+++ b/Documentation/config/grep.txt
@@ -11,8 +11,7 @@ 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::
 	Number of grep worker threads to use.
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 078dfeadf50..211ba6801b0 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -347,8 +347,7 @@ 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::
 	Number of grep worker threads to use. If unset (or set to 0), Git will
diff --git a/grep.c b/grep.c
index dda8e536fe3..ef8746d85f0 100644
--- a/grep.c
+++ b/grep.c
@@ -33,9 +33,8 @@ static const char *color_grep_slots[] = {
 
 static int parse_pattern_type_arg(const char *opt, const char *arg)
 {
-	if (!strcmp(arg, "default"))
-		return GREP_PATTERN_TYPE_UNSPECIFIED;
-	else if (!strcmp(arg, "basic"))
+	if (!strcmp(arg, "basic") ||
+	    !strcmp(arg, "default"))
 		return GREP_PATTERN_TYPE_BRE;
 	else if (!strcmp(arg, "extended"))
 		return GREP_PATTERN_TYPE_ERE;
@@ -60,8 +59,7 @@ int grep_config(const char *var, const char *value, void *cb)
 	if (userdiff_config(var, value) < 0)
 		return -1;
 
-	if (opt->pattern_type_option == GREP_PATTERN_TYPE_UNSPECIFIED &&
-	    !strcmp(var, "grep.extendedregexp") &&
+	if (!strcmp(var, "grep.extendedregexp") &&
 	    git_config_bool(var, value)) {
 		opt->pattern_type_option = GREP_PATTERN_TYPE_ERE;
 		return 0;
diff --git a/grep.h b/grep.h
index e4e548aed90..55b23d045e0 100644
--- a/grep.h
+++ b/grep.h
@@ -94,8 +94,7 @@ enum grep_expr_node {
 };
 
 enum grep_pattern_type {
-	GREP_PATTERN_TYPE_UNSPECIFIED = 0,
-	GREP_PATTERN_TYPE_BRE,
+	GREP_PATTERN_TYPE_BRE = 0,
 	GREP_PATTERN_TYPE_ERE,
 	GREP_PATTERN_TYPE_FIXED,
 	GREP_PATTERN_TYPE_PCRE
@@ -179,7 +178,6 @@ struct grep_opt {
 	.relative = 1, \
 	.pathname = 1, \
 	.max_depth = -1, \
-	.pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED, \
 	.colors = { \
 		[GREP_COLOR_CONTEXT] = "", \
 		[GREP_COLOR_FILENAME] = "", \
@@ -191,7 +189,6 @@ struct grep_opt {
 		[GREP_COLOR_SELECTED] = "", \
 		[GREP_COLOR_SEP] = GIT_COLOR_CYAN, \
 	}, \
-	.only_matching = 0, \
 	.color = -1, \
 	.output = std_output, \
 }
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index a59a9726357..afca938a4d0 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -461,7 +461,7 @@ do
 	'
 
 	test_expect_success "grep $L with grep.patternType=basic and grep.extendedRegexp=true" '
-		echo "${HC}ab:a+bc" >expected &&
+		echo "${HC}ab:abc" >expected &&
 		git \
 			-c grep.patternType=basic \
 			-c grep.extendedRegexp=true \
-- 
2.34.0.rc1.741.gab7bfd97031

