{"thread":{"id":"6578","subject":"blameview and file line number","startedAt":"2007-01-30T07:25:44Z","lastAt":"2007-01-30T15:48:52Z","messageCount":7,"participants":["Aneesh Kumar","Junio C Hamano","Jeff King","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"33008","messageId":"cc723f590701292325k1c85d9edgb9e72ae380fcaad7@mail.gmail.com","threadId":"6578","inReplyTo":null,"subject":"blameview and file line number","fromName":"Aneesh Kumar","fromEmail":"aneesh.kumar@gmail.com","sentAt":"2007-01-30T07:25:44Z","receivedAt":"2007-01-30T07:25:44Z","isPatch":false,"sender":{"key":"aneesh.kumar@gmail.com","avatar":"https://gravatar.com/avatar/0621fc0b2f14ead1e9024382f16053a808c148596da30c1b92572fa075621f68?d=mp&s=160"},"body":"Is it a typo or intentional ? I found the blameview output confusing.\n\n-aneesh\n\n\ndiff --git a/contrib/blameview/blameview.perl b/contrib/blameview/blameview.perl\nindex a55f799..e8bcb1b 100755\n--- a/contrib/blameview/blameview.perl\n+++ b/contrib/blameview/blameview.perl\n@@ -62,7 +62,7 @@ sub flush_blame_line {\n \tfor(my $i = 0; $i < $cnt; $i++) {\n \t\t@{$fileview->{data}->[$lno+$i-1]}[0,1,2] =\n \t\t    (substr($commit, 0, 8), $info,\n-\t\t     $filename . ':' . ($s_lno+$i));\n+\t\t     $filename . ':' . ($lno+$i));\n \t}\n }\n \n"},{"id":"33018","messageId":"7vireo52xs.fsf@assigned-by-dhcp.cox.net","threadId":"6578","inReplyTo":"cc723f590701292325k1c85d9edgb9e72ae380fcaad7@mail.gmail.com","subject":"Re: blameview and file line number","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-30T08:33:35Z","receivedAt":"2007-01-30T08:33:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Aneesh Kumar\" <aneesh.kumar@gmail.com> writes:\n\n> Is it a typo or intentional ? I found the blameview output confusing.\n\nCompare it with gitweb or textual output (blame -n -f).\n\nIt is supposed to show the line number in the original file.\nUsing lno is obviously wrong, as we are not reinventing \"cat -n\"\nhere ;-).\n"},{"id":"33023","messageId":"cc723f590701300112x6493a0eaw153aad1a18cd289a@mail.gmail.com","threadId":"6578","inReplyTo":"7vireo52xs.fsf@assigned-by-dhcp.cox.net","subject":"Re: blameview and file line number","fromName":"Aneesh Kumar","fromEmail":"aneesh.kumar@gmail.com","sentAt":"2007-01-30T09:12:31Z","receivedAt":"2007-01-30T09:12:31Z","isPatch":false,"sender":{"key":"aneesh.kumar@gmail.com","avatar":"https://gravatar.com/avatar/0621fc0b2f14ead1e9024382f16053a808c148596da30c1b92572fa075621f68?d=mp&s=160"},"body":"On 1/30/07, Junio C Hamano <junkio@cox.net> wrote:\n> \"Aneesh Kumar\" <aneesh.kumar@gmail.com> writes:\n>\n> > Is it a typo or intentional ? I found the blameview output confusing.\n>\n> Compare it with gitweb or textual output (blame -n -f).\n>\n> It is supposed to show the line number in the original file.\n> Using lno is obviously wrong, as we are not reinventing \"cat -n\"\n> here ;-).\n\nIn that case the heading is wrong. It should be something other than\nFilenum. How about the patch below. On top of the series i sent you\nbefore.\n\n-aneesh\n\n\ndiff --git a/contrib/blameview/blameview.perl b/contrib/blameview/blameview.perl\nindex 8ad9bcf..9149e35 100755\n--- a/contrib/blameview/blameview.perl\n+++ b/contrib/blameview/blameview.perl\n@@ -29,6 +29,7 @@ my $scrolled_window = Gtk2::ScrolledWindow->new;\n $window->add($scrolled_window);\n my $fileview = Gtk2::SimpleList->new(\n     'Commit' => 'text',\n+    'OrigLine' => 'text',\n     'CommitInfo' => 'text',\n     'FileLine' => 'text',\n     'Data' => 'text'\n@@ -50,7 +51,7 @@ open($fh, '-|', \"git cat-file blob $hash:$fn\")\n \n while(<$fh>) {\n   chomp;\n-  $fileview->{data}->[$.] = ['HEAD', '?', \"$fn:$.\", $_];\n+  $fileview->{data}->[$.] = ['HEAD','?', '?', \"$.\", $_];\n }\n \n my $blame;\n@@ -79,8 +80,8 @@ sub flush_blame_line {\n \n \tfor(my $i = 0; $i < $cnt; $i++) {\n \t\t@{$fileview->{data}->[$lno+$i-1]}[0,1,2] =\n-\t\t    (substr($commit, 0, 8), $info,\n-\t\t     $filename . ':' . ($lno+$i));\n+\t\t    (substr($commit, 0, 8), \n+\t\t     $filename . ':' . ($s_lno+$i), $info, ($lno+$i));\n \t}\n }\n \n"},{"id":"33032","messageId":"7vd54w23zh.fsf@assigned-by-dhcp.cox.net","threadId":"6578","inReplyTo":"cc723f590701300112x6493a0eaw153aad1a18cd289a@mail.gmail.com","subject":"Re: blameview and file line number","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-30T10:39:14Z","receivedAt":"2007-01-30T10:39:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Aneesh Kumar\" <aneesh.kumar@gmail.com> writes:\n\n> In that case the heading is wrong. It should be something other than\n> Filenum.\n\nYou mean \"FileLine\"?\n\n>  my $fileview = Gtk2::SimpleList->new(\n>      'Commit' => 'text',\n> +    'OrigLine' => 'text',\n>      'CommitInfo' => 'text',\n>      'FileLine' => 'text',\n>      'Data' => 'text'\n\nI do not have strong feeling to defend what Jeff originally did,\nespecially as this is only a sample program, whose primary\npurpose is to demonstrate how the incremental display can be\nused.\n\nHaving said that, I think the behaviour of the original makes\nquite a lot of sense.  HEAD commit starts out to be tentatively\nblamed for everything, and the filename and the line number in\nthat HEAD commit are shown for everything at the beginning, and\nas the processing progresses, the labels for the ones whose\ntruely guilty party are known are updated to show the guilty\ncommit, the filename from that guilty commit and the line number\nin that guilty commit.  I think the label \"FileLine\" reflects\nwhat it is showing quite well.\n\nAs Linus mentioned, the screen real estate is already wasted by\ntoo much metainfomation.  Although I do not care too much about\nthe UI issue in it since this is only a sample program, showing\nthe line number for each line in the final image ($lno) to waste\nmore space feels doubly wrong.\n\nBy the way, telling git-gui to annotate revision.h with the\nattached patch was fun to watch.\n\n-- >8 --\n[PATCH] Louder git-blame --incremental\n\nThis patch takes the ability for --incremental to monitor the\nprocess of \"HEAD starts out to be tentatively blamed for\neverything, and then blame is passed onto the parents\" to the\nextreme.  As blame is passed on to parents, incremental output\ngives the information for tentatively blamed commits.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n builtin-blame.c |   22 ++++++++++++++++------\n 1 files changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 3033e9b..107524c 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -549,6 +549,8 @@ static void free_patch(struct patch *p)\n \tfree(p);\n }\n \n+static void found_guilty_entry(struct blame_entry *ent, int final);\n+\n /*\n  * Link in a new blame entry to the scorebord.  Entries that cover the\n  * same line range have been removed from the scoreboard previously.\n@@ -682,13 +684,16 @@ static void split_blame(struct scoreboard *sb,\n \t\tnew_entry = xmalloc(sizeof(*new_entry));\n \t\tmemcpy(new_entry, &(split[1]), sizeof(struct blame_entry));\n \t\tadd_blame_entry(sb, new_entry);\n+\t\tfound_guilty_entry(new_entry, 0);\n \t}\n-\telse if (!split[0].suspect && !split[2].suspect)\n+\telse if (!split[0].suspect && !split[2].suspect) {\n \t\t/*\n \t\t * The parent covers the entire area; reuse storage for\n \t\t * e and replace it with the parent.\n \t\t */\n \t\tdup_entry(e, &split[1]);\n+\t\tfound_guilty_entry(e, 0);\n+\t}\n \telse if (split[0].suspect) {\n \t\t/* me and then parent */\n \t\tdup_entry(e, &split[0]);\n@@ -696,10 +701,12 @@ static void split_blame(struct scoreboard *sb,\n \t\tnew_entry = xmalloc(sizeof(*new_entry));\n \t\tmemcpy(new_entry, &(split[1]), sizeof(struct blame_entry));\n \t\tadd_blame_entry(sb, new_entry);\n+\t\tfound_guilty_entry(new_entry, 0);\n \t}\n \telse {\n \t\t/* parent and then me */\n \t\tdup_entry(e, &split[1]);\n+\t\tfound_guilty_entry(e, 0);\n \n \t\tnew_entry = xmalloc(sizeof(*new_entry));\n \t\tmemcpy(new_entry, &(split[2]), sizeof(struct blame_entry));\n@@ -1359,11 +1366,13 @@ static void write_filename_info(const char *path)\n  * The blame_entry is found to be guilty for the range.  Mark it\n  * as such, and show it in incremental output.\n  */\n-static void found_guilty_entry(struct blame_entry *ent)\n+static void found_guilty_entry(struct blame_entry *ent, int final)\n {\n-\tif (ent->guilty)\n-\t\treturn;\n-\tent->guilty = 1;\n+\tif (final) {\n+\t\tif (ent->guilty)\n+\t\t\treturn;\n+\t\tent->guilty = 1;\n+\t}\n \tif (incremental) {\n \t\tstruct origin *suspect = ent->suspect;\n \n@@ -1385,6 +1394,7 @@ static void found_guilty_entry(struct blame_entry *ent)\n \t\t\tprintf(\"summary %s\\n\", ci.summary);\n \t\t\tif (suspect->commit->object.flags & UNINTERESTING)\n \t\t\t\tprintf(\"boundary\\n\");\n+\t\t\tprintf(\"tentative %s\\n\", final ? \"yes\" : \"no\");\n \t\t}\n \t\twrite_filename_info(suspect->path);\n \t}\n@@ -1432,7 +1442,7 @@ static void assign_blame(struct scoreboard *sb, struct rev_info *revs, int opt)\n \t\t/* Take responsibility for the remaining entries */\n \t\tfor (ent = sb->ent; ent; ent = ent->next)\n \t\t\tif (!cmp_suspect(ent->suspect, suspect))\n-\t\t\t\tfound_guilty_entry(ent);\n+\t\t\t\tfound_guilty_entry(ent, 1);\n \t\torigin_decref(suspect);\n \n \t\tif (DEBUG) /* sanity */\n"},{"id":"33035","messageId":"20070130140650.GB6949@coredump.intra.peff.net","threadId":"6578","inReplyTo":"cc723f590701292325k1c85d9edgb9e72ae380fcaad7@mail.gmail.com","subject":"Re: blameview and file line number","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-01-30T14:06:50Z","receivedAt":"2007-01-30T14:06:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 30, 2007 at 12:55:44PM +0530, Aneesh Kumar wrote:\n\n> Is it a typo or intentional ? I found the blameview output confusing.\n\nThe original output was 0-based, so I suspect that line simply didn't\nget updated when Junio changed the incremental format to be 1-based.\n\n-Peff\n"},{"id":"33036","messageId":"20070130140859.GC6949@coredump.intra.peff.net","threadId":"6578","inReplyTo":"20070130140650.GB6949@coredump.intra.peff.net","subject":"Re: blameview and file line number","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-01-30T14:08:59Z","receivedAt":"2007-01-30T14:08:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 30, 2007 at 09:06:50AM -0500, Jeff King wrote:\n\n> On Tue, Jan 30, 2007 at 12:55:44PM +0530, Aneesh Kumar wrote:\n> \n> > Is it a typo or intentional ? I found the blameview output confusing.\n> \n> The original output was 0-based, so I suspect that line simply didn't\n> get updated when Junio changed the incremental format to be 1-based.\n\nEr, disregard this. Junio explained what's happening already.\n\n-Peff\n"},{"id":"33042","messageId":"20070130154852.GA25950@spearce.org","threadId":"6578","inReplyTo":"7vd54w23zh.fsf@assigned-by-dhcp.cox.net","subject":"Re: blameview and file line number","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-01-30T15:48:52Z","receivedAt":"2007-01-30T15:48:52Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> As Linus mentioned, the screen real estate is already wasted by\n> too much metainfomation.  Although I do not care too much about\n> the UI issue in it since this is only a sample program, showing\n> the line number for each line in the final image ($lno) to waste\n> more space feels doubly wrong.\n\nActually including the final image line number is probably something\nyou want to do in a blame viewer.  When I rework git-gui's blame\nUI I'm going to keep the original line number column, but ditch\neverything else into some sort of cursor-following-floating window\n(Linus' idea).\n\nThe reason is, I'll be looking at a line of code in a 5000 line\nsource file in Eclipse (or vi!) and want to know how it came to be.\nI'll go open a blame, but now I have 5000 lines to scan through.\n\nIf there's line numbers and a scrollbar, I can binary search to\nit relatively quickly.  If there's a text search function, sure I\ncould try to enter part of the symbol to match, but at that point\nI might as well just enter the line number to jump to, especially\nif the symbol appears a few times in that file.\n\nSo yes, the -L option to git-blame is *very* handy on the command\nline.  But I think you already knew that...\n\nI realize that git-gui's blame feature won't be used very often\nby the really hard-core developers on this list (you know who you\nare) as the command line is simply faster, easier to use, and more\npowerful.  Most of the features in git-gui are being created for\npeople who are a tad bit afraid of a command line and prefer to\npoint their way through their life with a small rodent shaped device.\nThose folks need something like a -L that they can make use of.\n\n> By the way, telling git-gui to annotate revision.h with the\n> attached patch was fun to watch.\n\nYes, especially with its current technicolor interface.  :-)\n\nI just had to go run this, and aside from the rather horrible blame\ninterface in git-gui, I'm seeing that git-blame is producing data\nfaster than git-gui can really process it.  This causes the UI to\nflash through huge batches of updates.  Probably would be much more\ninteresting if I disabled the fileevent handler for a few hundred\nms to let the UI catch up.  :-)\n\n-- \nShawn.\n"}]}