git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [nit] diff func headers ignore context

From
Andreas Ericsson <ae@op5.se>
Date
Sep 14, 2011, 10:13 UTC
Message-ID
<4E707E45.5070102@op5.se>
In-Reply-To
<20110913220421.GA24549@sigill.intra.peff.net>
On 09/14/2011 12:04 AM, Jeff King wrote:
Show 20 quoted lines
> 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.
Previous: Jeff KingNext: Junio C Hamano
Message 14 of 18 in “create argv_array API”
  1. 0/7 create argv_array APIJeff King, Sep 13, 2011
  2. 1/7 add sha1_array API docsJeff King, Sep 13, 2011
  3. 2/7 quote.h: fix bogus commentJeff King, Sep 13, 2011
  4. 3/7 refactor argv_array into generic codeJeff King, Sep 13, 2011
  5. Christian CouderSep 14, 2011
  6. Jeff KingSep 14, 2011
  7. Junio C HamanoSep 14, 2011
  8. Junio C HamanoSep 14, 2011
  9. 4/7 quote: provide sq_dequote_to_argv_arrayJeff King, Sep 13, 2011
  10. 5/7 bisect: use argv_array APIJeff King, Sep 13, 2011
  11. 6/7 checkout: use argv_array APIJeff King, Sep 13, 2011
  12. 7/7 run_hook: use argv_array APIJeff King, Sep 13, 2011
  13. [nit] diff func headers ignore contextJeff King, Sep 13, 2011
  14. Andreas EricssonSep 14, 2011
  15. Junio C HamanoSep 14, 2011
  16. Junio C HamanoSep 14, 2011
  17. Jeff KingSep 14, 2011
  18. Junio C HamanoSep 14, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.