[PATCH] Teach git diff about Objective-C syntax

[PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.

Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
---
 Documentation/gitattributes.txt |    2 ++
 diff.c                          |    6 ++++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 6f3551d..c1f13e2 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -315,6 +315,8 @@ patterns are available:
 
 - `java` suitable for source code in the Java language.
 
+- `objc` suitable for source code in the Objective-C language.
+
 - `pascal` suitable for source code in the Pascal/Delphi language.
 
 - `php` suitable for source code in the PHP language.
diff --git a/diff.c b/diff.c
index 998dcaa..6ed8322 100644
--- a/diff.c
+++ b/diff.c
@@ -1403,6 +1403,12 @@ static struct builtin_funcname_pattern {
 			"^[ 	]*\\(\\([ 	]*"
 			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
 			"[ 	]*([^;]*\\)$" },
+	{ "objc", "^[\t ]*\\([-+][\t ]*([\t ]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$"      // Objective-C methods
+			"\\|"
+			"^[\t ]*\\(\\([\t ]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[\t ]*([^;]*\\)$" // C functions
+			"\\|"
+			"^@\\(implementation\\|interface\\|protocol\\).*"   // Objective-C class/protocol definitions
+			},
 	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
 			"destructor\\|interface\\|implementation\\|"
 			"initialization\\|finalization\\)[ \t]*.*\\)$"
-- 
1.6.0.2

Re: [PATCH] Teach git diff about Objective-C syntax

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

On Wed, Sep 17, 2008 at 12:21:37AM +0100, Jonathan del Strother <jon.delStrother@bestbefore.tv> wrote:
> +	{ "objc", "^[\t ]*\\([-+][\t ]*([\t ]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$"      // Objective-C methods
> +			"\\|"
> +			"^[\t ]*\\(\\([\t ]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[\t ]*([^;]*\\)$" // C functions
> +			"\\|"
> +			"^@\\(implementation\\|interface\\|protocol\\).*"   // Objective-C class/protocol definitions

Please avoid C++-style comments. Just use /* */.

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.

Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
---
 Documentation/gitattributes.txt |    2 ++
 diff.c                          |    6 ++++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 6f3551d..c1f13e2 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -315,6 +315,8 @@ patterns are available:
 
 - `java` suitable for source code in the Java language.
 
+- `objc` suitable for source code in the Objective-C language.
+
 - `pascal` suitable for source code in the Pascal/Delphi language.
 
 - `php` suitable for source code in the PHP language.
diff --git a/diff.c b/diff.c
index 998dcaa..5f4e63b 100644
--- a/diff.c
+++ b/diff.c
@@ -1403,6 +1403,12 @@ static struct builtin_funcname_pattern {
 			"^[ 	]*\\(\\([ 	]*"
 			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
 			"[ 	]*([^;]*\\)$" },
+	{ "objc", "^[\t ]*\\([-+][\t ]*([\t ]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$" /* Objective-C methods */
+			"\\|"
+			"^[\t ]*\\(\\([\t ]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[\t ]*([^;]*\\)$" /* C functions */
+			"\\|"
+			"^@\\(implementation\\|interface\\|protocol\\).*" /* Objective-C class/protocol definitions */
+			},
 	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
 			"destructor\\|interface\\|implementation\\|"
 			"initialization\\|finalization\\)[ \t]*.*\\)$"
-- 
1.6.0.2

[PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.

Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
---

This version of the patch adds the negate line to avoid picking up if()/while()/etc statements as funcnames
I also took the liberty of replacing the invisible tab characters in the java pattern with \t's - is that OK?

 Documentation/gitattributes.txt |    2 ++
 diff.c                          |   19 ++++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 6f3551d..c1f13e2 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -315,6 +315,8 @@ patterns are available:
 
 - `java` suitable for source code in the Java language.
 
+- `objc` suitable for source code in the Objective-C language.
+
 - `pascal` suitable for source code in the Pascal/Delphi language.
 
 - `php` suitable for source code in the PHP language.
diff --git a/diff.c b/diff.c
index 998dcaa..fdfdf26 100644
--- a/diff.c
+++ b/diff.c
@@ -1398,17 +1398,26 @@ static struct builtin_funcname_pattern {
 } builtin_funcname_pattern[] = {
 	{ "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$" },
 	{ "html", "^\\s*\\(<[Hh][1-6]\\s.*>.*\\)$" },
-	{ "java", "!^[ 	]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
+
+	{ "java", "!^[ \t]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
 			"new\\|return\\|switch\\|throw\\|while\\)\n"
-			"^[ 	]*\\(\\([ 	]*"
+			"^[ \t]*\\(\\([ \t]*"
 			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
-			"[ 	]*([^;]*\\)$" },
+			"[ \t]*([^;]*\\)$" },
+
+	{ "objc", "![ \t]*\\(do\\|for\\|if\\|else\\|return\\|switch\\|while\\)\n" /* Negate C statements that can look like functions */
+			"^[ \t]*\\([-+][ \t]*([ \t]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$" /* Objective-C methods */
+			"\\|"
+			"^[ \t]*\\(\\([ \t]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[ \t]*([^;]*\\)$" /* C functions */
+			"\\|"
+			"^@\\(implementation\\|interface\\|protocol\\).*"}, /* Objective-C class/protocol definitions */
+
 	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
 			"destructor\\|interface\\|implementation\\|"
 			"initialization\\|finalization\\)[ \t]*.*\\)$"
 			"\\|"
-			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"
-			},
+			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"},
+
 	{ "php", "^[\t ]*\\(\\(function\\|class\\).*\\)" },
 	{ "python", "^\\s*\\(\\(class\\|def\\)\\s.*\\)$" },
 	{ "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$" },
-- 
1.6.0.2.250.g2fdd2.dirty

Re: [PATCH] Teach git diff about Objective-C syntax

[Johannes Schindelin]

Hi,

On Wed, 17 Sep 2008, Jonathan del Strother wrote:

> -			"^[ 	]*\\(\\([ 	]*"
> +			"^[ \t]*\\(\\([ \t]*"

Why?

>  			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
> -			"[ 	]*([^;]*\\)$" },
> +			"[ \t]*([^;]*\\)$" },
> +
> +	{ "objc", "![ \t]*\\(do\\|for\\|if\\|else\\|return\\|switch\\|while\\)\n" /* Negate C statements that can look like functions */
> +			"^[ \t]*\\([-+][ \t]*([ \t]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$" /* Objective-C methods */

I see you still have these awfully long lines.

Ciao,
Dscho

Re: [PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 17 Sep 2008, Jonathan del Strother wrote:
>
>> -                     "^[     ]*\\(\\([       ]*"
>> +                     "^[ \t]*\\(\\([ \t]*"
>
> Why?
>
>>                       "[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
>> -                     "[      ]*([^;]*\\)$" },
>> +                     "[ \t]*([^;]*\\)$" },
>> +
>> +     { "objc", "![ \t]*\\(do\\|for\\|if\\|else\\|return\\|switch\\|while\\)\n" /* Negate C statements that can look like functions */
>> +                     "^[ \t]*\\([-+][ \t]*([ \t]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$" /* Objective-C methods */
>
> I see you still have these awfully long lines.

I was hoping I'd get away with it since the overlong parts are just comments...

You'd prefer something like
	{ "objc",
			/* Negate C statements that can look like functions */
			"![ \t]*\\(do\\|for\\|if\\|else\\|return\\|switch\\|while\\)\n"
			/* Objective-C methods */
			"^[ \t]*\\([-+][ \t]*([ \t]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$"
			"\\|"
			/* C functions */
			"^[ \t]*\\(\\([ \t]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[ \t]*([^;]*\\)$"
			"\\|"
			/* Objective-C class/protocol definitions */
			"^@\\(implementation\\|interface\\|protocol\\).*"},

?

[PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.

Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
---
This version anchors the negated match to the beginning of the line, and shuffles the comments around to avoid the excessively long lines.  Better?


 Documentation/gitattributes.txt |    2 ++
 diff.c                          |   24 +++++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 6f3551d..c1f13e2 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -315,6 +315,8 @@ patterns are available:
 
 - `java` suitable for source code in the Java language.
 
+- `objc` suitable for source code in the Objective-C language.
+
 - `pascal` suitable for source code in the Pascal/Delphi language.
 
 - `php` suitable for source code in the PHP language.
diff --git a/diff.c b/diff.c
index 998dcaa..e5ec503 100644
--- a/diff.c
+++ b/diff.c
@@ -1398,17 +1398,31 @@ static struct builtin_funcname_pattern {
 } builtin_funcname_pattern[] = {
 	{ "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$" },
 	{ "html", "^\\s*\\(<[Hh][1-6]\\s.*>.*\\)$" },
-	{ "java", "!^[ 	]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
+
+	{ "java", "!^[ \t]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
 			"new\\|return\\|switch\\|throw\\|while\\)\n"
-			"^[ 	]*\\(\\([ 	]*"
+			"^[ \t]*\\(\\([ \t]*"
 			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
-			"[ 	]*([^;]*\\)$" },
+			"[ \t]*([^;]*\\)$" },
+
+	{ "objc",
+			/* Negate C statements that can look like functions */
+			"!^[ \t]*\\(do\\|for\\|if\\|else\\|return\\|switch\\|while\\)\n"
+			/* Objective-C methods */
+			"^[ \t]*\\([-+][ \t]*([ \t]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$"
+			"\\|"
+			/* C functions */
+			"^[ \t]*\\(\\([ \t]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[ \t]*([^;]*\\)$"
+			"\\|"
+			/* Objective-C class/protocol definitions */
+			"^@\\(implementation\\|interface\\|protocol\\).*"},
+
 	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
 			"destructor\\|interface\\|implementation\\|"
 			"initialization\\|finalization\\)[ \t]*.*\\)$"
 			"\\|"
-			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"
-			},
+			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"},
+
 	{ "php", "^[\t ]*\\(\\(function\\|class\\).*\\)" },
 	{ "python", "^\\s*\\(\\(class\\|def\\)\\s.*\\)$" },
 	{ "ruby", "^\\s*\\(\\(class\\|module\\|def\\)\\s.*\\)$" },
-- 
1.6.0.2.250.ge7a3b.dirty

Re: [PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 17 Sep 2008, Jonathan del Strother wrote:
>
>> -                     "^[     ]*\\(\\([       ]*"
>> +                     "^[ \t]*\\(\\([ \t]*"
>
> Why?

Sorry - missed that in the original reply.  You really prefer having
an invisible tab character to an explicit \t ?

Re: [PATCH] Teach git diff about Objective-C syntax

[Johannes Schindelin]

Hi,

On Wed, 17 Sep 2008, Jonathan del Strother wrote:

> On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Wed, 17 Sep 2008, Jonathan del Strother wrote:
> >
> >> -                     "^[     ]*\\(\\([       ]*"
> >> +                     "^[ \t]*\\(\\([ \t]*"
> >
> > Why?
> 
> Sorry - missed that in the original reply.  You really prefer having
> an invisible tab character to an explicit \t ?

No.  But I prefer clear-cut commits, not sneaked-in changes.  This is a 
change in the java part FYI.

Ciao,
Dscho

Re: [PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

On Wed, Sep 17, 2008 at 3:28 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 17 Sep 2008, Jonathan del Strother wrote:
>
>> On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi,
>> >
>> > On Wed, 17 Sep 2008, Jonathan del Strother wrote:
>> >
>> >> -                     "^[     ]*\\(\\([       ]*"
>> >> +                     "^[ \t]*\\(\\([ \t]*"
>> >
>> > Why?
>>
>> Sorry - missed that in the original reply.  You really prefer having
>> an invisible tab character to an explicit \t ?
>
> No.  But I prefer clear-cut commits, not sneaked-in changes.  This is a
> change in the java part FYI.
>

Sorry, I considered it to be a formatting change & didn't think it
would matter.  I'll separate them out & repost later

Re: [PATCH] Teach git diff about Objective-C syntax

[Andreas Ericsson]

Jonathan del Strother wrote:
> Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.
> 
> Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
> ---
> This version anchors the negated match to the beginning of the line, and shuffles the comments around to avoid the excessively long lines.  Better?
> 

Why do you insist on touching surrounding patterns? I've left them in
from your patch below so you can see where you're going wrong. Those
changes provide no value and make your patch harder to read. If you
want to make that cleanup, submit it separately, as it has nothing
to do with teaching git diff about objective C methods.


> diff --git a/diff.c b/diff.c
> index 998dcaa..e5ec503 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1398,17 +1398,31 @@ static struct builtin_funcname_pattern {
>  } builtin_funcname_pattern[] = {
>  	{ "bibtex", "\\(@[a-zA-Z]\\{1,\\}[ \t]*{\\{0,1\\}[ \t]*[^ \t\"@',\\#}{~%]*\\).*$" },
>  	{ "html", "^\\s*\\(<[Hh][1-6]\\s.*>.*\\)$" },
> -	{ "java", "!^[ 	]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
> +
> +	{ "java", "!^[ \t]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
>  			"new\\|return\\|switch\\|throw\\|while\\)\n"
> -			"^[ 	]*\\(\\([ 	]*"
> +			"^[ \t]*\\(\\([ \t]*"
>  			"[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}"
> -			"[ 	]*([^;]*\\)$" },
> +			"[ \t]*([^;]*\\)$" },
> +

The above should be in a separate patch.

>  	{ "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
>  			"destructor\\|interface\\|implementation\\|"
>  			"initialization\\|finalization\\)[ \t]*.*\\)$"
>  			"\\|"
> -			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"
> -			},
> +			"^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"},
> +

This last change is just pure nonsense. Please remove it altogether.

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

Re: [PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

On Wed, Sep 17, 2008 at 4:03 PM, Andreas Ericsson <ae@op5.se> wrote:
> Jonathan del Strother wrote:
>>        { "pascal", "^\\(\\(procedure\\|function\\|constructor\\|"
>>                        "destructor\\|interface\\|implementation\\|"
>>                        "initialization\\|finalization\\)[ \t]*.*\\)$"
>>                        "\\|"
>> -                       "^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"
>> -                       },
>> +                       "^\\(.*=[ \t]*\\(class\\|record\\).*\\)$"},
>> +
>
> This last change is just pure nonsense. Please remove it altogether.
>


I was changing it to match the style in the existing java pattern (and
my objc pattern).  You think the java one should be changed to match
the pascal one, then?

Re: [PATCH] Teach git diff about Objective-C syntax

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

On Wed, Sep 17, 2008 at 04:31:17PM +0100, Jonathan del Strother <maillist@steelskies.com> wrote:
> I was changing it to match the style in the existing java pattern (and
> my objc pattern).  You think the java one should be changed to match
> the pascal one, then?

The point is that it's unrelated, so you should not change that part in
the same patch. Send a separate patch if you want to do something
unrelated to Objective-C.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

On Wed, Sep 17, 2008 at 4:55 PM, Miklos Vajna <vmiklos@frugalware.org> wrote:
> On Wed, Sep 17, 2008 at 04:31:17PM +0100, Jonathan del Strother <maillist@steelskies.com> wrote:
>> I was changing it to match the style in the existing java pattern (and
>> my objc pattern).  You think the java one should be changed to match
>> the pascal one, then?
>
> The point is that it's unrelated, so you should not change that part in
> the same patch. Send a separate patch if you want to do something
> unrelated to Objective-C.

Johannes already convinced me to do it as a separate patch.  Andreas
seems to think that even if that change were in a separate patch, it
is pure nonsense.  I think it's pretty subjective - I was just making
things consistent.

Re: [PATCH] Teach git diff about Objective-C syntax

[Junio C Hamano]

"Jonathan del Strother" <maillist@steelskies.com> writes:

> Johannes already convinced me to do it as a separate patch.  Andreas
> seems to think that even if that change were in a separate patch, it
> is pure nonsense.  I think it's pretty subjective - I was just making
> things consistent.

I think the discussion is getting into personal preference, but I do agree
making things consistent is good.

Short and simple ones can stay the same as before, but I'd actually prefer
doing ones on multiple lines like this:

	{ "java",
	  /* Do not match these keywords */
	  "!^[ \t]*\\(catch\\|do\\|for\\|if\\|instanceof\\|"
		"new\\|return\\|switch\\|throw\\|while\\)\n"

	  /* possibly indented "foo bar(..." */
	  "^[ \t]*\\(\\([ \t]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[ \t]*([^;]*\\)$"
	},

	{ "objc",
	  /* Do not match with these C statements */
	  "!^[ \t]*\\(do\\|for\\|if\\|else\\|return\\|switch\\|while\\)\n"

	  /* Objective-C methods "-(fo*&^)%*&%$^" or "+(bar&^)%$#$%" */
	  "^[ \t]*\\([-+][ \t]*([ \t]*[A-Za-z_][A-Za-z_0-9]*.*).*\\)$"

	  "\\|" /* C functions "foo bar(..." */
	  "^[ \t]*\\(\\([ \t]*[A-Za-z_][A-Za-z_0-9]*\\)\\{2,\\}[ \t]*([^;]*\\)$"

	  "\\|" /* Objective-C class/protocol definitions */
	  "^@\\(implementation\\|interface\\|protocol\\).*"
	},

That is:

 - The pattern string indented to align with the pattern name;
 - Indent second and later lines of a single alternatives list that is
   split over multiple physical lines;

Are you sure the regexp you have for Objective-C methods quotes a dot "."
correctly, by the way?  It appears to match almost anything enclosed in a
pair of parentheses, as long as you have two alpha after open paren.

Also I am not sure if you can do the pattern alternates the way you did.
If you have this:

	"...\\(A\\|B\\)$"
        "\\|"
        "...\\(C\\|D\\)$"

A and B will be captured as $1 but wouldn't C or D captured as $2?

Re: [PATCH] Teach git diff about Objective-C syntax

[Jonathan del Strother]

On Wed, Sep 17, 2008 at 9:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Are you sure the regexp you have for Objective-C methods quotes a dot "."
> correctly, by the way?  It appears to match almost anything enclosed in a
> pair of parentheses, as long as you have two alpha after open paren.

It could be a little more restrictive, but I'm not sure how much
further it could be taken without it getting ridiculous.  Just as some
examples, it needs to match :
+(id)someClassMethod
-(id)someInstanceMethod
-(NSNumber*)numberWithNumerator:(int)n denominator:(int)d
-(NSString*)stringWithVariableArguments:(NSString*)format ...

(I seem to remember that if your method returns 'id' you can skip the
return type, eg "- someInstanceMethod", but I don't think there's any
hope for people that use that notation...)

^[ \t]*\\([-+][ \t]*([ \t]*[A-Za-z_][A-Za-z_0-9* \t]*)[ \t]*_?[A-Za-z]*\\)$
would be a small improvement - that is, a plus or minus, brackets
around the return type, then at least one letter for the method name,
potentially preceded by an underscore.  Can anyone suggest anything
better?

Also, why can't I use \s for whitespace, \w for word characters etc?
I thought we were using the gnu regex library?


> Also I am not sure if you can do the pattern alternates the way you did.
> If you have this:
>
>        "...\\(A\\|B\\)$"
>        "\\|"
>        "...\\(C\\|D\\)$"
>
> A and B will be captured as $1 but wouldn't C or D captured as $2?

Hmm, hadn't considered that.  It appears to work fine, but I haven't
yet had chance to track down exactly where the funcname regexp match
occurs to see what the code is doing with the different matches.  I'll
try and take a look tomorrow, unless someone can enlighten me before
then...

Re: [PATCH] Teach git diff about Objective-C syntax

[Andreas Ericsson]

Jonathan del Strother wrote:
> On Wed, Sep 17, 2008 at 4:55 PM, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> On Wed, Sep 17, 2008 at 04:31:17PM +0100, Jonathan del Strother <maillist@steelskies.com> wrote:
>>> I was changing it to match the style in the existing java pattern (and
>>> my objc pattern).  You think the java one should be changed to match
>>> the pascal one, then?
>> The point is that it's unrelated, so you should not change that part in
>> the same patch. Send a separate patch if you want to do something
>> unrelated to Objective-C.
> 
> Johannes already convinced me to do it as a separate patch.  Andreas
> seems to think that even if that change were in a separate patch, it
> is pure nonsense.  I think it's pretty subjective - I was just making
> things consistent.

My apologies. I started writing that reply before Dscho's one got to the
list. Then it was lunch-time, so I didn't send it until Dscho's reply
was already answered by you.

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

[PATCH v2] Teach git diff about Objective-C syntax

[Jonathan del Strother]

Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.

Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
---
This version is much the same, but rebuilt on top of 1883a0d3b to use the extended regexp stuff, and it doesn't attempt to tidy up other patterns.

I've been trying to make the objc-method matching line a bit more specific - I think I'm running into a bug (or more likely a misunderstanding) in the matching process.
Every pattern there uses either .*$ or [^;]*$ to match up to the end of a line.  But in trying to come up with a whitelist of characters to match up to the end of a line, I couldn't do it : there seems to be an invisible character at the end of the line that I can't match.
That is, a line containing just "FUNCNAME" (terminated by a newline) will be matched by the pattern "^(FUNCNAME.$)" but not "^(FUNCNAME$)".
Why is this?


 Documentation/gitattributes.txt |    2 ++
 diff.c                          |   10 ++++++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2ae771f..2694559 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -315,6 +315,8 @@ patterns are available:
 
 - `java` suitable for source code in the Java language.
 
+- `objc` suitable for source code in the Objective-C language.
+
 - `pascal` suitable for source code in the Pascal/Delphi language.
 
 - `php` suitable for source code in the PHP language.
diff --git a/diff.c b/diff.c
index b001d7b..3694602 100644
--- a/diff.c
+++ b/diff.c
@@ -1429,6 +1429,16 @@ static const struct funcname_pattern_entry builtin_funcname_pattern[] = {
 	  "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 	  "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$",
 	  REG_EXTENDED },
+	{ "objc",
+	  /* Negate C statements that can look like functions */
+	  "!^[ \t]*(do|for|if|else|return|switch|while)\n"
+	  /* Objective-C methods */
+	  "^[ \t]*([-+][ \t]*\\([ \t]*[A-Za-z_][A-Za-z_0-9* \t]*\\)[ \t]*[A-Za-z_].*)$\n"
+	  /* C functions */
+	  "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$\n"
+	  /* Objective-C class/protocol definitions */
+	  "^(@(implementation|interface|protocol)[ \t].*)$",
+	  REG_EXTENDED },
 	{ "pascal",
 	  "^((procedure|function|constructor|destructor|interface|"
 		"implementation|initialization|finalization)[ \t]*.*)$"
-- 
1.6.0.2.416.g8cab.dirty

Re: [PATCH v2] Teach git diff about Objective-C syntax

[Brandon Casey]

Jonathan del Strother wrote:
> Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.
> 
> Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
> ---
> This version is much the same, but rebuilt on top of 1883a0d3b to use the extended regexp stuff, and it doesn't attempt to tidy up other patterns.
> 
> I've been trying to make the objc-method matching line a bit more specific - I think I'm running into a bug (or more likely a misunderstanding) in the matching process.
> Every pattern there uses either .*$ or [^;]*$ to match up to the end of a line.  But in trying to come up with a whitelist of characters to match up to the end of a line, I couldn't do it : there seems to be an invisible character at the end of the line that I can't match.
> That is, a line containing just "FUNCNAME" (terminated by a newline) will be matched by the pattern "^(FUNCNAME.$)" but not "^(FUNCNAME$)".
> Why is this?

I think it is the newline which git is not removing from the string passed to regexec.

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

   "...In the regular expression processing described in IEEE Std 1003.1-2001,
    the <newline> is regarded as an ordinary character and both a period and a
    non-matching list can match one."

   "...Those utilities (like grep) that do not allow <newline>s to match are
    responsible for eliminating any <newline> from strings before matching against
    the RE."


Possibly something like:

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 8bab82e..f471c7c 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -191,7 +191,7 @@ struct ff_regs {
 static long ff_regexp(const char *line, long len,
		char *buffer, long buffer_size, void *priv)
 {
-	char *line_buffer = xstrndup(line, len); /* make NUL terminated */
+	char *line_buffer = xstrndup(line, len-1); /* make NUL terminated */
 	struct ff_regs *regs = priv;
 	regmatch_t pmatch[2];
 	int i;

[PATCH] xdiff-interface.c: strip newline (and cr) from line before pattern matching

[Brandon Casey]

POSIX doth sayeth:

   "In the regular expression processing described in IEEE Std 1003.1-2001,
    the <newline> is regarded as an ordinary character and both a period and
    a non-matching list can match one. ... Those utilities (like grep) that
    do not allow <newline>s to match are responsible for eliminating any
    <newline> from strings before matching against the RE."

Thus far git has not been removing the trailing newline from strings matched
against regular expression patterns. This has the effect that (quoting
Jonathan del Strother) "... a line containing just 'FUNCNAME' (terminated by
a newline) will be matched by the pattern '^(FUNCNAME.$)' but not
'^(FUNCNAME$)'", and more simply not '^FUNCNAME$'.

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


This could be a little simpler if I knew what was guaranteed from xdiff.
Such as whether the len elements of line were guaranteed to be newline
terminated, or be greater than zero. But, the code in def_ff() in xemit.c
is wrapped in 'if (len > 0)', so..

-brandon


 xdiff-interface.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 8bab82e..61f5dab 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -191,12 +191,22 @@ struct ff_regs {
 static long ff_regexp(const char *line, long len,
 		char *buffer, long buffer_size, void *priv)
 {
-	char *line_buffer = xstrndup(line, len); /* make NUL terminated */
+	char *line_buffer;
 	struct ff_regs *regs = priv;
 	regmatch_t pmatch[2];
 	int i;
 	int result = -1;
 
+	/* Exclude terminating newline (and cr) from matching */
+	if (len > 0 && line[len-1] == '\n') {
+		if (len > 1 && line[len-2] == '\r')
+			len -= 2;
+		else
+			len--;
+	}
+
+	line_buffer = xstrndup(line, len); /* make NUL terminated */
+
 	for (i = 0; i < regs->nr; i++) {
 		struct ff_reg *reg = regs->array + i;
 		if (!regexec(&reg->re, line_buffer, 2, pmatch, 0)) {
-- 
1.6.0.2.323.g7c850

Re: [PATCH] xdiff-interface.c: strip newline (and cr) from line before pattern matching

[Johannes Schindelin]

Hi,

On Wed, 1 Oct 2008, Brandon Casey wrote:

> POSIX doth sayeth:
> 
>    "In the regular expression processing described in IEEE Std 1003.1-2001,
>     the <newline> is regarded as an ordinary character and both a period and
>     a non-matching list can match one. ... Those utilities (like grep) that
>     do not allow <newline>s to match are responsible for eliminating any
>     <newline> from strings before matching against the RE."
> 
> Thus far git has not been removing the trailing newline from strings matched
> against regular expression patterns. This has the effect that (quoting
> Jonathan del Strother) "... a line containing just 'FUNCNAME' (terminated by
> a newline) will be matched by the pattern '^(FUNCNAME.$)' but not
> '^(FUNCNAME$)'", and more simply not '^FUNCNAME$'.

Personally, I find the second paragraph pretty convincing.  Which is good, 
because the first fails to do that.

Ciao,
Dscho

Re: [PATCH] xdiff-interface.c: strip newline (and cr) from line before pattern matching

[Jonathan del Strother]

On Wed, Oct 1, 2008 at 8:28 PM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> POSIX doth sayeth:
>
>   "In the regular expression processing described in IEEE Std 1003.1-2001,
>    the <newline> is regarded as an ordinary character and both a period and
>    a non-matching list can match one. ... Those utilities (like grep) that
>    do not allow <newline>s to match are responsible for eliminating any
>    <newline> from strings before matching against the RE."
>
> Thus far git has not been removing the trailing newline from strings matched
> against regular expression patterns. This has the effect that (quoting
> Jonathan del Strother) "... a line containing just 'FUNCNAME' (terminated by
> a newline) will be matched by the pattern '^(FUNCNAME.$)' but not
> '^(FUNCNAME$)'", and more simply not '^FUNCNAME$'.
>
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>
>
> This could be a little simpler if I knew what was guaranteed from xdiff.
> Such as whether the len elements of line were guaranteed to be newline
> terminated, or be greater than zero. But, the code in def_ff() in xemit.c
> is wrapped in 'if (len > 0)', so..
>
> -brandon
>
>
>  xdiff-interface.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index 8bab82e..61f5dab 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -191,12 +191,22 @@ struct ff_regs {
>  static long ff_regexp(const char *line, long len,
>                char *buffer, long buffer_size, void *priv)
>  {
> -       char *line_buffer = xstrndup(line, len); /* make NUL terminated */
> +       char *line_buffer;
>        struct ff_regs *regs = priv;
>        regmatch_t pmatch[2];
>        int i;
>        int result = -1;
>
> +       /* Exclude terminating newline (and cr) from matching */
> +       if (len > 0 && line[len-1] == '\n') {
> +               if (len > 1 && line[len-2] == '\r')
> +                       len -= 2;
> +               else
> +                       len--;
> +       }
> +
> +       line_buffer = xstrndup(line, len); /* make NUL terminated */
> +
>        for (i = 0; i < regs->nr; i++) {
>                struct ff_reg *reg = regs->array + i;
>                if (!regexec(&reg->re, line_buffer, 2, pmatch, 0)) {
> --
> 1.6.0.2.323.g7c850
>
>

Looks good to me, or at least, works as advertised with a bunch of my
funcname patterns.

Re: [PATCH v2] Teach git diff about Objective-C syntax

[Jonathan del Strother]

On Wed, Oct 1, 2008 at 12:46 AM, Jonathan del Strother
<jon.delStrother@bestbefore.tv> wrote:
> Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces.
>
> Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
> ---
> This version is much the same, but rebuilt on top of 1883a0d3b to use the extended regexp stuff, and it doesn't attempt to tidy up other patterns.
>
>  Documentation/gitattributes.txt |    2 ++
>  diff.c                          |   10 ++++++++++
>  2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 2ae771f..2694559 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -315,6 +315,8 @@ patterns are available:
>
>  - `java` suitable for source code in the Java language.
>
> +- `objc` suitable for source code in the Objective-C language.
> +
>  - `pascal` suitable for source code in the Pascal/Delphi language.
>
>  - `php` suitable for source code in the PHP language.
> diff --git a/diff.c b/diff.c
> index b001d7b..3694602 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1429,6 +1429,16 @@ static const struct funcname_pattern_entry builtin_funcname_pattern[] = {
>          "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
>          "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$",
>          REG_EXTENDED },
> +       { "objc",
> +         /* Negate C statements that can look like functions */
> +         "!^[ \t]*(do|for|if|else|return|switch|while)\n"
> +         /* Objective-C methods */
> +         "^[ \t]*([-+][ \t]*\\([ \t]*[A-Za-z_][A-Za-z_0-9* \t]*\\)[ \t]*[A-Za-z_].*)$\n"
> +         /* C functions */
> +         "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$\n"
> +         /* Objective-C class/protocol definitions */
> +         "^(@(implementation|interface|protocol)[ \t].*)$",
> +         REG_EXTENDED },
>        { "pascal",
>          "^((procedure|function|constructor|destructor|interface|"
>                "implementation|initialization|finalization)[ \t]*.*)$"
> --


Given Brandon's "strip newline (and cr) from line before pattern
matching" patch, the objective C line could be changed to
"^[ \t]*([-+][ \t]*\\([ \t]*[A-Za-z_][A-Za-z_0-9* \t]*\\)[
\t]*[A-Za-z_][A-Za-z_0-9:{()*& \t]*)$\n"
to be more specific about what's allowed to occur on a method line.
Depends how often we really care about getting the funcname right -
for instance, do we want to deal with cases like :

-(void)doStuff:(NSString*)foo {    // TODO : This is a %@^$#@ method
name, change it.

?   I suspect the additional complexity that would be added to the
regex isn't worth the small gain - any thoughts?