git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonas Fonseca <fonseca@diku.dk>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: [PATCH] Improve parent blame to detect renames by using the previous information
Date: Sat,  5 Jun 2010 15:56:05 -0400	[thread overview]
Message-ID: <1275767765-8509-1-git-send-email-fonseca@diku.dk> (raw)
In-Reply-To: <20100523075503.GA24598@coredump.intra.peff.net>

>From git commit 96e117099c0e4f7d508eb071f60b6275038f6f37:

 It gives the parent commit of the blamed commit, _and_ a path in that
 parent commit that corresponds to the blamed path --- in short, it is
 the origin that would have been blamed (or passed blame through) for
 the line _if_ the blamed commit did not change that line.

This functionality was released in git version 1.6.3 in 2009-05-06.
---
 NEWS  |    2 +
 tig.c |   99 +++++++++++++++-------------------------------------------------
 2 files changed, 25 insertions(+), 76 deletions(-)

 I finally got some more time to dig around this. What if we simply uses
 the information given by the porcelain output's previous line? It
 handles your simple test case, and navigating in the tig repository. It
 also makes it possible to delete a lot of code.

diff --git a/NEWS b/NEWS
index 190e5dc..499bdbc 100644
--- a/NEWS
+++ b/NEWS
@@ -38,6 +38,8 @@ Bug fixes:
  - Fix unbind to behave as if the keybinding was never defined.
  - Fix unbind to also cover built-in run requests.
  - Fix parsing of unknown keymap names.
+ - Blame view: fix parent blame to detect renames. It uses "previous"
+   line info from the blame porcelain output added in git version 1.6.3.
 
 tig-0.15
 --------
diff --git a/tig.c b/tig.c
index 01f48c3..044da28 100644
--- a/tig.c
+++ b/tig.c
@@ -3977,73 +3977,6 @@ parse_author_line(char *ident, const char **author, struct time *time)
 	}
 }
 
-static bool
-open_commit_parent_menu(char buf[SIZEOF_STR], int *parents)
-{
-	char rev[SIZEOF_REV];
-	const char *revlist_argv[] = {
-		"git", "log", "--no-color", "-1", "--pretty=format:%s", rev, NULL
-	};
-	struct menu_item *items;
-	char text[SIZEOF_STR];
-	bool ok = TRUE;
-	int i;
-
-	items = calloc(*parents + 1, sizeof(*items));
-	if (!items)
-		return FALSE;
-
-	for (i = 0; i < *parents; i++) {
-		string_copy_rev(rev, &buf[SIZEOF_REV * i]);
-		if (!io_run_buf(revlist_argv, text, sizeof(text)) ||
-		    !(items[i].text = strdup(text))) {
-			ok = FALSE;
-			break;
-		}
-	}
-
-	if (ok) {
-		*parents = 0;
-		ok = prompt_menu("Select parent", items, parents);
-	}
-	for (i = 0; items[i].text; i++)
-		free((char *) items[i].text);
-	free(items);
-	return ok;
-}
-
-static bool
-select_commit_parent(const char *id, char rev[SIZEOF_REV], const char *path)
-{
-	char buf[SIZEOF_STR * 4];
-	const char *revlist_argv[] = {
-		"git", "log", "--no-color", "-1",
-			"--pretty=format:%P", id, "--", path, NULL
-	};
-	int parents;
-
-	if (!io_run_buf(revlist_argv, buf, sizeof(buf)) ||
-	    (parents = strlen(buf) / 40) < 0) {
-		report("Failed to get parent information");
-		return FALSE;
-
-	} else if (parents == 0) {
-		if (path)
-			report("Path '%s' does not exist in the parent", path);
-		else
-			report("The selected commit has no parents");
-		return FALSE;
-	}
-
-	if (parents == 1)
-		parents = 0;
-	else if (!open_commit_parent_menu(buf, &parents))
-		return FALSE;
-
-	string_copy_rev(rev, &buf[41 * parents]);
-	return TRUE;
-}
-
 /*
  * Pager backend
  */
