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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]