[RFC/PATCH] Use compatibility regex library for OSX/Darwin

[RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Arjen Laarhoven]

The standard libc regex library on OSX does not support alternation
in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
functionality on OSX.

To fix this, we use the GNU regex library which is already present in
the compat/ diretory for the MinGW port.  However, simply adding compat/
to the COMPAT_CFLAGS variable causes a conflict between the system
fnmatch.h and the one present in compat/.  To remedy this, move the
regex and fnmatch functionality to their own subdirectories in compat/
so they can be included seperately.

Signed-off-by: Arjen Laarhoven <arjen@yaph.org>
---
This patch is based on 'maint'.  It needs testing on MinGW (although
the change is trivial).

Also, I'm sure the problem occurs on more non-Linux systems (or non
GNU libc systems).  If people who have access to those systems (BSD's,
HP-UX, AIX, etc) can test it, I'd be happy to add those systems to the
patch so it can fix for multiple systems at once.

 Makefile                       |    6 ++++--
 compat/{ => fnmatch}/fnmatch.c |    0
 compat/{ => fnmatch}/fnmatch.h |    0
 compat/{ => regex}/regex.c     |    0
 compat/{ => regex}/regex.h     |    0
 t/t4018-diff-funcname.sh       |    6 ++++++
 6 files changed, 10 insertions(+), 2 deletions(-)
 rename compat/{ => fnmatch}/fnmatch.c (100%)
 rename compat/{ => fnmatch}/fnmatch.h (100%)
 rename compat/{ => regex}/regex.c (100%)
 rename compat/{ => regex}/regex.h (100%)

diff --git a/Makefile b/Makefile
index 672ea74..a8b3f9e 100644
--- a/Makefile
+++ b/Makefile
@@ -626,6 +626,8 @@ ifeq ($(uname_S),Darwin)
 	endif
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
+	COMPAT_CFLAGS += -Icompat/regex
+	COMPAT_OBJS += compat/regex/regex.o
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -750,10 +752,10 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_SVN_TESTS = YesPlease
 	NO_PERL_MAKEMAKER = YesPlease
 	NO_POSIX_ONLY_PROGRAMS = YesPlease
-	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat
+	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch
 	COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
-	COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o compat/winansi.o
+	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/regex/regex.o compat/winansi.o
 	EXTLIBS += -lws2_32
 	X = .exe
 	gitexecdir = ../libexec/git-core
diff --git a/compat/fnmatch.c b/compat/fnmatch/fnmatch.c
similarity index 100%
rename from compat/fnmatch.c
rename to compat/fnmatch/fnmatch.c
diff --git a/compat/fnmatch.h b/compat/fnmatch/fnmatch.h
similarity index 100%
rename from compat/fnmatch.h
rename to compat/fnmatch/fnmatch.h
diff --git a/compat/regex.c b/compat/regex/regex.c
similarity index 100%
rename from compat/regex.c
rename to compat/regex/regex.c
diff --git a/compat/regex.h b/compat/regex/regex.h
similarity index 100%
rename from compat/regex.h
rename to compat/regex/regex.h
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 833d6cb..18bcd97 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -57,4 +57,10 @@ test_expect_success 'last regexp must not be negated' '
 	test_must_fail git diff --no-index Beer.java Beer-correct.java
 '
 
+test_expect_success 'alternation in pattern' '
+	git config diff.java.funcname "^[ 	]*\\(\\(public\\|static\\).*\\)$"
+	git diff --no-index Beer.java Beer-correct.java |
+	grep "^@@.*@@ public static void main("
+'
+
 test_done
-- 
1.6.0.1.337.g5c7d67

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Mike Ralphson]

2008/9/7 Arjen Laarhoven <arjen@yaph.org>
> The standard libc regex library on OSX does not support alternation
> in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
> functionality on OSX.
>
> Also, I'm sure the problem occurs on more non-Linux systems (or non
> GNU libc systems).  If people who have access to those systems (BSD's,
> HP-UX, AIX, etc) can test it, I'd be happy to add those systems to the
> patch so it can fix for multiple systems at once.

I can confirm that the issue shown up by your new testcase is also
present in AIX 5.3.

Doing the mv's and adding regex.o to COMPAT_OBJS works fine and
compat/lib.a is suitably amended

ar -t compat/lib.a
regex.o
...

but I'm afraid I don't know how to get the combination of gcc + AIX ld
to link to this in preference to the system defined regex functions.
At least I guess this is what's happening.

Adding -lcompat/regex to COMPAT_CFLAGS as per your Darwin hunk
provokes lots of warnings:

gcc: -lcompat/regex: linker input file unused because linking not done
... (" for each .c file)
    LINK git-fast-import
collect2: library libcompat/regex not found

That said, I'm not specifically arguing in favour of the patch (I
wouldn't know a regex alternation if it bit me), someone writing
regexs on HP-UX might expect them to work like the rest of their OS...

If the shipped java funcname pattern requires alternation then the
testcase should stand though.

Mike

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Johannes Sixt]

Mike Ralphson schrieb:
> 2008/9/7 Arjen Laarhoven <arjen@yaph.org>
>> The standard libc regex library on OSX does not support alternation
>> in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
>> functionality on OSX.
>>
>> Also, I'm sure the problem occurs on more non-Linux systems (or non
>> GNU libc systems).  If people who have access to those systems (BSD's,
>> HP-UX, AIX, etc) can test it, I'd be happy to add those systems to the
>> patch so it can fix for multiple systems at once.

The MinGW part of this patch works just fine:

Tested-by: Johannes Sixt <johannes.sixt@telecom.at>

> I can confirm that the issue shown up by your new testcase is also
> present in AIX 5.3.
...
> but I'm afraid I don't know how to get the combination of gcc + AIX ld
> to link to this in preference to the system defined regex functions.

Does this patchlet help? It gives no warnings or errors on my AIX 4.3.3
box and passes the new test (which fails without this).

diff --git a/Makefile b/Makefile
index 98d67f1..0637419 100644
--- a/Makefile
+++ b/Makefile
@@ -702,6 +702,8 @@ ifeq ($(uname_S),AIX)
 	INTERNAL_QSORT = UnfortunatelyYes
 	NEEDS_LIBICONV=YesPlease
 	BASIC_CFLAGS += -D_LARGE_FILES
+	COMPAT_CFLAGS += -Icompat/regex
+	COMPAT_OBJS += compat/regex/regex.o
 endif
 ifeq ($(uname_S),GNU)
 	# GNU/Hurd

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Arjen Laarhoven]

On Wed, Sep 10, 2008 at 09:03:05AM +0100, Mike Ralphson wrote:
> 2008/9/7 Arjen Laarhoven <arjen@yaph.org>
> > The standard libc regex library on OSX does not support alternation
> > in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
> > functionality on OSX.
> >
> > Also, I'm sure the problem occurs on more non-Linux systems (or non
> > GNU libc systems).  If people who have access to those systems (BSD's,
> > HP-UX, AIX, etc) can test it, I'd be happy to add those systems to the
> > patch so it can fix for multiple systems at once.
> 
> I can confirm that the issue shown up by your new testcase is also
> present in AIX 5.3.

Ok, I'll add AIX to the fix and a Tested-by: line.

[snip]

> Adding -lcompat/regex to COMPAT_CFLAGS as per your Darwin hunk
> provokes lots of warnings:

I think your problem is a lowercase ell instead of an uppercase i ;-)

Arjen

-- 
Arjen Laarhoven

The presence of those seeking the truth is infinitely to be preferred to 
those who think they've found it.
                                -- Terry Pratchett, "Monstrous Regiment"

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Mike Ralphson]

2008/9/10 Arjen Laarhoven <arjen@yaph.org>:
> On Wed, Sep 10, 2008 at 09:03:05AM +0100, Mike Ralphson wrote:
>> 2008/9/7 Arjen Laarhoven <arjen@yaph.org>
>> > The standard libc regex library on OSX does not support alternation
>> > in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
>> > functionality on OSX.
>> >
>> > Also, I'm sure the problem occurs on more non-Linux systems (or non
>> > GNU libc systems).  If people who have access to those systems (BSD's,
>> > HP-UX, AIX, etc) can test it, I'd be happy to add those systems to the
>> > patch so it can fix for multiple systems at once.
>>
>> I can confirm that the issue shown up by your new testcase is also
>> present in AIX 5.3.

>> Adding -lcompat/regex to COMPAT_CFLAGS as per your Darwin hunk
>> provokes lots of warnings:
>
> I think your problem is a lowercase ell instead of an uppercase i ;-)