@@ -4898,7 +4831,8 @@ struct blame_commit {
 	const char *author;		/* Author of the commit. */
 	struct time time;		/* Date from the author ident. */
 	char filename[128];		/* Name of file. */
-	bool has_previous;		/* Was a "previous" line detected. */
+	char parent_id[SIZEOF_REV];	/* Parent/previous SHA1 ID. */
+	char parent_filename[128];	/* Parent/previous name of file. */
 };
 
 struct blame {
@@ -5097,7 +5031,11 @@ blame_read(struct view *view, char *line)
 		string_ncopy(commit->title, line, strlen(line));
 
 	} else if (match_blame_header("previous ", &line)) {
-		commit->has_previous = TRUE;
+		if (strlen(line) <= SIZEOF_REV)
+			return FALSE;
+		string_copy_rev(commit->parent_id, line);
+		line += SIZEOF_REV;
+		string_ncopy(commit->parent_filename, line, strlen(line));
 
 	} else if (match_blame_header("filename ", &line)) {
 		string_ncopy(commit->filename, line, strlen(line));
@@ -5153,15 +5091,21 @@ check_blame_commit(struct blame *blame, bool check_null_id)
 static void
 setup_blame_parent_line(struct view *view, struct blame *blame)
 {
+	char from[SIZEOF_REF + SIZEOF_STR];
+	char to[SIZEOF_REF + SIZEOF_STR];
 	const char *diff_tree_argv[] = {
-		"git", "diff-tree", "-U0", blame->commit->id,
-			"--", blame->commit->filename, NULL
+		"git", "diff", "--no-textconv", "--no-extdiff", "--no-color",
+			"-U0", from, to, "--", NULL
 	};
 	struct io io;
 	int parent_lineno = -1;
 	int blamed_lineno = -1;
 	char *line;
 
+	snprintf(from, sizeof(from), "%s:%s", opt_ref, opt_file);
+	snprintf(to, sizeof(to), "%s:%s", blame->commit->id,
+		 blame->commit->filename);
+
 	if (!io_run(&io, IO_RD, NULL, diff_tree_argv))
 		return;
 
@@ -5204,10 +5148,13 @@ blame_request(struct view *view, enum request request, struct line *line)
 		break;
 
 	case REQ_PARENT:
-		if (check_blame_commit(blame, TRUE) &&
-		    select_commit_parent(blame->commit->id, opt_ref,
-					 blame->commit->filename)) {
-			string_copy(opt_file, blame->commit->filename);
+		if (!check_blame_commit(blame, TRUE))
+			break;
+		if (!*blame->commit->parent_id) {
+			report("The selected commit has no parents");
+		} else {
+			string_copy_rev(opt_ref, blame->commit->parent_id);
+			string_copy_rev(opt_file, blame->commit->parent_filename);
 			setup_blame_parent_line(view, blame);
 			open_view(view, REQ_VIEW_BLAME, OPEN_REFRESH);
 		}
@@ -5228,7 +5175,7 @@ blame_request(struct view *view, enum request request, struct line *line)
 					"-C", "-M", "HEAD", "--", view->vid, NULL
 			};
 
-			if (!blame->commit->has_previous) {
+			if (!*blame->commit->parent_id) {
 				diff_index_argv[1] = "diff";
 				diff_index_argv[2] = "--no-color";
 				diff_index_argv[6] = "--";
-- 
1.7.1.354.ge64bd

  reply	other threads:[~2010-06-05 19:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-10  8:55 [tig PATCH] fix off-by-one on parent selection Jeff King
2010-05-22 17:19 ` Jonas Fonseca
2010-05-23  7:40   ` Jeff King
2010-05-23  7:55     ` Jeff King
2010-06-05 19:56       ` Jonas Fonseca [this message]
2010-06-05 19:57         ` [TIG PATCH] Improve parent blame to detect renames by using the previous information Jonas Fonseca
2010-06-06 22:35         ` [PATCH] " Jeff King
2010-06-09 16:10           ` Jonas Fonseca
2010-06-10  2:23           ` Jonas Fonseca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1275767765-8509-1-git-send-email-fonseca@diku.dk \
    --to=fonseca@diku.dk \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).