Re: blameview and file line number

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

 



"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

[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]