Doh. I think my problem is this font! And having not used a language
with a separate linker since last century.

I guess I picked a really bad patch to try and pick up and test direct
from gmail. No reflection on your patch, just my workflow.

Having carefully checked the content of the testcase too, I can now
say this does fix the issue without extra warnings or testcase
failures on AIX 5.3, so for what it's worth:

Tested-by: Mike Ralphson <mike@abacus.co.uk>

Thanks both, and sorry for the noise.

Mike

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Mike Ralphson]

2008/9/10 Mike Ralphson <mike.ralphson@gmail.com>:
> 2008/9/10 Arjen Laarhoven <arjen@yaph.org>:
>> On Wed, Sep 10, 2008 at 09:03:05AM +0100, Mike Ralphson wrote:
>>> 2008/9/7 Arjen Laarhoven <arjen@yaph.org>
>>> > The standard libc regex library on OSX does not support alternation
>>> > in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
>>> > functionality on OSX.
>>> >
>>> > Also, I'm sure the problem occurs on more non-Linux systems (or non
>>> > GNU libc systems).  If people who have access to those systems (BSD's,
>>> > HP-UX, AIX, etc) can test it, I'd be happy to add those systems to the
>>> > patch so it can fix for multiple systems at once.
>>>
>>> I can confirm that the issue shown up by your new testcase is also
>>> present in AIX 5.3.
>
>>> Adding -lcompat/regex to COMPAT_CFLAGS as per your Darwin hunk
>>> provokes lots of warnings:
>>
>> I think your problem is a lowercase ell instead of an uppercase i ;-)
>
> Doh. I think my problem is this font! And having not used a language
> with a separate linker since last century.
>
> I guess I picked a really bad patch to try and pick up and test direct
> from gmail. No reflection on your patch, just my workflow.
>
> Having carefully checked the content of the testcase too, I can now
> say this does fix the issue without extra warnings or testcase
> failures on AIX 5.3, so for what it's worth:
>
> Tested-by: Mike Ralphson <mike@abacus.co.uk>

Junio, sorry, I should have made this clear, but as above in the
thread, Johannes Sixt's 'patchlet' is required to be squashed into
3632cfc24, and I think Arjen was going to re-roll the patch.

I can submit the required follow-up, but it really should have J6's S-o-b.

Mike

[PATCH] Use compatibility regex library also on AIX

[Johannes Sixt]

This augments 3632cfc24 (Use compatibility regex library on Darwin,
2008-09-07), which already carries a "Tested-by" statement for AIX,
but that test was actually done with this patch included.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
Tested-by: Mike Ralphson <mike@abacus.co.uk>
---
Mike Ralphson schrieb:
> 2008/9/10 Mike Ralphson <mike.ralphson@gmail.com>:
> Junio, sorry, I should have made this clear, but as above in the
> thread, Johannes Sixt's 'patchlet' is required to be squashed into
> 3632cfc24, and I think Arjen was going to re-roll the patch.
> 
> I can submit the required follow-up, but it really should have J6's S-o-b.

Here it is. Disclaimer: This patch submission was hand-crafted. ;)

-- Hannes

diff --git a/Makefile b/Makefile
index 98d67f1..0637419 100644
--- a/Makefile
+++ b/Makefile
@@ -702,6 +702,8 @@ ifeq ($(uname_S),AIX)
 	INTERNAL_QSORT = UnfortunatelyYes
 	NEEDS_LIBICONV=YesPlease
 	BASIC_CFLAGS += -D_LARGE_FILES
+	COMPAT_CFLAGS += -Icompat/regex
+	COMPAT_OBJS += compat/regex/regex.o
 endif
 ifeq ($(uname_S),GNU)
 	# GNU/Hurd

Re: [PATCH] Use compatibility regex library also on AIX

[Arjen Laarhoven]

On Thu, Sep 11, 2008 at 10:14:44AM +0200, Johannes Sixt wrote:
> This augments 3632cfc24 (Use compatibility regex library on Darwin,
> 2008-09-07), which already carries a "Tested-by" statement for AIX,
> but that test was actually done with this patch included.

I hadn't realized it had already gone in to 'pu'.  I've put the AIX part
into my local patch, and also had it tested on HP-UX.  Today I'll
probably can test it on Solaris too, and add that as well.

Arjen

-- 
Arjen Laarhoven

The presence of those seeking the truth is infinitely to be preferred to 
those who think they've found it.
                                -- Terry Pratchett, "Monstrous Regiment"

Re: [PATCH] Use compatibility regex library also on AIX

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> This augments 3632cfc24 (Use compatibility regex library on Darwin,
> 2008-09-07), which already carries a "Tested-by" statement for AIX,
> but that test was actually done with this patch included.
>
> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
> Tested-by: Mike Ralphson <mike@abacus.co.uk>
> ---
> Mike Ralphson schrieb:
>> 2008/9/10 Mike Ralphson <mike.ralphson@gmail.com>:
>> Junio, sorry, I should have made this clear, but as above in the
>> thread, Johannes Sixt's 'patchlet' is required to be squashed into
>> 3632cfc24, and I think Arjen was going to re-roll the patch.
>> 
>> I can submit the required follow-up, but it really should have J6's S-o-b.
>
> Here it is. Disclaimer: This patch submission was hand-crafted. ;)

Sorry, my fault --- I should have noticed the missing "ifeq AIX"
anywhere in the hunk headers.

Thanks.

Re: [PATCH] Use compatibility regex library also on AIX

[Mike Ralphson]

2008/9/11 Arjen Laarhoven <arjen@yaph.org>:
> On Thu, Sep 11, 2008 at 10:14:44AM +0200, Johannes Sixt wrote:
>> This augments 3632cfc24 (Use compatibility regex library on Darwin,
>> 2008-09-07), which already carries a "Tested-by" statement for AIX,
>> but that test was actually done with this patch included.
>
> I hadn't realized it had already gone in to 'pu'.  I've put the AIX part
> into my local patch, and also had it tested on HP-UX.  Today I'll
> probably can test it on Solaris too, and add that as well.

I think the reason it's in pu is that it's in maint, master and next too...

All my test runs went bang this morning. 8-)

2008/9/11 Junio C Hamano <gitster@pobox.com>:
> Sorry, my fault --- I should have noticed the missing "ifeq AIX"
> anywhere in the hunk headers.

No, I shouldn't have added the Tested-by without making it clear I was
testing the tip of the thread, not the original patch. My bad.

Mike

Re: [PATCH] Use compatibility regex library also on AIX

[Jeff King]

On Thu, Sep 11, 2008 at 09:31:25AM +0100, Mike Ralphson wrote:

> I think the reason it's in pu is that it's in maint, master and next too...
> 
> All my test runs went bang this morning. 8-)

Mine too. :)

This is needed for FreeBSD, as well. No idea about OpenBSD or others.
Should probably be squashed with the AIX patch if it's not too late.

-- >8 --
Use compatibility regex library also on FreeBSD

Commit 3632cfc24 makes the same change for Darwin; however, the problem
also exists on FreeBSD.

Signed-off-by: Jeff King <peff@peff.net>
---

diff --git a/Makefile b/Makefile
index 247cd2d..9b1bd7b 100644
--- a/Makefile
+++ b/Makefile
@@ -688,6 +688,8 @@ ifeq ($(uname_S),FreeBSD)
 	BASIC_LDFLAGS += -L/usr/local/lib
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	COMPAT_CFLAGS += -Icompat/regex
+	COMPAT_OBJS += compat/regex/regex.o
 endif
 ifeq ($(uname_S),OpenBSD)
 	NO_STRCASESTR = YesPlease

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Brandon Casey]

Arjen Laarhoven wrote:
> The standard libc regex library on OSX does not support alternation
> in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
> functionality on OSX.
> 
> To fix this, we use the GNU regex library which is already present in
> the compat/ diretory for the MinGW port.  However, simply adding compat/
> to the COMPAT_CFLAGS variable causes a conflict between the system
> fnmatch.h and the one present in compat/.  To remedy this, move the
> regex and fnmatch functionality to their own subdirectories in compat/
> so they can be included seperately.


I wonder if this is the right fix? Right now the GNU regex library is
necessary for Darwin, FreeBSD and AIX. I can add IRIX6.5 and Solaris 7
to that list. Have newer Solaris's been tested yet? (Jeff?) I wonder if
the new test which triggers this flaw has been tested on the other
non-GNU platforms in the Makefile which have not been updated. Boyd
Lynn Gerber and his 12 platforms comes to mind.

