Re: [PATCH 08/12] grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"

2017-04-11 Thread Jeff King
On Sat, Apr 08, 2017 at 01:25:02PM +, Ævar Arnfjörð Bjarmason wrote:

> Make the pattern types "pcre" & "pcre1" synonyms for long-standing
> "perl" grep.patternType.
> 
> This change is part of a longer patch series to add pcre2 support to
> Git. It's nice to be able to performance test PCRE v1 v.s. v2 without
> having to recompile git, and doing that via grep.patternType makes
> sense.
> 
> However, just adding "pcre2" when we only have "perl" would be
> confusing, so start by adding a "pcre" & "pcre1" synonym.
> 
> In the future "perl" and "pcre" might be changed to default to "pcre2"
> instead of "pcre1", and depending on how Git is compiled the more
> specific "pcre1" or "pcre2" pattern types might produce an error.

I think this makes sense.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d51..5ef12d0694 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1624,6 +1624,15 @@ grep.patternType::
>   'fixed', or 'perl' will enable the `--basic-regexp`, 
> `--extended-regexp`,
>   `--fixed-strings`, or `--perl-regexp` option accordingly, while the
>   value 'default' will return to the default matching behavior.
> ++
> +The 'pcre' and 'pcre1' values are synonyms for 'perl'. The other
> +values starting with 'pcre' are reserved for future use, e.g. if we'd
> +like to use 'pcre2' for the PCRE v2 library.

Do you want to define this the other way around (that perl and pcre are
synonyms for pcre1)?

I know that hey are being added now as synonyms, but from the
perspective of the user (and this is the user-facing documentation), the
end-game we want is "you can pick pcre1 or pcre2, and the others are
floating synonyms that may change".

I think it would be OK to continue to refer to "perl" elsewhere. Even
though it's a synonym, it's the one we'd expect people to use.

-Peff


[PATCH 08/12] grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"

2017-04-08 Thread Ævar Arnfjörð Bjarmason
Make the pattern types "pcre" & "pcre1" synonyms for long-standing
"perl" grep.patternType.

This change is part of a longer patch series to add pcre2 support to
Git. It's nice to be able to performance test PCRE v1 v.s. v2 without
having to recompile git, and doing that via grep.patternType makes
sense.

However, just adding "pcre2" when we only have "perl" would be
confusing, so start by adding a "pcre" & "pcre1" synonym.

In the future "perl" and "pcre" might be changed to default to "pcre2"
instead of "pcre1", and depending on how Git is compiled the more
specific "pcre1" or "pcre2" pattern types might produce an error.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt |  9 +
 grep.c   |  4 +++-
 t/t7810-grep.sh  | 10 ++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d51..5ef12d0694 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1624,6 +1624,15 @@ grep.patternType::
'fixed', or 'perl' will enable the `--basic-regexp`, 
`--extended-regexp`,
`--fixed-strings`, or `--perl-regexp` option accordingly, while the
value 'default' will return to the default matching behavior.
++
+The 'pcre' and 'pcre1' values are synonyms for 'perl'. The other
+values starting with 'pcre' are reserved for future use, e.g. if we'd
+like to use 'pcre2' for the PCRE v2 library.
++
+In the future 'perl' and 'pcre' might become synonyms for some other
+implementation or PCRE version, such as 'pcre2', while the more
+specific 'pcre1' & 'pcre2' might throw errors depending on whether git
+is compiled to include those libraries.
 
 grep.extendedRegexp::
If set to true, enable `--extended-regexp` option by default. This
diff --git a/grep.c b/grep.c
index 8564fe726d..1575f8f9ed 100644
--- a/grep.c
+++ b/grep.c
@@ -60,7 +60,9 @@ static int parse_pattern_type_arg(const char *opt, const char 
*arg)
return GREP_PATTERN_TYPE_ERE;
else if (!strcmp(arg, "fixed"))
return GREP_PATTERN_TYPE_FIXED;
-   else if (!strcmp(arg, "perl"))
+   else if (!strcmp(arg, "perl") ||
+!strcmp(arg, "pcre") ||
+!strcmp(arg, "pcre1"))
return GREP_PATTERN_TYPE_PCRE;
die("bad %s argument: %s", opt, arg);
 }
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 83b0ee53be..b50f1dff43 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1522,4 +1522,14 @@ test_expect_success 'grep with thread options' '
test_must_fail git -c grep.threads=1 grep --threads=-1 st.*dio
 '
 
+test_expect_success LIBPCRE "grep with grep.patternType synonyms 
perl/pcre/pcre1" '
+   echo "#include " >expected &&
+   git -c grep.patternType=perl  grep -h --no-line-number "st(?=dio)" 
>actual &&
+   test_cmp expected actual &&
+   git -c grep.patternType=pcre  grep -h --no-line-number "st(?=dio)" 
>actual &&
+   test_cmp expected actual &&
+   git -c grep.patternType=pcre1 grep -h --no-line-number "st(?=dio)" 
>actual &&
+   test_cmp expected actual
+'
+
 test_done
-- 
2.11.0