Re: misleading diff-hunk header
- From
- Tim Chase <git@tim.thechases.com>
- Date
- Aug 24, 2012, 15:05 UTC
- Message-ID
- <50379827.3010709@tim.thechases.com>
- In-Reply-To
- <20120824142908.GA15162@sigill.intra.peff.net>
On 08/24/12 09:29, Jeff King wrote:
Show 16 quoted lines
> 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. ...
Show 21 quoted lines
>> 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