From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johannes Sixt Subject: Re: [RFC/PATCH] diff: simplify cpp funcname regex Date: Fri, 14 Mar 2014 07:56:46 +0100 Message-ID: <5322A82E.9060808@viscovery.net> References: <20140305003639.GA9474@sigill.intra.peff.net> <5316D922.9010501@viscovery.net> <20140306212835.GA11743@sigill.intra.peff.net> <531973D9.9070803@viscovery.net> <20140314035457.GA31906@sigill.intra.peff.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: git@vger.kernel.org, Thomas Rast To: Jeff King X-From: git-owner@vger.kernel.org Fri Mar 14 07:57:24 2014 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1WOM3W-0003Xb-Ce for gcvg-git-2@plane.gmane.org; Fri, 14 Mar 2014 07:57:22 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755328AbaCNG4x (ORCPT ); Fri, 14 Mar 2014 02:56:53 -0400 Received: from so.liwest.at ([212.33.55.23]:45265 "EHLO so.liwest.at" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755234AbaCNG4w (ORCPT ); Fri, 14 Mar 2014 02:56:52 -0400 Received: from [81.10.228.254] (helo=theia.linz.viscovery) by so.liwest.at with esmtpa (Exim 4.80.1) (envelope-from ) id 1WOM2x-0005b0-CC; Fri, 14 Mar 2014 07:56:47 +0100 Received: from [192.168.1.95] (J6T.linz.viscovery [192.168.1.95]) by theia.linz.viscovery (Postfix) with ESMTP id 20B2F16613; Fri, 14 Mar 2014 07:56:47 +0100 (CET) User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 In-Reply-To: <20140314035457.GA31906@sigill.intra.peff.net> X-Enigmail-Version: 1.6 X-Spam-Score: -1.0 (-) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: Am 3/14/2014 4:54, schrieb Jeff King: > 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