It seems POSIX only mentions alternation under Extended Regular Expressions.
Likewise for the vertical-line character '|'.

http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_04_07

A look at compat/regex/regex.c: line 4723 shows that the default mode is
RE_SYNTAX_POSIX_BASIC.

>From the description in regex.h this mode includes "...bits common to both
basic and extended POSIX regex syntax". It seems this mode allows backslashed
versions of the extended regular expression operators ?, +, and |.

Other platforms which adhere more strictly to the POSIX spec do not interpret
the backslashed Ext-RE operators in Basic Regular Expression mode. Similar
to GNU RE_SYNTAX_POSIX_MINIMAL_BASIC.

If I'm interpreting things correctly, then all non-GNU platforms may need the
compat regex library.

On a related note: Is there any reason why extended regular expressions
were not used by default? Wouldn't they have looked prettier (fewer backslashes)?

It's too late to change diff.*.funcname now, but an alternative fix which would
probably not require every other platform to use GNU regex, is to introduce a
new funcname option which would allow extended regular expression syntax and to
convert the internal regular expressions to that format.

-brandon

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> It's too late to change diff.*.funcname now, but an alternative fix
> which would probably not require every other platform to use GNU regex,
> is to introduce a new funcname option which would allow extended regular
> expression syntax and to convert the internal regular expressions to
> that format.

That's a very sensible approach, I would agree.

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Jeff King]

On Tue, Sep 16, 2008 at 12:49:20PM -0500, Brandon Casey wrote:

> I wonder if this is the right fix? Right now the GNU regex library is
> necessary for Darwin, FreeBSD and AIX. I can add IRIX6.5 and Solaris 7
> to that list. Have newer Solaris's been tested yet? (Jeff?) I wonder if

I haven't tested; getting tests running under Solaris is on my long-term
todo. I'll try to look later today or tomorrow to see how this
particular one behaves.

-Peff

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Boyd Lynn Gerber]

Hello,

On Tue, 16 Sep 2008, Brandon Casey wrote:
> Arjen Laarhoven wrote:
>> The standard libc regex library on OSX does not support alternation
>> in POSIX Basic Regular Expression mode.  This breaks the diff.funcname
>> functionality on OSX.
>>
>> To fix this, we use the GNU regex library which is already present in
>> the compat/ diretory for the MinGW port.  However, simply adding compat/
>> to the COMPAT_CFLAGS variable causes a conflict between the system
>> fnmatch.h and the one present in compat/.  To remedy this, move the
>> regex and fnmatch functionality to their own subdirectories in compat/
>> so they can be included seperately.
>
> I wonder if this is the right fix? Right now the GNU regex library is
> necessary for Darwin, FreeBSD and AIX. I can add IRIX6.5 and Solaris 7
> to that list. Have newer Solaris's been tested yet? (Jeff?) I wonder if
> the new test which triggers this flaw has been tested on the other
> non-GNU platforms in the Makefile which have not been updated. Boyd
> Lynn Gerber and his 12 platforms comes to mind.
>
> It seems POSIX only mentions alternation under Extended Regular Expressions.
> Likewise for the vertical-line character '|'.

Someone forwarded me this email and asked if I had tested it.  I have not. 
Where is the easiest place to get it to test with the various platforms? 
I will check it out on 3-5 of them.  I now have 1.6.0.2 on 5 platforms 
running and working.  I have not had the time to get it on the other's 
yet.

--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Jeff King]

On Tue, Sep 16, 2008 at 05:25:46PM -0600, Boyd Lynn Gerber wrote:

>> I wonder if this is the right fix? Right now the GNU regex library is
>> necessary for Darwin, FreeBSD and AIX. I can add IRIX6.5 and Solaris 7
>> to that list. Have newer Solaris's been tested yet? (Jeff?) I wonder if
>> the new test which triggers this flaw has been tested on the other
>> non-GNU platforms in the Makefile which have not been updated. Boyd
>> Lynn Gerber and his 12 platforms comes to mind.
>
> Someone forwarded me this email and asked if I had tested it.  I have not. 
> Where is the easiest place to get it to test with the various platforms?  
> I will check it out on 3-5 of them.  I now have 1.6.0.2 on 5 platforms  
> running and working.  I have not had the time to get it on the other's  
> yet.

The problematic test is in 1.6.0.2; if you can run t4018-diff-funcname
successfully, then I believe you are not affected.

-Peff

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Boyd Lynn Gerber]

On Tue, 16 Sep 2008, Jeff King wrote:
> On Tue, Sep 16, 2008 at 05:25:46PM -0600, Boyd Lynn Gerber wrote:
>>> I wonder if this is the right fix? Right now the GNU regex library is
>>> necessary for Darwin, FreeBSD and AIX. I can add IRIX6.5 and Solaris 7
>>> to that list. Have newer Solaris's been tested yet? (Jeff?) I wonder if
>>> the new test which triggers this flaw has been tested on the other
>>> non-GNU platforms in the Makefile which have not been updated. Boyd
>>> Lynn Gerber and his 12 platforms comes to mind.
>>
>> Someone forwarded me this email and asked if I had tested it.  I have not.
>> Where is the easiest place to get it to test with the various platforms?
>> I will check it out on 3-5 of them.  I now have 1.6.0.2 on 5 platforms
>> running and working.  I have not had the time to get it on the other's
>> yet.
>
> The problematic test is in 1.6.0.2; if you can run t4018-diff-funcname
> successfully, then I believe you are not affected.

When I do a gmake test these 4 platforms all fail only these 2 tests.

* FAIL 10: reinit


                 (
                         unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG

                         mkdir again &&
                         cd again &&
                         git init >out1 2>err1 &&
                         git init >out2 2>err2
                 ) &&
                 grep "Initialized empty" again/out1 &&
                 grep "Reinitialized existing" again/out2 &&
                 >again/empty &&
                 test_cmp again/empty again/err1 &&
                 test_cmp again/empty again/err2

* FAIL 11: init with --template

                 mkdir template-source &&
                 echo content >template-source/file &&
                 (
                         mkdir template-custom &&
                         cd template-custom &&
                         git init --template=../template-source
                 ) &&
                 test_cmp template-source/file template-custom/.git/file

