Re: [BUG] difference of info from diff and blame

[Semyon Kirnosenko <semyon.kirnosenko@gmail.com>]

11.01.2011 16:40, Thomas Rast пишет:
> Semyon Kirnosenko wrote:
> [Word wrap fixed.]
>>
>> I have jquery repo (https://github.com/jquery/jquery.git)
>> Let's get blame for some file in some revision:
>> git blame -l -s 2ad223aedd1f93c783d98d60adc9fda3bdfbb4b6 -- src/event/event.js
>> According to blame, line 127 was added in revision
>> 2ad223aedd1f93c783d98d60adc9fda3bdfbb4b6.
>
> The surrounding context (with authorship and some whitespace snipped,
> obviously it's always the same) is
>
> 2ad223ae (124)		
> 2ad223ae (125)		// Pass along a fake event
> 2ad223ae (126)		data.unshift( this.fix({ type: type, target: element }) );
> 2ad223ae (127)
> 2ad223ae (128)		// Trigger the event
> 2ad223ae (129)		if ( (val = this.handle.apply( element, data )) !== false )
> 2ad223ae (130)			this.triggered = true;
>
>
>> Let's get diff for that revision:
>> git diff-tree -p 2ad223aedd1f93c783d98d60adc9fda3bdfbb4b6 -- src/event/event.js
>> We can see this:
>> @@ -105,19 +120,16 @@ jQuery.event = {
>>
>>    		// Handle triggering a single element
>>    		else {
>> -			var handler = element["on" + type ], val,
>> -				fn = jQuery.isFunction( element[ type
>> +			var val, ret, fn = jQuery.isFunction( element
>> +			
>> +			// Pass along a fake event
>> +			data.unshift( this.fix({ type: type, target:
>>
> [this blank context line is line 127 in the postimage]
>> -			if ( handler ) {
>>
>> As you can see line 127 is not marked with '+' char, which means it was
>> not added in this revision. But blame sad otherwise.
>
> git-blame internally runs a diff with no context lines to "pass
> blame".  On all lines in this diff, the current commit can pass on
> blame to the parent, thus avoiding having to take it for itself.
>
> And indeed, running
>
>    git show -U0 2ad223ae -- src/event/event.js
>
> in your repository gives a hunk
>
> @@ -108,11 +123,8 @@ jQuery.event = {
> -			var handler = element["on" + type ], val,
> -				fn = jQuery.isFunction( element[ type ] );
> -
> -			if ( handler ) {
> -				// Pass along a fake event
> -				data.unshift( this.fix({ type: type, target: element }) );
> -	
> -				// Trigger the event
> -				if ( (val = handler.apply( element, data )) !== false )
> -					this.triggered = true;
> -			}
> +			var val, ret, fn = jQuery.isFunction( element[ type ] );
> +			
> +			// Pass along a fake event
> +			data.unshift( this.fix({ type: type, target: element }) );
> +
> +			// Trigger the event
> +			if ( (val = this.handle.apply( element, data )) !== false )
> +				this.triggered = true;
>
> So I would tend to agree with the blame implementation.  If anything
> it's a bug in git-diff when not using any context.
>
> Do you have a reproduction recipe that exhibits the flaw on a
> non-whitespace line?  I'm not up to speed on the diff implementation
> (maybe someone else can help?), but I wouldn't be too surprised if it
> had heuristics that put lines consisting only of whitespace at a lower
> importance than "actual" lines.
>

All cases I have seen were about whitespace lines.

-- 
Regards,
Semyon Kirnosenko
kirnosenko@mail.ru