Re: misleading diff-hunk header

[Tim Chase <git@tim.thechases.com>]

On 08/24/12 09:29, Jeff King wrote:
> On Tue, Aug 21, 2012 at 10:52:03AM -0700, Junio C Hamano wrote:
> 
>>>>> diff.{type}.xfuncname seems to start searching backwards in
>>>>> from the beginning of the hunk, not the first differing line.
>>>> [...]
>>>>>   @@ -4,4 +4,5 @@ int call_me(int maybe)
>>>>>
>>>>>    int main()
>>>>>    {
>>>>>   +  return 0;
>>>>>    }
>>>>>
>>>>> misleadingly suggesting that the change occurred in the call_me()
>>>>> function, rather than in main()
>>>>
>>>> I think that's intentional, and matches what 'diff -p' does. 
...
>>  xdiff/xemit.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
>> index 277e2ee..5f9c0e0 100644
>> --- a/xdiff/xemit.c
>> +++ b/xdiff/xemit.c
>> @@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
>>  
>>  		if (xecfg->flags & XDL_EMIT_FUNCNAMES) {
>>  			long l;
>> -			for (l = s1 - 1; l >= 0 && l > funclineprev; l--) {
>> +			for (l = s1; l >= 0 && l > funclineprev; l--) {
>>  				const char *rec;
>>  				long reclen = xdl_get_rec(&xe->xdf1, l, &rec);
>>  				long newfunclen = ff(rec, reclen, funcbuf,
> 
> In the case we were discussing then, the modified function started on
> the first line of context. But as Tim's example shows, it doesn't
> necessarily have to. I think it would make more sense to start counting
> from the first modified line.

Junio mentions that it matches the "diff -p" output, though I'd
consider that a bug in diff as well, since the diff(1) man/info
pages state "-p  Show which C function each change is in."  In the
above (both with "diff -p" and with git), the change was clearly in
main() but it's not showing main().  Documented behavior and
implemented behavior conflict.

Starting at the first differing line rather than the first line of
context in the hunk would ameliorate this.  It doesn't address what
happens if multiple functions were changed in the same hunk, but at
least it becomes correct for the first one.  More complex code might
be doable to split hunks if an xfuncname match occurs between two
disjoint changes in the same hunk.  But for my purposes here, the
above should suffice.

-tkc