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

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Mar 14, 2014, 06:56 UTC
Message-ID: <5322A82E.9060808@viscovery.net>
In-Reply-To: <20140314035457.GA31906@sigill.intra.peff.net>

Am 3/14/2014 4:54, schrieb Jeff King:

Show 26 quoted lines

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

We are in agreement here. So, let's base a decision on the implications on git grep.

> ... but I'm not sure if I understand all
> of the implications for "git grep". Can you give some concrete examples?

Consider this code:

  void above()
  {}
  static int Y;
  static int A;
  int bar()
  {
    return X;
  }
  void below()
  {}

When you 'git grep --function-context X', then you get this output with the current pattern, you proposal, and my proposal (file name etc omitted for brevity):

  int bar()
  {
    return X;
  }

because we start the context at the last hunk header pattern above *or including the matching line* (and write it in the output), and we stop at the next hunk header pattern below the match (and do not write it in the output).

When you 'git grep --function-context Y', what do you want to see? With the current pattern, and with your pattern that forbids semicolon we get:

  void above()
  {}
  static int Y;
  static int A;

and with my simple pattern, which allows semicolon, we get merely

  static int Y;

because the line itself is a hunk header (and we do not look back any further) and the next line is as well. That is not exactly "function context", and that is what I'm a bit worried about.

-- Hannes