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

[Jeff King <peff@peff.net>]

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

> 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).

> > 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