Re: [PATCH 03/12] Makefile & configure: reword outdated comment about PCRE

2017-04-15 Thread Ævar Arnfjörð Bjarmason
On Tue, Apr 11, 2017 at 12:14 PM, Jeff King  wrote:
> On Sat, Apr 08, 2017 at 01:24:57PM +, Ævar Arnfjörð Bjarmason wrote:
>
>> Reword an outdated comment which suggests that only git-grep can use
>> PCRE.
>
> Makes sense.
>
>> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
>> -# able to use Perl-compatible regular expressions.
>> +# Define USE_LIBPCRE if you have and want to use libpcre. Various
>> +# commands such as like log, grep offer runtime options to use
>> +# Perl-compatible regular expressions instead of standard or extended
>> +# POSIX regular expressions.
>
> s/such as like log, grep/such as log and grep/ ?
Thanks.

>> diff --git a/configure.ac b/configure.ac
>> [...]
>> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
>> -# able to use Perl-compatible regular expressions.
>> +# Define USE_LIBPCRE if you have and want to use libpcre. Various
>> +# commands such as like log, grep offer runtime options to use
>> +# Perl-compatible regular expressions instead of standard or extended
>> +# POSIX regular expressions.
>
> Ditto.
>
>> @@ -499,8 +501,10 @@ GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])
>>  GIT_CONF_SUBST([NO_OPENSSL])
>>
>>  #
>> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
>> -# able to use Perl-compatible regular expressions.
>> +# Define USE_LIBPCRE if you have and want to use libpcre. Various
>> +# commands such as like log, grep offer runtime options to use
>> +# Perl-compatible regular expressions instead of standard or extended
>> +# POSIX regular expressions.
>
> And again. It's weird that the text appears twice in configure.ac.
> Apparently you can use --with-libpcre or USE_LIBPCRE in the environment?
> Configure is weird.

You can't, I've added this to the commit message:

Copy/pasting this so much in configure.ac is lame, these Makefile-like
flags aren't even used by autoconf, just the corresponding
--with[out]-* options. But copy/pasting the comments that make sense
for the Makefile to configure.ac where they make less sence is the
pattern everything else follows in that file. I'm not going to war
against that as part of this change, just following the existing
pattern.


Re: [PATCH 03/12] Makefile & configure: reword outdated comment about PCRE

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

> Reword an outdated comment which suggests that only git-grep can use
> PCRE.

Makes sense.

> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
> -# able to use Perl-compatible regular expressions.
> +# Define USE_LIBPCRE if you have and want to use libpcre. Various
> +# commands such as like log, grep offer runtime options to use
> +# Perl-compatible regular expressions instead of standard or extended
> +# POSIX regular expressions.

s/such as like log, grep/such as log and grep/ ?

> diff --git a/configure.ac b/configure.ac
> [...]
> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
> -# able to use Perl-compatible regular expressions.
> +# Define USE_LIBPCRE if you have and want to use libpcre. Various
> +# commands such as like log, grep offer runtime options to use
> +# Perl-compatible regular expressions instead of standard or extended
> +# POSIX regular expressions.

Ditto.

> @@ -499,8 +501,10 @@ GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])
>  GIT_CONF_SUBST([NO_OPENSSL])
>  
>  #
> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
> -# able to use Perl-compatible regular expressions.
> +# Define USE_LIBPCRE if you have and want to use libpcre. Various
> +# commands such as like log, grep offer runtime options to use
> +# Perl-compatible regular expressions instead of standard or extended
> +# POSIX regular expressions.

And again. It's weird that the text appears twice in configure.ac.
Apparently you can use --with-libpcre or USE_LIBPCRE in the environment?
Configure is weird.

-Peff


[PATCH 03/12] Makefile & configure: reword outdated comment about PCRE

2017-04-08 Thread Ævar Arnfjörð Bjarmason
Reword an outdated comment which suggests that only git-grep can use
PCRE.

This comment was added back when PCRE support was initially added in
commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09), and was true
at the time.

It hasn't been telling the full truth since git-log learned to use
PCRE with --grep in commit 727b6fc3ed ("log --grep: accept
--basic-regexp and --perl-regexp", 2012-10-03), and more importantly
is likely to get more inaccurate over time as more use is made of PCRE
in other areas.

Reword it to be more future-proof, and to more clearly explain that
this enables user-initiated runtime behavior.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Makefile |  6 --
 configure.ac | 12 
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 9b36068ac5..23945d87cf 100644
--- a/Makefile
+++ b/Makefile
@@ -24,8 +24,10 @@ all::
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
 # This also implies BLK_SHA1.
 #
-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
-# able to use Perl-compatible regular expressions.
+# Define USE_LIBPCRE if you have and want to use libpcre. Various
+# commands such as like log, grep offer runtime options to use
+# Perl-compatible regular expressions instead of standard or extended
+# POSIX regular expressions.
 #
 # Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
diff --git a/configure.ac b/configure.ac
index 128165529f..d09a204a7e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,8 +250,10 @@ AS_HELP_STRING([--with-openssl],[use OpenSSL library 
(default is YES)])
 AS_HELP_STRING([],  [ARG can be prefix for openssl library and 
headers]),
 GIT_PARSE_WITH([openssl]))
 
-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
-# able to use Perl-compatible regular expressions.
+# Define USE_LIBPCRE if you have and want to use libpcre. Various
+# commands such as like log, grep offer runtime options to use
+# Perl-compatible regular expressions instead of standard or extended
+# POSIX regular expressions.
 #
 # Define LIBPCREDIR=/foo/bar if your libpcre header and library files are in
 # /foo/bar/include and /foo/bar/lib directories.
@@ -499,8 +501,10 @@ GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])
 GIT_CONF_SUBST([NO_OPENSSL])
 
 #
-# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
-# able to use Perl-compatible regular expressions.
+# Define USE_LIBPCRE if you have and want to use libpcre. Various
+# commands such as like log, grep offer runtime options to use
+# Perl-compatible regular expressions instead of standard or extended
+# POSIX regular expressions.
 #
 
 if test -n "$USE_LIBPCRE"; then
-- 
2.11.0