# [PATCH] Teach git diff about Objective-C syntax 23 messages from 2008-09-16 to 2008-10-02. Participants: Jonathan del Strother, Miklos Vajna, Johannes Schindelin, Andreas Ericsson, Junio C Hamano, Brandon Casey. Thread: https://gitlist.dev/t/15559 ## Jonathan del Strother, 2008-09-16 23:21 Subject: [PATCH] Teach git diff about Objective-C syntax Message-ID: <1221607297-33212-1-git-send-email-jon.delStrother@bestbefore.tv> URL: https://gitlist.dev/e/1221607297-33212-1-git-send-email-jon.delStrother%40bestbefore.tv ``` Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces. Signed-off-by: Jonathan del Strother --- 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 ``` ## Miklos Vajna, 2008-09-16 23:39 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <20080916233923.GF4829@genesis.frugalware.org> URL: https://gitlist.dev/e/20080916233923.GF4829%40genesis.frugalware.org In-Reply-To: <1221607297-33212-1-git-send-email-jon.delStrother@bestbefore.tv> ``` On Wed, Sep 17, 2008 at 12:21:37AM +0100, Jonathan del Strother 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. ``` ## Jonathan del Strother, 2008-09-17 00:07 Subject: [PATCH] Teach git diff about Objective-C syntax Message-ID: <1221610063-33610-1-git-send-email-jon.delStrother@bestbefore.tv> URL: https://gitlist.dev/e/1221610063-33610-1-git-send-email-jon.delStrother%40bestbefore.tv In-Reply-To: <20080916233923.GF4829@genesis.frugalware.org> ``` Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces. Signed-off-by: Jonathan del Strother --- 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 ``` ## Jonathan del Strother, 2008-09-17 10:49 Subject: [PATCH] Teach git diff about Objective-C syntax Message-ID: <1221648564-56239-1-git-send-email-jon.delStrother@bestbefore.tv> URL: https://gitlist.dev/e/1221648564-56239-1-git-send-email-jon.delStrother%40bestbefore.tv In-Reply-To: <1221610063-33610-1-git-send-email-jon.delStrother@bestbefore.tv> ``` Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces. Signed-off-by: Jonathan del Strother --- 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 ``` ## Johannes Schindelin, 2008-09-17 11:06 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0809171205430.14329%40racer In-Reply-To: <1221648564-56239-1-git-send-email-jon.delStrother@bestbefore.tv> ``` 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 ``` ## Jonathan del Strother, 2008-09-17 12:26 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <57518fd10809170526i5c1e7dadgc38bb00e8073ba55@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10809170526i5c1e7dadgc38bb00e8073ba55%40mail.gmail.com In-Reply-To: ``` On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin 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\\).*"}, ? ``` ## Jonathan del Strother, 2008-09-17 13:29 Subject: [PATCH] Teach git diff about Objective-C syntax Message-ID: <1221658141-75698-1-git-send-email-jon.delStrother@bestbefore.tv> URL: https://gitlist.dev/e/1221658141-75698-1-git-send-email-jon.delStrother%40bestbefore.tv In-Reply-To: <57518fd10809170526i5c1e7dadgc38bb00e8073ba55@mail.gmail.com> ``` Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces. Signed-off-by: Jonathan del Strother --- 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 ``` ## Jonathan del Strother, 2008-09-17 13:37 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <57518fd10809170637n585afacbs94fdddc5b8bb61b6@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10809170637n585afacbs94fdddc5b8bb61b6%40mail.gmail.com In-Reply-To: ``` On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin 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 ? ``` ## Johannes Schindelin, 2008-09-17 14:28 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0809171528120.14329%40racer In-Reply-To: <57518fd10809170637n585afacbs94fdddc5b8bb61b6@mail.gmail.com> ``` Hi, On Wed, 17 Sep 2008, Jonathan del Strother wrote: > On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin > 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 ``` ## Jonathan del Strother, 2008-09-17 14:32 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <57518fd10809170732g507bc60ar4427f940b4343c37@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10809170732g507bc60ar4427f940b4343c37%40mail.gmail.com In-Reply-To: ``` On Wed, Sep 17, 2008 at 3:28 PM, Johannes Schindelin wrote: > Hi, > > On Wed, 17 Sep 2008, Jonathan del Strother wrote: > >> On Wed, Sep 17, 2008 at 12:06 PM, Johannes Schindelin >> 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 ``` ## Andreas Ericsson, 2008-09-17 15:03 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <48D11C3C.5070707@op5.se> URL: https://gitlist.dev/e/48D11C3C.5070707%40op5.se In-Reply-To: <1221658141-75698-1-git-send-email-jon.delStrother@bestbefore.tv> ``` 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 > --- > 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 ``` ## Jonathan del Strother, 2008-09-17 15:31 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <57518fd10809170831x6d84aeb0m9b0b2c4095a1de70@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10809170831x6d84aeb0m9b0b2c4095a1de70%40mail.gmail.com In-Reply-To: <48D11C3C.5070707@op5.se> ``` On Wed, Sep 17, 2008 at 4:03 PM, Andreas Ericsson 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? ``` ## Miklos Vajna, 2008-09-17 15:55 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <20080917155505.GH4829@genesis.frugalware.org> URL: https://gitlist.dev/e/20080917155505.GH4829%40genesis.frugalware.org In-Reply-To: <57518fd10809170831x6d84aeb0m9b0b2c4095a1de70@mail.gmail.com> ``` On Wed, Sep 17, 2008 at 04:31:17PM +0100, Jonathan del Strother 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. ``` ## Jonathan del Strother, 2008-09-17 19:14 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <57518fd10809171214u3b5b3b96yc432c1c410faf8b4@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10809171214u3b5b3b96yc432c1c410faf8b4%40mail.gmail.com In-Reply-To: <20080917155505.GH4829@genesis.frugalware.org> ``` On Wed, Sep 17, 2008 at 4:55 PM, Miklos Vajna wrote: > On Wed, Sep 17, 2008 at 04:31:17PM +0100, Jonathan del Strother 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. ``` ## Junio C Hamano, 2008-09-17 20:44 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <7vljxq4i5p.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vljxq4i5p.fsf%40gitster.siamese.dyndns.org In-Reply-To: <57518fd10809171214u3b5b3b96yc432c1c410faf8b4@mail.gmail.com> ``` "Jonathan del Strother" 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? ``` ## Jonathan del Strother, 2008-09-17 23:30 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <57518fd10809171630v97485aalcc5089f96082c0fc@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10809171630v97485aalcc5089f96082c0fc%40mail.gmail.com In-Reply-To: <7vljxq4i5p.fsf@gitster.siamese.dyndns.org> ``` On Wed, Sep 17, 2008 at 9:44 PM, Junio C Hamano 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... ``` ## Andreas Ericsson, 2008-09-18 06:50 Subject: Re: [PATCH] Teach git diff about Objective-C syntax Message-ID: <48D1FA3E.6020102@op5.se> URL: https://gitlist.dev/e/48D1FA3E.6020102%40op5.se In-Reply-To: <57518fd10809171214u3b5b3b96yc432c1c410faf8b4@mail.gmail.com> ``` Jonathan del Strother wrote: > On Wed, Sep 17, 2008 at 4:55 PM, Miklos Vajna wrote: >> On Wed, Sep 17, 2008 at 04:31:17PM +0100, Jonathan del Strother 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 ``` ## Jonathan del Strother, 2008-09-30 23:46 Subject: [PATCH v2] Teach git diff about Objective-C syntax Message-ID: <1222818394-11547-1-git-send-email-jon.delStrother@bestbefore.tv> URL: https://gitlist.dev/e/1222818394-11547-1-git-send-email-jon.delStrother%40bestbefore.tv In-Reply-To: <57518fd10809171630v97485aalcc5089f96082c0fc@mail.gmail.com> ``` Add support for recognition of Objective-C class & instance methods, C functions, and class implementation/interfaces. Signed-off-by: Jonathan del Strother --- 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 ``` ## Brandon Casey, 2008-10-01 00:52 Subject: Re: [PATCH v2] Teach git diff about Objective-C syntax Message-ID: URL: https://gitlist.dev/e/mJtumbyBuR0sRaFjwb6O5_D_ps0fVfz6APTAAjYMixV0MvzP8nEZRw%40cipher.nrlssc.navy.mil In-Reply-To: <1222818394-11547-1-git-send-email-jon.delStrother@bestbefore.tv> ``` 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 > --- > 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 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 s to match are responsible for eliminating any 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; ``` ## Brandon Casey, 2008-10-01 19:28 Subject: [PATCH] xdiff-interface.c: strip newline (and cr) from line before pattern matching Message-ID: URL: https://gitlist.dev/e/o5dqpNECJusQHKCTvRWiIqN2ZJ7w-fyC-0vM99FajJIgLsOwP3RNug%40cipher.nrlssc.navy.mil In-Reply-To: ``` POSIX doth sayeth: "In the regular expression processing described in IEEE Std 1003.1-2001, the 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 s to match are responsible for eliminating any 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 --- 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(®->re, line_buffer, 2, pmatch, 0)) { -- 1.6.0.2.323.g7c850 ``` ## Johannes Schindelin, 2008-10-01 20:05 Subject: Re: [PATCH] xdiff-interface.c: strip newline (and cr) from line before pattern matching Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0810012204280.22125%40pacific.mpi-cbg.de.mpi-cbg.de In-Reply-To: ``` 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 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 s to match are responsible for eliminating any > 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 ``` ## Jonathan del Strother, 2008-10-02 10:29 Subject: Re: [PATCH] xdiff-interface.c: strip newline (and cr) from line before pattern matching Message-ID: <57518fd10810020329w257d91f1u3e2205fdaed888eb@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10810020329w257d91f1u3e2205fdaed888eb%40mail.gmail.com In-Reply-To: ``` On Wed, Oct 1, 2008 at 8:28 PM, Brandon Casey wrote: > POSIX doth sayeth: > > "In the regular expression processing described in IEEE Std 1003.1-2001, > the 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 s to match are responsible for eliminating any > 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 > --- > > > 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(®->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. ``` ## Jonathan del Strother, 2008-10-02 10:40 Subject: Re: [PATCH v2] Teach git diff about Objective-C syntax Message-ID: <57518fd10810020340t15d9441av521b7ef088ea09ca@mail.gmail.com> URL: https://gitlist.dev/e/57518fd10810020340t15d9441av521b7ef088ea09ca%40mail.gmail.com In-Reply-To: <1222818394-11547-1-git-send-email-jon.delStrother@bestbefore.tv> ``` On Wed, Oct 1, 2008 at 12:46 AM, 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 > --- > 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? ```