Re: [RFC/PATCH] diff: simplify cpp funcname regex

From: Jeff King <peff@peff.net>
Date: Mar 14, 2014, 03:54 UTC
Message-ID: <20140314035457.GA31906@sigill.intra.peff.net>
In-Reply-To: <531973D9.9070803@viscovery.net>

On Fri, Mar 07, 2014 at 08:23:05AM +0100, Johannes Sixt wrote:

Show 8 quoted lines

> No, I meant lines like
> 
>     static double var;
>    -static int old;
>    +static int new;
> 
> The motivation is to show hints where in a file the change is located:
> Anything that could be of significance for the author should be picked up.

I see. Coupled with what you said below:

> As I said, my motivation is not so much to find a "container", but rather
> a clue to help locate a change while reading the patch text. I can speak
> for myself, but I have no idea what is more important for the majority.

your proposal makes a lot more sense to me, and I think that is really at the center of our discussion. I do not have a gut feeling for which is "more right" (i.e., "container" versus "context").

But given that most of the cases we are discussing are ones where the "diff -p" default regex gets it right (or at least better than) compared to the C regex, I am tempted to say that we should be erring in the direction of simplicity, and just finding interesting lines without worrying about containers (i.e., it argues for your patch).

Show 14 quoted lines

> > Makes sense. I noticed your fix is to look for end-of-line or comments
> > afterwards.  Would it be simpler to just check for a non-colon, like:
> > 
> >   !^[ \t]*[A-Za-z_][A-Za-z_0-9]*:($|[^:])
> 
> I want to match [[:space:]] after the label's colon, because I have lot's
> of C++ files with CRLF, and I need to match the CR. Your more liberal
> pattern would fit as well, although it would pick up a bit field as in
> 
>    struct foo {
>       unsigned
>         flag: 1;
>    -old
>    +new

Thanks, I was having trouble thinking of another good use of a colon, but bitfields are what I was missing. Your pattern is probably better here.

So I am leaning towards your patch, but I'm not sure if I understand all of the implications for "git grep". Can you give some concrete examples?

-Peff