*   ok 12: init with --template (blank)
* failed 2 among 12 test(s)
gmake[1]: *** [t0001-init.sh] Error 1
gmake[1]: Leaving directory `/home/zenez/build/osr6/git-1.6.0.2/t'
gmake: *** [test] Error 2

I have not had time to look into the failures.  How many tests should I 
see and pass.  The first 40 all pass.  Then 2 of 12 fail as above.

--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Jeff King]

On Tue, Sep 16, 2008 at 05:42:11PM -0600, Boyd Lynn Gerber wrote:

> When I do a gmake test these 4 platforms all fail only these 2 tests.
>
> * FAIL 10: reinit
> [...]
> * FAIL 11: init with --template
> [...]
> gmake[1]: *** [t0001-init.sh] Error 1
>
> I have not had time to look into the failures.  How many tests should I  
> see and pass.  The first 40 all pass.  Then 2 of 12 fail as above.

These failures are unrelated to the described problem, and prevent the
system from continuing on to run other tests.

So if you get a chance, please:

  1. Run t0001 in verbose mode and report the results so we can get a
     better idea of what's failing:

       gmake test GIT_TEST_OPTS=--verbose

  2. Run t4018 individually and report on the results:

       cd t && gmake t4018-diff-funcname.sh

-Peff

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Boyd Lynn Gerber]

On Tue, 16 Sep 2008, Jeff King wrote:
> On Tue, Sep 16, 2008 at 05:42:11PM -0600, Boyd Lynn Gerber wrote:
>> When I do a gmake test these 4 platforms all fail only these 2 tests.
>> * FAIL 10: reinit
>> [...]
>> * FAIL 11: init with --template
>> [...]
>> gmake[1]: *** [t0001-init.sh] Error 1
>>
>  1. Run t0001 in verbose mode and report the results so we can get a
>     better idea of what's failing:
>
>       gmake test GIT_TEST_OPTS=--verbose


* expecting success:

         (
                 unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG

                 mkdir again &&
                 cd again &&
                 git init >out1 2>err1 &&
                 git init >out2 2>err2
         ) &&
         grep "Initialized empty" again/out1 &&
         grep "Reinitialized existing" again/out2 &&
         >again/empty &&
         test_cmp again/empty again/err1 &&
         test_cmp again/empty again/err2

Initialized empty Git repository in 
/home/zenez/build/osr6/git-1.6.0.2/t/trash directory/again/.git/
Reinitialized existing Git repository in 
/home/zenez/build/osr6/git-1.6.0.2/t/trash directory/again/.git/
diff: ERROR: Illegal option -- u
Usage: diff [ -bcefhrC<n> ] file1 file2
* FAIL 10: reinit
                 (
                         unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG

                         mkdir again &&
                         cd again &&
                         git init >out1 2>err1 &&
                         git init >out2 2>err2
                 ) &&
                 grep "Initialized empty" again/out1 &&
                 grep "Reinitialized existing" again/out2 &&
                 >again/empty &&
                 test_cmp again/empty again/err1 &&
                 test_cmp again/empty again/err2

* expecting success:
         mkdir template-source &&
         echo content >template-source/file &&
         (
                 mkdir template-custom &&
                 cd template-custom &&
                 git init --template=../template-source
         ) &&
         test_cmp template-source/file template-custom/.git/file

Initialized empty Git repository in 
/home/zenez/build/osr6/git-1.6.0.2/t/trash directory/template-custom/.git/
diff: ERROR: Illegal option -- u
Usage: diff [ -bcefhrC<n> ] file1 file2
* FAIL 11: init with --template

                 mkdir template-source &&
                 echo content >template-source/file &&
                 (
                         mkdir template-custom &&
                         cd template-custom &&
                         git init --template=../template-source
                 ) &&
                 test_cmp template-source/file template-custom/.git/file

* expecting success:
         (
                 mkdir template-plain &&
                 cd template-plain &&
                 git init
         ) &&
         test -f template-plain/.git/info/exclude &&
         (
                 mkdir template-blank &&
                 cd template-blank &&
                 git init --template=
         ) &&
         ! test -f template-blank/.git/info/exclude

Initialized empty Git repository in 
/home/zenez/build/osr6/git-1.6.0.2/t/trash directory/template-plain/.git/
Initialized empty Git repository in 
/home/zenez/build/osr6/git-1.6.0.2/t/trash directory/template-blank/.git/

>  2. Run t4018 individually and report on the results:
>
>       cd t && gmake t4018-diff-funcname.sh


3 systems all give me this.

$ cd t && gmake t4018-diff-funcname.sh
*** t4018-diff-funcname.sh ***
t4018-diff-funcname.sh: syntax error at line 52: `(' unexpected
gmake: *** [t4018-diff-funcname.sh] Error 2


--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Brandon Casey]

Boyd Lynn Gerber wrote:

> * expecting success:
> 
>         (
>                 unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG
> 
>                 mkdir again &&
>                 cd again &&
>                 git init >out1 2>err1 &&
>                 git init >out2 2>err2
>         ) &&
>         grep "Initialized empty" again/out1 &&
>         grep "Reinitialized existing" again/out2 &&
>         >again/empty &&
>         test_cmp again/empty again/err1 &&
>         test_cmp again/empty again/err2
> 
> Initialized empty Git repository in
> /home/zenez/build/osr6/git-1.6.0.2/t/trash directory/again/.git/
> Reinitialized existing Git repository in
> /home/zenez/build/osr6/git-1.6.0.2/t/trash directory/again/.git/
> diff: ERROR: Illegal option -- u
> Usage: diff [ -bcefhrC<n> ] file1 file2

The tests are failing at the test_cmp line. By default, test_cmp is
set to 'diff -u'. Perhaps you usually set test_cmp to something else
like 'cmp -s' or modify your path so that a more modern diff is used?

-brandon

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Boyd Lynn Gerber]

On Tue, 16 Sep 2008, Jeff King wrote:
> On Tue, Sep 16, 2008 at 05:42:11PM -0600, Boyd Lynn Gerber wrote:
>> When I do a gmake test these 4 platforms all fail only these 2 tests.
>>
>> * FAIL 10: reinit
>> [...]
>> * FAIL 11: init with --template
>> [...]
>> gmake[1]: *** [t0001-init.sh] Error 1
>>
>> I have not had time to look into the failures.  How many tests should I
>> see and pass.  The first 40 all pass.  Then 2 of 12 fail as above.
>
> These failures are unrelated to the described problem, and prevent the
> system from continuing on to run other tests.
>
> So if you get a chance, please:
>
>  1. Run t0001 in verbose mode and report the results so we can get a
>     better idea of what's failing:
>
>       gmake test GIT_TEST_OPTS=--verbose

If I use gdiff or make gnu diff and put it in /usr/local/bin/ and change 
the path to have /usr/local/bin first it does not fail/.

I have a /usr/gnu/bin/... where I have gdiff linked to diff.

# /usr/gnu/bin/diff --version
diff (GNU diffutils) 2.8.1
Copyright (C) 2002 Free Software Foundation, Inc.

This program comes with NO WARRANTY, to the extent permitted by law.
You may redistribute copies of this program
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.

Written by Paul Eggert, Mike Haertel, David Hayes,
Richard Stallman, and Len Tower.


--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Brandon Casey]

Boyd Lynn Gerber wrote:

> If I use gdiff or make gnu diff and put it in /usr/local/bin/ and change
> the path to have /usr/local/bin first it does not fail/.
> 
> I have a /usr/gnu/bin/... where I have gdiff linked to diff.

You must have done something like this in the past when you were testing
previous versions since the test_cmp variable has not changed.

So, for future reference either have a diff that understands '-u' in your
path, or set the test_cmp environment variable.

-brandon

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Boyd Lynn Gerber]

On Tue, 16 Sep 2008, Brandon Casey wrote:
> Boyd Lynn Gerber wrote:
>> If I use gdiff or make gnu diff and put it in /usr/local/bin/ and change
>> the path to have /usr/local/bin first it does not fail/.
>>
>> I have a /usr/gnu/bin/... where I have gdiff linked to diff.
>
> You must have done something like this in the past when you were testing 
> previous versions since the test_cmp variable has not changed.
>
> So, for future reference either have a diff that understands '-u' in 
> your path, or set the test_cmp environment variable.

I had customized things so that any GNU util used gutil_name.  This is 
the easiest way for me to distinguish between Native utils and GNU utils.

Is there an easy way to have git use gcommand_name instead of 
command_name?  I changed the path to run the git test, but not all my 
clients will allow be to have the gnu named command instead of the native 
command.  There programs require the non GNU functionality.  That is why I 
create a /usr/gnu/  with all the GNU stuff available for my use, but 
having to change paths back and forth just to run git is a pain when 
build/trouble shooting customer  aplications.

Thanks,

--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Brandon Casey]

Boyd Lynn Gerber wrote:
> On Tue, 16 Sep 2008, Brandon Casey wrote:
>> Boyd Lynn Gerber wrote:
>>> If I use gdiff or make gnu diff and put it in /usr/local/bin/ and change
>>> the path to have /usr/local/bin first it does not fail/.
>>>
>>> I have a /usr/gnu/bin/... where I have gdiff linked to diff.
>>
>> You must have done something like this in the past when you were
>> testing previous versions since the test_cmp variable has not changed.
>>
>> So, for future reference either have a diff that understands '-u' in
>> your path, or set the test_cmp environment variable.
> 
> I had customized things so that any GNU util used gutil_name.  This is
> the easiest way for me to distinguish between Native utils and GNU utils.
> 
> Is there an easy way to have git use gcommand_name instead of
> command_name?

Not that I know of.

>  I changed the path to run the git test, but not all my
> clients will allow be to have the gnu named command instead of the
> native command.  There programs require the non GNU functionality.  That
> is why I create a /usr/gnu/  with all the GNU stuff available for my
> use, but having to change paths back and forth just to run git is a pain
> when build/trouble shooting customer  aplications.

I usually create a little compile script which sets the environment
variables the way I want, and calls make or gmake appropriately.

For the test suite you can set (if necessary):
  test_cmp
  TAR

'git grep' sometimes calls native grep. You can disallow this by setting
the Makefile variable NO_EXTERNAL_GREP.

And of course SHELL_PATH can be used to set the path to the shell that
all shell code should be executed with.

-brandon

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Boyd Lynn Gerber]

On Tue, 16 Sep 2008, Jeff King wrote:
> On Tue, Sep 16, 2008 at 05:42:11PM -0600, Boyd Lynn Gerber wrote:
>> When I do a gmake test these 4 platforms all fail only these 2 tests.
>>
>> * FAIL 10: reinit
>> [...]
>> * FAIL 11: init with --template
>> [...]
>> gmake[1]: *** [t0001-init.sh] Error 1
>>
>> I have not had time to look into the failures.  How many tests should I
>> see and pass.  The first 40 all pass.  Then 2 of 12 fail as above.
>
> These failures are unrelated to the described problem, and prevent the
> system from continuing on to run other tests.
>
> So if you get a chance, please:
>
>       gmake test GIT_TEST_OPTS=--verbose

