git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Ericsson <ae@op5.se>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [nit] diff func headers ignore context
Date: Wed, 14 Sep 2011 12:13:25 +0200	[thread overview]
Message-ID: <4E707E45.5070102@op5.se> (raw)
In-Reply-To: <20110913220421.GA24549@sigill.intra.peff.net>

On 09/14/2011 12:04 AM, Jeff King wrote:
> On Tue, Sep 13, 2011 at 05:58:25PM -0400, Jeff King wrote:
> 
>> @@ -609,26 +610,23 @@ int finish_async(struct async *async)
>>   int run_hook(const char *index_file, const char *name, ...)
>>   {
>>   	struct child_process hook;
>> -	const char **argv = NULL, *env[2];
>> +	struct argv_array argv = ARGV_ARRAY_INIT;
> 
> I find this diff function header pretty confusing. Of course we're not
> in finish_async, as you can see by the fact that the context contains
> the start of run_hook.
> 
> I don't think this is something that can be solved with xfuncname
> config; we would have to teach xdiff to look at context lines when
> picking a header line.
> 
> Am I the only one who finds this confusing? Can anyone think of a reason
> to keep showing finish_async in this example?
> 

I've never thought about the problem as I tend to read changes and
context before header.

Current behaviour makes sense if the closing brace of the previous
function is inside the context, so if we're going to change this,
we'd either have to teach xfuncname() (or whatever we'll be using)
about indentation to make it handle bannerstyle[1] indentation, or
take any closing brace before a new first-level indentation as a
sign to close the previous function. OTOH we could just ignore
bannerstyle indent, as it's sort of bizarre and not very widely
used, but we'd still have to handle the case of the previous
function being closed inside the context.

Personally, I don't think it's worth it, but I certainly see your
point.


*1. Bannerstyle indent looks like this, for those who aren't
    familiar with this very confusing style.
int prefixcmp(char *haystack, char *needle) {
	return strncmp(haystack, needle, strlen(needle));
	}

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

  reply	other threads:[~2011-09-14 10:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-13 21:50 [PATCH 0/7] create argv_array API Jeff King
2011-09-13 21:57 ` [PATCH 1/7] add sha1_array API docs Jeff King
2011-09-13 21:57 ` [PATCH 2/7] quote.h: fix bogus comment Jeff King
2011-09-13 21:57 ` [PATCH 3/7] refactor argv_array into generic code Jeff King
2011-09-14  5:54   ` Christian Couder
2011-09-14 23:18     ` Jeff King
2011-09-14 18:42   ` Junio C Hamano
2011-09-14 18:51     ` Junio C Hamano
2011-09-13 21:58 ` [PATCH 4/7] quote: provide sq_dequote_to_argv_array Jeff King
2011-09-13 21:58 ` [PATCH 5/7] bisect: use argv_array API Jeff King
2011-09-13 21:58 ` [PATCH 6/7] checkout: " Jeff King
2011-09-13 21:58 ` [PATCH 7/7] run_hook: " Jeff King
2011-09-13 22:04   ` [nit] diff func headers ignore context Jeff King
2011-09-14 10:13     ` Andreas Ericsson [this message]
2011-09-14 19:01     ` Junio C Hamano
2011-09-14 18:54   ` [PATCH 7/7] run_hook: use argv_array API Junio C Hamano
2011-09-14 18:56     ` Jeff King
2011-09-14 20:01       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E707E45.5070102@op5.se \
    --to=ae@op5.se \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).