summaryrefslogtreecommitdiff
path: root/builtin/blame.c
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2017-01-06 04:20:51 (GMT)
committerJunio C Hamano <gitster@pobox.com>2017-01-08 03:34:54 (GMT)
commit4e768329847c8226a230406041fe249adea245cf (patch)
tree798ef63b8c65d7d7605ff7a9c076da29aa0aa487 /builtin/blame.c
parented58d8088b570e7629bfc94b87e433f05229ef3c (diff)
downloadgit-4e768329847c8226a230406041fe249adea245cf.zip
git-4e768329847c8226a230406041fe249adea245cf.tar.gz
git-4e768329847c8226a230406041fe249adea245cf.tar.bz2
blame: output porcelain "previous" header for each file
It's possible for content currently found in one file to have originated in two separate files, each of which may have been modified in some single older commit. The --porcelain output generates an incorrect "previous" header in this case, whereas --line-porcelain gets it right. The problem is that the porcelain output tries to omit repeated details of commits, and treats "previous" as a property of the commit, when it is really a property of the blamed block of lines. Let's look at an example. In a case like this, you might see this output from --line-porcelain: SOME_SHA1 1 1 1 author ... committer ... previous SOME_SHA1^ file_one filename file_one ...some line content... SOME_SHA1 2 1 1 author ... committer ... previous SOME_SHA1^ file_two filename file_two ...some different content.... The "filename" fields tell us that the two lines are from two different files. But notice that the filename also appears in the "previous" field, which tells us where to start a re-blame. The second content line never appeared in file_one at all, so we would obviously need to re-blame from file_two (or possibly even some other file, if had just been renamed to file_two in SOME_SHA1). So far so good. Now here's what --porcelain looks like: SOME_SHA1 1 1 1 author ... committer ... previous SOME_SHA1^ file_one filename file_one ...some line content... SOME_SHA1 2 1 1 filename file_two ...some different content.... We've dropped the author and committer fields from the second line, as they would just be repeats. But we can't omit "filename", because it depends on the actual block of blamed lines, not just the commit. This is handled by emit_porcelain_details(), which will show the filename either if it is the first mention of the commit _or_ if the commit has multiple paths in it. But we don't give "previous" the same handling. It's written inside emit_one_suspect_detail(), which bails early if we've already seen that commit. And so the output above is wrong; a reader would assume that the correct place to re-blame line two is from file_one, but that's obviously nonsense. Let's treat "previous" the same as "filename", and show it fresh whenever we know we are in a confusing case like this. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin/blame.c')
-rw-r--r--builtin/blame.c23
1 files changed, 14 insertions, 9 deletions
diff --git a/builtin/blame.c b/builtin/blame.c
index 1fccbe6..d4f3ce9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1700,13 +1700,23 @@ static void get_commit_info(struct commit *commit,
}
/*
+ * Write out any suspect information which depends on the path. This must be
+ * handled separately from emit_one_suspect_detail(), because a given commit
+ * may have changes in multiple paths. So this needs to appear each time
+ * we mention a new group.
+ *
* To allow LF and other nonportable characters in pathnames,
* they are c-style quoted as needed.
*/
-static void write_filename_info(const char *path)
+static void write_filename_info(struct origin *suspect)
{
+ if (suspect->previous) {
+ struct origin *prev = suspect->previous;
+ printf("previous %s ", oid_to_hex(&prev->commit->object.oid));
+ write_name_quoted(prev->path, stdout, '\n');
+ }
printf("filename ");
- write_name_quoted(path, stdout, '\n');
+ write_name_quoted(suspect->path, stdout, '\n');
}
/*
@@ -1735,11 +1745,6 @@ static int emit_one_suspect_detail(struct origin *suspect, int repeat)
printf("summary %s\n", ci.summary.buf);
if (suspect->commit->object.flags & UNINTERESTING)
printf("boundary\n");
- if (suspect->previous) {
- struct origin *prev = suspect->previous;
- printf("previous %s ", oid_to_hex(&prev->commit->object.oid));
- write_name_quoted(prev->path, stdout, '\n');
- }
commit_info_destroy(&ci);
@@ -1760,7 +1765,7 @@ static void found_guilty_entry(struct blame_entry *ent,
oid_to_hex(&suspect->commit->object.oid),
ent->s_lno + 1, ent->lno + 1, ent->num_lines);
emit_one_suspect_detail(suspect, 0);
- write_filename_info(suspect->path);
+ write_filename_info(suspect);
maybe_flush_or_die(stdout, "stdout");
}
pi->blamed_lines += ent->num_lines;
@@ -1884,7 +1889,7 @@ static void emit_porcelain_details(struct origin *suspect, int repeat)
{
if (emit_one_suspect_detail(suspect, repeat) ||
(suspect->commit->object.flags & MORE_THAN_ONE_PATH))
- write_filename_info(suspect->path);
+ write_filename_info(suspect);
}
static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,