So with path set so my /usr/gnu/bin/ is before any others this is what I 
get as the next failure.  After a long run...

* FAIL 5: alternation in pattern

                 git config diff.java.funcname "^[ 
]*\(\(public\|static\).*\)$"
                 git diff --no-index Beer.java Beer-correct.java |
                 grep "^@@.*@@ public static void main("


* failed 1 among 5 test(s)
gmake[1]: *** [t4018-diff-funcname.sh] Error 1
gmake[1]: Leaving directory `/home/zenez/build/osr6/git-1.6.0.2/t'
gmake: *** [test] Error 2


--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Brandon Casey]

Boyd Lynn Gerber wrote:

> So with path set so my /usr/gnu/bin/ is before any others this is what I
> get as the next failure.  After a long run...
> 
> * FAIL 5: alternation in pattern
> 
>                 git config diff.java.funcname "^[
> ]*\(\(public\|static\).*\)$"
>                 git diff --no-index Beer.java Beer-correct.java |
>                 grep "^@@.*@@ public static void main("
> 
> 
> * failed 1 among 5 test(s)
> gmake[1]: *** [t4018-diff-funcname.sh] Error 1
> gmake[1]: Leaving directory `/home/zenez/build/osr6/git-1.6.0.2/t'
> gmake: *** [test] Error 2

That's the one, thanks.

And just so you know, this isn't a regression. It's just a new test that
was designed to reveal a shortcoming that no one knew existed before Arjen
discovered it.

-brandon

Re: [RFC/PATCH] Use compatibility regex library for OSX/Darwin

[Boyd Lynn Gerber]

