[PATCH] userdiff: allow * between cpp funcname words

The cpp pattern, used for C and C++, would not match the start of a
declaration such as

  static char *prepare_index(int argc,

because it did not allow for * anywhere between the various words that
constitute the modifiers, type and function name.  Fix it.

Signed-off-by: Thomas Rast <redacted>
---

This is a really sneaky one-character bug that I cannot believe went
unnoticed for so long, seeing as there are plenty of instances within
git itself where it matters.


 userdiff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/userdiff.c b/userdiff.c
index 7c983c1..76109da 100644
--- a/userdiff.c
+++ b/userdiff.c

@@ -118,7 +118,7 @@
 	 /* Jump targets or access declarations */
 	 "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n"
 	 /* C/++ functions/methods at top level */
-	 "^([A-Za-z_][A-Za-z_0-9]*([ \t]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[ \t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n"
+	 "^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[ \t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n"
 	 /* compound type at top level */
 	 "^((struct|class|enum)[^;]*)$",
 	 /* -- */

-- 
1.7.8.424.gcb840

On Tue, Dec 06, 2011 at 05:35:08PM +0100, Thomas Rast wrote:

The cpp pattern, used for C and C++, would not match the start of a
declaration such as

  static char *prepare_index(int argc,

because it did not allow for * anywhere between the various words that
constitute the modifiers, type and function name.  Fix it.

Signed-off-by: Thomas Rast <redacted>
---

This is a really sneaky one-character bug that I cannot believe went
unnoticed for so long, seeing as there are plenty of instances within
git itself where it matters.

Looks reasonable to me. You can see the difference, for instance, with:

  git show -U1 3c73a1d

(The -U1 is because of the annoying "we will start looking for the
header at the top of context, not the top of changes" behavior I
mentioned last week).

-Peff

Jeff King wrote:

On Tue, Dec 06, 2011 at 05:35:08PM +0100, Thomas Rast wrote:

quoted

The cpp pattern, used for C and C++, would not match the start of a
declaration such as

  static char *prepare_index(int argc,

because it did not allow for * anywhere between the various words that
constitute the modifiers, type and function name.  Fix it.

Signed-off-by: Thomas Rast <redacted>
---

This is a really sneaky one-character bug that I cannot believe went
unnoticed for so long, seeing as there are plenty of instances within
git itself where it matters.

Looks reasonable to me. You can see the difference, for instance, with:

  git show -U1 3c73a1d

(The -U1 is because of the annoying "we will start looking for the
header at the top of context, not the top of changes" behavior I
mentioned last week).

Actually (sadly) I'll have to revise it.  It doesn't match much of C++
either, and I haven't yet come up with a reasonable regex that
matches, say,

  foo::Bar<int>::t& Baz::operator<<(

which I would call ludicrous, but it's valid C++.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

On Tue, Dec 06, 2011 at 09:17:56PM +0100, Thomas Rast wrote:

quoted

Looks reasonable to me. You can see the difference, for instance, with:

  git show -U1 3c73a1d

(The -U1 is because of the annoying "we will start looking for the
header at the top of context, not the top of changes" behavior I
mentioned last week).

Actually (sadly) I'll have to revise it.  It doesn't match much of C++
either, and I haven't yet come up with a reasonable regex that
matches, say,

  foo::Bar<int>::t& Baz::operator<<(

which I would call ludicrous, but it's valid C++.

Ick, yeah. Maybe it is worth doing the "*" thing for now, and then
worrying about advanced C++ stuff on top as another patch. AFAICT, your
original patch is a strict improvement.

-Peff

Am 06.12.2011 21:19, schrieb Jeff King:

On Tue, Dec 06, 2011 at 09:17:56PM +0100, Thomas Rast wrote:

quoted

quoted

Looks reasonable to me. You can see the difference, for instance, with:

  git show -U1 3c73a1d

(The -U1 is because of the annoying "we will start looking for the
header at the top of context, not the top of changes" behavior I
mentioned last week).

Actually (sadly) I'll have to revise it.  It doesn't match much of C++
either, and I haven't yet come up with a reasonable regex that
matches, say,

  foo::Bar<int>::t& Baz::operator<<(

which I would call ludicrous, but it's valid C++.

Ick, yeah. Maybe it is worth doing the "*" thing for now, and then
worrying about advanced C++ stuff on top as another patch. AFAICT, your
original patch is a strict improvement.

Excuse me, where's the problem? The above example shows this

@@ -105,8 +105,8 @@ char *url_decode(const char *url)
        struct strbuf out = STRBUF_INIT;
-       const char *slash = strchr(url, '/');
+       const char *colon = strchr(url, ':');

...

with current 4cb5d10b. This looks quite correct, no?

-- Hannes

Am 06.12.2011 21:52, schrieb Johannes Sixt:

That's with the default heuristic; try something like this first to turn 
on userdiff:

	$ echo url.c diff=cpp >>.gitattributes

René