"Aneesh Kumar" <aneesh.kumar@xxxxxxxxx> writes: > In that case the heading is wrong. It should be something other than > Filenum. You mean "FileLine"? > my $fileview = Gtk2::SimpleList->new( > 'Commit' => 'text', > + 'OrigLine' => 'text', > 'CommitInfo' => 'text', > 'FileLine' => 'text', > 'Data' => 'text' I do not have strong feeling to defend what Jeff originally did, especially as this is only a sample program, whose primary purpose is to demonstrate how the incremental display can be used. Having said that, I think the behaviour of the original makes quite a lot of sense. HEAD commit starts out to be tentatively blamed for everything, and the filename and the line number in that HEAD commit are shown for everything at the beginning, and as the processing progresses, the labels for the ones whose truely guilty party are known are updated to show the guilty commit, the filename from that guilty commit and the line number in that guilty commit. I think the label "FileLine" reflects what it is showing quite well. As Linus mentioned, the screen real estate is already wasted by too much metainfomation. Although I do not care too much about the UI issue in it since this is only a sample program, showing the line number for each line in the final image ($lno) to waste more space feels doubly wrong. By the way, telling git-gui to annotate revision.h with the attached patch was fun to watch. -- >8 -- [PATCH] Louder git-blame --incremental This patch takes the ability for --incremental to monitor the process of "HEAD starts out to be tentatively blamed for everything, and then blame is passed onto the parents" to the extreme. As blame is passed on to parents, incremental output gives the information for tentatively blamed commits. Signed-off-by: Junio C Hamano <junkio@xxxxxxx> --- builtin-blame.c | 22 ++++++++++++++++------ 1 files changed, 16 insertions(+), 6 deletions(-) diff --git a/builtin-blame.c b/builtin-blame.c index 3033e9b..107524c 100644 --- a/builtin-blame.c +++ b/builtin-blame.c @@ -549,6 +549,8 @@ static void free_patch(struct patch *p) free(p); } +static void found_guilty_entry(struct blame_entry *ent, int final); + /* * Link in a new blame entry to the scorebord. Entries that cover the * same line range have been removed from the scoreboard previously. @@ -682,13 +684,16 @@ static void split_blame(struct scoreboard *sb, new_entry = xmalloc(sizeof(*new_entry)); memcpy(new_entry, &(split[1]), sizeof(struct blame_entry)); add_blame_entry(sb, new_entry); + found_guilty_entry(new_entry, 0); } - else if (!split[0].suspect && !split[2].suspect) + else if (!split[0].suspect && !split[2].suspect) { /* * The parent covers the entire area; reuse storage for * e and replace it with the parent. */ dup_entry(e, &split[1]); + found_guilty_entry(e, 0); + } else if (split[0].suspect) { /* me and then parent */ dup_entry(e, &split[0]); @@ -696,10 +701,12 @@ static void split_blame(struct scoreboard *sb, new_entry = xmalloc(sizeof(*new_entry)); memcpy(new_entry, &(split[1]), sizeof(struct blame_entry)); add_blame_entry(sb, new_entry); + found_guilty_entry(new_entry, 0); } else { /* parent and then me */ dup_entry(e, &split[1]); + found_guilty_entry(e, 0); new_entry = xmalloc(sizeof(*new_entry)); memcpy(new_entry, &(split[2]), sizeof(struct blame_entry)); @@ -1359,11 +1366,13 @@ static void write_filename_info(const char *path) * The blame_entry is found to be guilty for the range. Mark it * as such, and show it in incremental output. */ -static void found_guilty_entry(struct blame_entry *ent) +static void found_guilty_entry(struct blame_entry *ent, int final) { - if (ent->guilty) - return; - ent->guilty = 1; + if (final) { + if (ent->guilty) + return; + ent->guilty = 1; + } if (incremental) { struct origin *suspect = ent->suspect; @@ -1385,6 +1394,7 @@ static void found_guilty_entry(struct blame_entry *ent) printf("summary %s\n", ci.summary); if (suspect->commit->object.flags & UNINTERESTING) printf("boundary\n"); + printf("tentative %s\n", final ? "yes" : "no"); } write_filename_info(suspect->path); } @@ -1432,7 +1442,7 @@ static void assign_blame(struct scoreboard *sb, struct rev_info *revs, int opt) /* Take responsibility for the remaining entries */ for (ent = sb->ent; ent; ent = ent->next) if (!cmp_suspect(ent->suspect, suspect)) - found_guilty_entry(ent); + found_guilty_entry(ent, 1); origin_decref(suspect); if (DEBUG) /* sanity */ - 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