On Tue, 16 Sep 2008, Brandon Casey wrote:
> Boyd Lynn Gerber wrote:
>> So with path set so my /usr/gnu/bin/ is before any others this is what I
>> get as the next failure.  After a long run...
>>
>> * FAIL 5: alternation in pattern
>>
>>                 git config diff.java.funcname "^[
>> ]*\(\(public\|static\).*\)$"
>>                 git diff --no-index Beer.java Beer-correct.java |
>>                 grep "^@@.*@@ public static void main("
>>
>>
>> * failed 1 among 5 test(s)
>> gmake[1]: *** [t4018-diff-funcname.sh] Error 1
>> gmake[1]: Leaving directory `/home/zenez/build/osr6/git-1.6.0.2/t'
>> gmake: *** [test] Error 2
>
> That's the one, thanks.
>
> And just so you know, this isn't a regression. It's just a new test that
> was designed to reveal a shortcoming that no one knew existed before Arjen
> discovered it.

Six of the 12 all have the same problem.  I would say all 12 have it as my 
experience has shown that if these have it then all have it.  The fixes I 
did were so generic that once I got these first 6 working the others just 
worked.  So I would say they all need the patch or fix.  Sorry I did not 
follow the whole thread.  I read it from the archive what I could.  It 
seems the archive is about 2 days behind.

Good Luck,

--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

[PATCH 1/4] diff.c: return pattern entry pointer rather than just the hunk header pattern

[Brandon Casey]

This is in preparation for associating a flag with each pattern which will
control how the pattern is interpreted. For example, as a basic or extended
regular expression.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Sorry for the dup, I forgot to add the list.


Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> It's too late to change diff.*.funcname now, but an alternative fix
>> which would probably not require every other platform to use GNU regex,
>> is to introduce a new funcname option which would allow extended regular
>> expression syntax and to convert the internal regular expressions to
>> that format.
> 
> That's a very sensible approach, I would agree.


 diff.c |   53 ++++++++++++++++++++++++++++++-----------------------
 1 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/diff.c b/diff.c
index 998dcaa..e040088 100644
--- a/diff.c
+++ b/diff.c
@@ -94,6 +94,8 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val
  * 'diff.<what>.funcname' attribute can be specified in the configuration
  * to define a customized regexp to find the beginning of a function to
  * be used for hunk header lines of "diff -p" style output.
+ * Note: If this structure is modified, it must retain the ability to be cast
+ * to a struct funcname_pattern_entry, defined elsewhere.
  */
 static struct funcname_pattern {
 	char *name;
@@ -1382,17 +1384,11 @@ int diff_filespec_is_binary(struct diff_filespec *one)
 	return one->is_binary;
 }
 
-static const char *funcname_pattern(const char *ident)
-{
-	struct funcname_pattern *pp;
-
-	for (pp = funcname_pattern_list; pp; pp = pp->next)
-		if (!strcmp(ident, pp->name))
-			return pp->pattern;
-	return NULL;
-}
-
-static struct builtin_funcname_pattern {
+/*
+ * Note: The elements of this structure must be arranged so that they are
+ *       compatible with the elements of the funcname_pattern structure.
+ */
+static struct funcname_pattern_entry {
 	const char *name;
 	const char *pattern;
 } builtin_funcname_pattern[] = {
@@ -1415,9 +1411,20 @@ static struct builtin_funcname_pattern {
 	{ "tex", "^\\(\\\\\\(\\(sub\\)*section\\|chapter\\|part\\)\\*\\{0,1\\}{.*\\)$" },
 };
 
-static const char *diff_funcname_pattern(struct diff_filespec *one)
+static const struct funcname_pattern_entry *funcname_pattern(const char *ident)
+{
+	struct funcname_pattern *pp;
+
+	for (pp = funcname_pattern_list; pp; pp = pp->next)
+		if (!strcmp(ident, pp->name))
+			return (struct funcname_pattern_entry*) pp;
+	return NULL;
+}
+
+static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_filespec *one)
 {
-	const char *ident, *pattern;
+	const char *ident;
+	const struct funcname_pattern_entry *pe;
 	int i;
 
 	diff_filespec_check_attr(one);
@@ -1432,9 +1439,9 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)
 		return funcname_pattern("default");
 
 	/* Look up custom "funcname.$ident" regexp from config. */
-	pattern = funcname_pattern(ident);
-	if (pattern)
-		return pattern;
+	pe = funcname_pattern(ident);
+	if (pe)
+		return pe;
 
 	/*
 	 * And define built-in fallback patterns here.  Note that
@@ -1442,7 +1449,7 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)
 	 */
 	for (i = 0; i < ARRAY_SIZE(builtin_funcname_pattern); i++)
 		if (!strcmp(ident, builtin_funcname_pattern[i].name))
-			return builtin_funcname_pattern[i].pattern;
+			return &builtin_funcname_pattern[i];
 
 	return NULL;
 }
@@ -1520,11 +1527,11 @@ static void builtin_diff(const char *name_a,
 		xdemitconf_t xecfg;
 		xdemitcb_t ecb;
 		struct emit_callback ecbdata;
-		const char *funcname_pattern;
+		const struct funcname_pattern_entry *pe;
 
-		funcname_pattern = diff_funcname_pattern(one);
-		if (!funcname_pattern)
-			funcname_pattern = diff_funcname_pattern(two);
+		pe = diff_funcname_pattern(one);
+		if (!pe)
+			pe = diff_funcname_pattern(two);
 
 		memset(&xecfg, 0, sizeof(xecfg));
 		memset(&ecbdata, 0, sizeof(ecbdata));
@@ -1536,8 +1543,8 @@ static void builtin_diff(const char *name_a,
 		xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
 		xecfg.ctxlen = o->context;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
-		if (funcname_pattern)
-			xdiff_set_find_func(&xecfg, funcname_pattern);
+		if (pe)
+			xdiff_set_find_func(&xecfg, pe->pattern);
 		if (!diffopts)
 			;
 		else if (!prefixcmp(diffopts, "--unified="))
-- 
1.6.0.1.244.gdc19

[PATCH 2/4] diff.c: associate a flag with each pattern and use it for compiling regex

[Brandon Casey]

This is in preparation for allowing extended regular expression patterns.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 diff.c            |   28 ++++++++++++++++------------
 xdiff-interface.c |    4 ++--
 xdiff-interface.h |    2 +-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/diff.c b/diff.c
index e040088..4363d0d 100644
--- a/diff.c
+++ b/diff.c
@@ -100,10 +100,11 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val
 static struct funcname_pattern {
 	char *name;
 	char *pattern;
+	int cflags;
 	struct funcname_pattern *next;
 } *funcname_pattern_list;
 
-static int parse_funcname_pattern(const char *var, const char *ep, const char *value)
+static int parse_funcname_pattern(const char *var, const char *ep, const char *value, int cflags)
 {
 	const char *name;
 	int namelen;
@@ -123,6 +124,7 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v
 	}
 	free(pp->pattern);
 	pp->pattern = xstrdup(value);
+	pp->cflags = cflags;
 	return 0;
 }
 
@@ -191,7 +193,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 			if (!strcmp(ep, ".funcname")) {
 				if (!value)
 					return config_error_nonbool(var);
-				return parse_funcname_pattern(var, ep, value);
+				return parse_funcname_pattern(var, ep, value,
+					0);
 			}
 		}
 	}
@@ -1391,24 +1394,25 @@ int diff_filespec_is_binary(struct diff_filespec *one)
 static struct funcname_pattern_entry {
 	const char *name;
 	const char *pattern;
+	const int cflags;
 } builtin_funcname_pattern[] = {
-	{ "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$" },
-	{ "html", "^\\s*\\(<[Hh][1-6]\\s.*>.*\\)$" },
+	{ "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$", 0 },
+	{ "html", "^\\s*\\(<[Hh][1-6]\\s.*>.*\\)$", 0 },
 	{ "java", "!^[ 	]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
 			"new\\|return\\|switch\\|throw\\|while\\)\n"
 			"^[ 	]*\\(\\([ 	]*"
 			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
-			"[ 	]*([^;]*\\)$" },
+			"[ 	]*([^;]*\\)$", 0 },
 	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
 			"destructor\\|interface\\|implementation\\|"
 			"initialization\\|finalization\\)[ \t]*.*\\)$"
 			"\\|"
-			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"
-			},
-	{ "php", "^[\t ]*\\(\\(function\\|class\\).*\\)" },
-	{ "python", "^\\s*\\(\\(class\\|def\\)\\s.*\\)$" },
-	{ "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$" },
-	{ "tex", "^\\(\\\\\\(\\(sub\\)*section\\|chapter\\|part\\)\\*\\{0,1\\}{.*\\)$" },
+			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$",
+			0 },
+	{ "php", "^[\t ]*\\(\\(function\\|class\\).*\\)", 0 },
+	{ "python", "^\\s*\\(\\(class\\|def\\)\\s.*\\)$", 0 },
+	{ "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$", 0 },
+	{ "tex", "^\\(\\\\\\(\\(sub\\)*section\\|chapter\\|part\\)\\*\\{0,1\\}{.*\\)$", 0 },
 };
 
 static const struct funcname_pattern_entry *funcname_pattern(const char *ident)
@@ -1544,7 +1548,7 @@ static void builtin_diff(const char *name_a,
 		xecfg.ctxlen = o->context;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		if (pe)
-			xdiff_set_find_func(&xecfg, pe->pattern);
+			xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
 		if (!diffopts)
 			;
 		else if (!prefixcmp(diffopts, "--unified="))
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 944ad98..7f1a7d3 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -218,7 +218,7 @@ static long ff_regexp(const char *line, long len,
 	return result;
 }
 
-void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)
+void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags)
 {
 	int i;
 	struct ff_regs *regs;
@@ -243,7 +243,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)
 			expression = buffer = xstrndup(value, ep - value);
 		else
 			expression = value;
-		if (regcomp(&reg->re, expression, 0))
+		if (regcomp(&reg->re, expression, cflags))
 			die("Invalid regexp to look for hunk header: %s", expression);
 		free(buffer);
 		value = ep + 1;
diff --git a/xdiff-interface.h b/xdiff-interface.h
index 558492b..23c49b9 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -16,6 +16,6 @@ int parse_hunk_header(char *line, int len,
 int read_mmfile(mmfile_t *ptr, const char *filename);
 int buffer_is_binary(const char *ptr, unsigned long size);
 
-extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line);
+extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
 
 #endif
-- 
1.6.0.1.244.gdc19

[PATCH 3/4] diff.*.xfuncname which uses "extended" regex's for hunk header selection

[Brandon Casey]

Currently, the hunk headers produced by 'diff -p' are customizable by
setting the diff.*.funcname option in the config file. The 'funcname' option
takes a basic regular expression. This functionality was designed using the
GNU regex library which, by default, allows using backslashed versions of
some extended regular expression operators, even in Basic Regular Expression
mode. For example, the following characters, when backslashed, are
interpreted according to the extended regular expression rules: ?, +, and |.
As such, the builtin funcname patterns were created using some extended
regular expression operators.

Other platforms which adhere more strictly to the POSIX spec do not
interpret the backslashed extended RE operators in Basic Regular Expression
mode. This causes the pattern matching for the builtin funcname patterns to
fail on those platforms.

Introduce a new option 'xfuncname' which uses extended regular expressions,
and advertise it _instead_ of funcname. Since most users are on GNU
platforms, the majority of funcname patterns are created and tested there.
Advertising only xfuncname should help to avoid the creation of non-portable
patterns which work with GNU regex but not elsewhere.

Additionally, the extended regular expressions may be less ugly and
complicated compared to the basic RE since many common special operators do
not need to be backslashed.

For example, the GNU Basic RE:

    ^[ 	]*\\(\\(public\\|static\\).*\\)$

becomes the following Extended RE:

    ^[ 	]*((public|static).*)$

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 Documentation/gitattributes.txt |    4 ++--
 diff.c                          |    5 +++++
 t/t4018-diff-funcname.sh        |    2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 6f3551d..fa04eca 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -288,13 +288,13 @@ for paths.
 *.tex	diff=tex
 ------------------------
 
-Then, you would define "diff.tex.funcname" configuration to
+Then, you would define "diff.tex.xfuncname" configuration to
 specify a regular expression that matches a line that you would
 want to appear as the hunk header, like this:
 
 ------------------------
 [diff "tex"]
-	funcname = "^\\(\\\\\\(sub\\)*section{.*\\)$"
+	xfuncname = "^(\\\\(sub)*section{.*)$"
 ------------------------
 
 Note.  A single level of backslashes are eaten by the
diff --git a/diff.c b/diff.c
index 4363d0d..ad5e551 100644
--- a/diff.c
+++ b/diff.c
@@ -195,6 +195,11 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 					return config_error_nonbool(var);
 				return parse_funcname_pattern(var, ep, value,
 					0);
+			} else if (!strcmp(ep, ".xfuncname")) {
+				if (!value)
+					return config_error_nonbool(var);
+				return parse_funcname_pattern(var, ep, value,
+					REG_EXTENDED);
 			}
 		}
 	}
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 18bcd97..602d68f 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -58,7 +58,7 @@ test_expect_success 'last regexp must not be negated' '
 '
 
 test_expect_success 'alternation in pattern' '
-	git config diff.java.funcname "^[ 	]*\\(\\(public\\|static\\).*\\)$"
+	git config diff.java.xfuncname "^[ 	]*((public|static).*)$" &&
 	git diff --no-index Beer.java Beer-correct.java |
 	grep "^@@.*@@ public static void main("
 '
-- 
1.6.0.1.244.gdc19

[PATCH 4/4] diff.c: convert builtin funcname patterns to extended regular expressions

[Brandon Casey]

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


This is a blind conversion removing \\ before ( and { etc.
and adding \\ before naked ( and { etc.

I hope the authors who last touched these patterns will help with testing:

bibtex: Johan Herland
  html: Johan Herland
  java: Junio Hamano, Jeff King
pascal: Avery Pennarun
   php: Andreas Ericsson
python: Kirill Smelkov
  ruby: Giuseppe Bilotta
   tex: Johan Herland

thanks,
-brandon


 diff.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/diff.c b/diff.c
index ad5e551..25d2259 100644
--- a/diff.c
+++ b/diff.c
@@ -1401,23 +1401,23 @@ static struct funcname_pattern_entry {
 	const char *pattern;
 	const int cflags;
 } builtin_funcname_pattern[] = {
-	{ "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$", 0 },
-	{ "html", "^\\s*\\(<[Hh][1-6]\\s.*>.*\\)$", 0 },
-	{ "java", "!^[ 	]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
-			"new\\|return\\|switch\\|throw\\|while\\)\n"
-			"^[ 	]*\\(\\([ 	]*"
-			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
-			"[ 	]*([^;]*\\)$", 0 },
-	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
-			"destructor\\|interface\\|implementation\\|"
-			"initialization\\|finalization\\)[ \t]*.*\\)$"
-			"\\|"
-			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$",
-			0 },
-	{ "php", "^[\t ]*\\(\\(function\\|class\\).*\\)", 0 },
-	{ "python", "^\\s*\\(\\(class\\|def\\)\\s.*\\)$", 0 },
-	{ "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$", 0 },
-	{ "tex", "^\\(\\\\\\(\\(sub\\)*section\\|chapter\\|part\\)\\*\\{0,1\\}{.*\\)$", 0 },
+	{ "bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#\\}\\{~%]*).*$", 1 },
+	{ "html", "^\\s*(<[Hh][1-6]\\s.*>.*)$", 1 },
+	{ "java", "!^[ 	]*(catch|do|for|if|instanceof|"
+			"new|return|switch|throw|while)\n"
+			"^[ 	]*(([ 	]*"
+			"[A-Za-z_][A-Za-z_0-9]*){2,}"
+			"[ 	]*\\([^;]*)$", 1 },
+	{ "pascal", "^((procedure|function|constructor|"
+			"destructor|interface|implementation|"
+			"initialization|finalization)[ \t]*.*)$"
+			"|"
+			"^(.*=[ \t]*(class|record).*)$",
+			1 },
+	{ "php", "^[\t ]*((function|class).*)", 1 },
+	{ "python", "^\\s*((class|def)\\s.*)$", 1 },
+	{ "ruby", "^\\s*((class|module|def)\\s.*)$", 1 },
+	{ "tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$", 1 },
 };
 
 static const struct funcname_pattern_entry *funcname_pattern(const char *ident)
-- 
1.6.0.1.244.gdc19

[PATCH 4/4 v2] diff.c: convert builtin funcname patterns to extended regular expressions

[Brandon Casey]

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


The original version changed the cflags parameter from '0' to '1'
rather than to the macro REG_EXTENDED as it should have been.

This version is corrected.

-brandon


 diff.c |   35 ++++++++++++++++++-----------------
 1 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/diff.c b/diff.c
index ad5e551..8018544 100644
--- a/diff.c
+++ b/diff.c
@@ -1401,23 +1401,24 @@ static struct funcname_pattern_entry {
 	const char *pattern;
 	const int cflags;
 } builtin_funcname_pattern[] = {
-	{ "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$", 0 },
-	{ "html", "^\\s*\\(<[Hh][1-6]\\s.*>.*\\)$", 0 },
-	{ "java", "!^[ 	]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
-			"new\\|return\\|switch\\|throw\\|while\\)\n"
-			"^[ 	]*\\(\\([ 	]*"
-			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
-			"[ 	]*([^;]*\\)$", 0 },
-	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
-			"destructor\\|interface\\|implementation\\|"
-			"initialization\\|finalization\\)[ \t]*.*\\)$"
-			"\\|"
-			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$",
-			0 },
-	{ "php", "^[\t ]*\\(\\(function\\|class\\).*\\)", 0 },
-	{ "python", "^\\s*\\(\\(class\\|def\\)\\s.*\\)$", 0 },
-	{ "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$", 0 },
-	{ "tex", "^\\(\\\\\\(\\(sub\\)*section\\|chapter\\|part\\)\\*\\{0,1\\}{.*\\)$", 0 },
+	{ "bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#\\}\\{~%]*).*$", REG_EXTENDED },
+	{ "html", "^\\s*(<[Hh][1-6]\\s.*>.*)$", REG_EXTENDED },
+	{ "java", "!^[ 	]*(catch|do|for|if|instanceof|"
+			"new|return|switch|throw|while)\n"
+			"^[ 	]*(([ 	]*"
+			"[A-Za-z_][A-Za-z_0-9]*){2,}"
+			"[ 	]*\\([^;]*)$", REG_EXTENDED },
+	{ "pascal", "^((procedure|function|constructor|"
+			"destructor|interface|implementation|"
+			"initialization|finalization)[ \t]*.*)$"
+			"|"
+			"^(.*=[ \t]*(class|record).*)$",
+			REG_EXTENDED },
+	{ "php", "^[\t ]*((function|class).*)", REG_EXTENDED },
+	{ "python", "^\\s*((class|def)\\s.*)$", REG_EXTENDED },
+	{ "ruby", "^\\s*((class|module|def)\\s.*)$", REG_EXTENDED },
+	{ "tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
+		REG_EXTENDED },
 };
 
 static const struct funcname_pattern_entry *funcname_pattern(const char *ident)
-- 
1.6.0.1.244.gdc19

Re: [PATCH 2/4] diff.c: associate a flag with each pattern and use it for compiling regex

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> This is in preparation for allowing extended regular expression patterns.
> ...
> @@ -100,10 +100,11 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val
>  static struct funcname_pattern {
>  	char *name;
>  	char *pattern;
> +	int cflags;

What does "C" stand for?

Re: [PATCH 2/4] diff.c: associate a flag with each pattern and use it for compiling regex

[Andreas Ericsson]

Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> This is in preparation for allowing extended regular expression patterns.
>> ...
>> @@ -100,10 +100,11 @@ static int parse_lldiff_command(const char *var, const char *ep, const char *val
>>  static struct funcname_pattern {
>>  	char *name;
>>  	char *pattern;
>> +	int cflags;
> 
> What does "C" stand for?


"compile". It's the same name as regcomp(3) uses for the flags being
used to compile the regular expression. The full mnemonic name would
be regex_compile_flag, which is a bit unwieldy. Perhaps regcomp_flags
would be a good compromise?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 2/4] diff.c: associate a flag with each pattern and use it for compiling regex

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

> Junio C Hamano wrote:
> ...
>>>  static struct funcname_pattern {
>>>  	char *name;
>>>  	char *pattern;
>>> +	int cflags;
>>
>> What does "C" stand for?
>
> "compile". It's the same name as regcomp(3) uses for the flags being
> used to compile the regular expression. The full mnemonic name would
> be regex_compile_flag, which is a bit unwieldy. Perhaps regcomp_flags
> would be a good compromise?

Ah, I see.

When I saw that new field for the first time, I didn't think it will be
used to store the bare flag values regcomp/regexec library would accept
directly (I expected we would see #define or enum to tweak our own set of
features, not limiting ourselves EXTENDED/ICASE etc. that regcomp/regexec
library supports)

IOW, it just did not click for me to look at "man 3 regcomp" which says:

    int regcomp(regex_t *preg, const char *regex, int cflags);

So unless others feel that we might get a better layering separation by
not storing REG_EXTENDED and stuff directly in that field (which was my
initial reaction without looking at 4/4 which does store REG_EXTENDED
there without our own enums), cflag is perfectly a good name here.

Thanks --- I am bit under the weather and not thinking quite straight.

Re: [PATCH 4/4] diff.c: convert builtin funcname patterns to extended regular expressions

[Andreas Ericsson]

Brandon Casey wrote:
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
> 
> 
> This is a blind conversion removing \\ before ( and { etc.
> and adding \\ before naked ( and { etc.
> 
> I hope the authors who last touched these patterns will help with testing:
> 
> bibtex: Johan Herland
>   html: Johan Herland
>   java: Junio Hamano, Jeff King
> pascal: Avery Pennarun
>    php: Andreas Ericsson
> python: Kirill Smelkov
>   ruby: Giuseppe Bilotta
>    tex: Johan Herland
> 

The PHP one seems to work just fine.

Signed-off-by: Andreas Ericsson <ae@op5.se>

Nicely done, although I'd rather have "ereg_funcname" instead of
"xfuncname", but I don't care very much for myself, as I'll
rather submit my patterns upstream than add them to .git/config ;-)

Junio:
Can we issue a deprecation heads-up for the current "funcname"
along with a "call for patterns" and then have "funcname" and
"ereg_funcname" mean the same for a while until we obsolete
ereg_funcname in favour of funcname, perhaps? I can't imagine
anyone wanting to use posix regular expressions if extended
ones are available everywhere.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 4/4] diff.c: convert builtin funcname patterns to extended regular expressions

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

> Can we issue a deprecation heads-up for the current "funcname"
> along with a "call for patterns" and then have "funcname" and
> "ereg_funcname" mean the same for a while until we obsolete
> ereg_funcname in favour of funcname, perhaps? I can't imagine
> anyone wanting to use posix regular expressions if extended
> ones are available everywhere.

I prefer not to obsolete anything, and that is one of the larger reasons
that I did not object to xfuncname at all.  It's shorter to spell than
ereg_funcname (and sweeter to the eye).

Even when in some future _everybody_ uses xfuncname and nobody you and I
know personally uses funcname anymore, I do not think it is worth the
hassle to change the semantics of "funcname".

For one thing, "xfuncname" is _not_ that ugly that people would wish they
could spell it just "funcname".

This reminds me of what Eric did to "commit" vs "dcommit".  "commit" was
renamed to "set-tree", and a command with a better semantics is called
"dcommit".  Perhaps not many people use "set-tree" and everybody keeps
typing "dcommit" these days, but it is not worth renaming it to "commit",
ever.

Re: [PATCH 2/4] diff.c: associate a flag with each pattern and use it for compiling regex

[Andreas Ericsson]

Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
>> Junio C Hamano wrote:
>> ...
>>>>  static struct funcname_pattern {
>>>>  	char *name;
>>>>  	char *pattern;
>>>> +	int cflags;
>>> What does "C" stand for?
>> "compile". It's the same name as regcomp(3) uses for the flags being
>> used to compile the regular expression. The full mnemonic name would
>> be regex_compile_flag, which is a bit unwieldy. Perhaps regcomp_flags
>> would be a good compromise?
> 
> Ah, I see.
> 
> When I saw that new field for the first time, I didn't think it will be
> used to store the bare flag values regcomp/regexec library would accept
> directly (I expected we would see #define or enum to tweak our own set of
> features, not limiting ourselves EXTENDED/ICASE etc. that regcomp/regexec
> library supports)
> 
> IOW, it just did not click for me to look at "man 3 regcomp" which says:
> 
>     int regcomp(regex_t *preg, const char *regex, int cflags);
> 
> So unless others feel that we might get a better layering separation by
> not storing REG_EXTENDED and stuff directly in that field (which was my
> initial reaction without looking at 4/4 which does store REG_EXTENDED
> there without our own enums), cflag is perfectly a good name here.
> 

I think it makes perfect sense to use whatever we pass when compiling
the regex. I wouldn't dare try to hack up something that pre-mangles
a regular expression and assume it gets it right everywhere anyway, so
I'm quite happy with leaving it all to regcomp(3) and friends.


> Thanks --- I am bit under the weather and not thinking quite straight.
> 

Mix 2cc's of 7yo Havana Club into a large cup of tea. Drink one such
cup every hour and eat a fresh fruit with it. I haven't been ill a day
in my life since I came up with that most excellent cure for absolutely
everything. If nothing else, it makes it a bit less boring to be ill.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 2/4] diff.c: associate a flag with each pattern and use it for compiling regex

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

> Junio C Hamano wrote:
>> Andreas Ericsson <ae@op5.se> writes:
> ...
> I think it makes perfect sense to use whatever we pass when compiling
> the regex. I wouldn't dare try to hack up something that pre-mangles
> a regular expression and assume it gets it right everywhere anyway, so
> I'm quite happy with leaving it all to regcomp(3) and friends.

Oh, I never meant pre-mangling or anything funky like that.

What I was envisioning we might want to make more flexible was what we
build on top of regexp, such as the way how these multi-line stuff is
treated for example.  Currently more than one positive regexp concatenated
with "\n" are ANDed together and the captured string from the last one is
used, but it is plausible we might want to say "first positive capturing
match yields result for this pattern string", or something like that.

>> Thanks --- I am bit under the weather and not thinking quite straight.
>
> Mix 2cc's of 7yo Havana Club into a large cup of tea. Drink one such
> cup every hour and eat a fresh fruit with it. I haven't been ill a day
> in my life since I came up with that most excellent cure for absolutely
> everything. If nothing else, it makes it a bit less boring to be ill.

Heh, unfortunately I happen to live in the US.

Re: [PATCH 4/4] diff.c: convert builtin funcname patterns to extended regular expressions

[Johan Herland]

On Thursday 18 September 2008, Brandon Casey wrote:
> This is a blind conversion removing \\ before ( and { etc.
> and adding \\ before naked ( and { etc.
>
> I hope the authors who last touched these patterns will help with
> testing:
>
> bibtex: Johan Herland

This was moved by Junio when he applied my patch; the line was originally 
written by Gustaf Hendeby in 23b5beb28fdadbb1d80ebf686a35385609f7a180

>   html: Johan Herland

Works fine!

>    tex: Johan Herland

This was moved by Junio when he applied my patch; the line was last 
rewritten by Giuseppe Bilotta in 807d86945336f676c9f650a6cbae9baa3191aaec


...I just became a BIG fan of "git gui blame" ;)


Have fun!

...Johan


-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH 4/4] diff.c: convert builtin funcname patterns to extended regular expressions

[Gustaf Hendeby]

On 09/18/2008 10:39 AM, Johan Herland wrote:
> On Thursday 18 September 2008, Brandon Casey wrote:
>> This is a blind conversion removing \\ before ( and { etc.
>> and adding \\ before naked ( and { etc.
>>
>> I hope the authors who last touched these patterns will help with
>> testing:
>>
>> bibtex: Johan Herland
> 
> This was moved by Junio when he applied my patch; the line was originally 
> written by Gustaf Hendeby in 23b5beb28fdadbb1d80ebf686a35385609f7a180

I'm on the road the rest of this week and don't have access to a
suitable machine for testing until I get back.  Will put testing this on
the list of things to do for Monday.  The patch looks good, though, but
I haven't actually tested it.

/Gustaf

Re: [PATCH 4/4] diff.c: convert builtin funcname patterns to extended regular expressions

[Jonathan del Strother]

On Thu, Sep 18, 2008 at 1:21 AM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>
>
> This is a blind conversion removing \\ before ( and { etc.
> and adding \\ before naked ( and { etc.
>
> I hope the authors who last touched these patterns will help with testing:


None of the patterns using \\s seem to work for me.  I had to replace
them with [ \t] - is this a problem with the darwin regex
compatibility library or something?  I applied the patches on master
(97d7fee2cb), and am running OS X 10.5.5.

Re: [PATCH 4/4] diff.c: convert builtin funcname patterns to extended regular expressions

[Brandon Casey]

Jonathan del Strother wrote:
> On Thu, Sep 18, 2008 at 1:21 AM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
>> ---
>>
>>
>> This is a blind conversion removing \\ before ( and { etc.
>> and adding \\ before naked ( and { etc.
>>
>> I hope the authors who last touched these patterns will help with testing:
> 
> 
> None of the patterns using \\s seem to work for me.  I had to replace
> them with [ \t] - is this a problem with the darwin regex
> compatibility library or something?  I applied the patches on master
> (97d7fee2cb), and am running OS X 10.5.5.

I was going to say possibly \s is a gnu extension, but if by "compatibility
library", you mean compat/regex/regex.[ch] in the git source which is used
by default now on OSX, then that _is_ the gnu library.

I just tried the ruby pattern on IRIX6.5 and Solaris7 and \\s does not work.
I am not using compat/regex/regex.[ch]. Same pattern works on linux.

Looks like '\\s' needs to be changed to ' '.

-brandon

Re: [PATCH 3/4] diff.*.xfuncname which uses "extended" regex's for hunk header selection

[Brandon Casey]

Junio C Hamano wrote:

> I personally think it was a mistake that the loop returns failure when
> there is any pattern that does not match (it is a useful feature that a
> match with a negated one forces an early return, though).
> 
> We may want to fix the semantics to be something like:
> 
> 	for (i = 0; i < regs->nr; i++) {
>         	struct ff_reg *reg = &regs->array[i];
>                 if (!regexec(&reg->re, line_buffer, 2, pmatch, 0)) {
> 			if (reg->negate) {
> 	                	free(line_buffer);
> 	                	return -1;
> 			}
>                         break;
> 		}
> 	}
>         if (regs->nr <= i) {
>         	free(line_buffer);
> 		return -1;
> 	}
> 	... use pmatch() ...
> 
> I.e. (1) negative match forces an early return (useful for catching
> language keywords), (2) first positive match is used, and (3) no match is
> a failure.
> 
> Of course, by definition the above "fix" changes the semantics, and will
> break people's existing setup if somebody has an existing custom pattern
> string that does use more than one positive regexp anded together with
> "\n", but I somehow suspect nobody sane depends on the current broken
> semantics.
> 
> It would help making JdS's ObjC alternates easier to write.  You can say:
> 
> 
> 	/* A or B */
> 	"...(A|B)$"
>         "\n" /* or C or D */
>         "...(C|D)$"
> 
> and both captures around A|B and C|D would be saved in $1.

Just to let you know another pair of eyes has given the above a
once over, I agree with your proposal. "or" semantics makes more sense
than "and".

-brandon