Skip to content

Commit

Permalink
Fix blame when opened from an initial diff view (#1135)
Browse files Browse the repository at this point in the history
When running "tig show 529182c -- README.adoc", blames were started
on HEAD instead of 529182c.  Fix this by reading the commit line of
the displayed diff.  This also works when multiple commits are shown
("tig show --all").

Using %(commit) instead of parsing the commit line did unfortunately
not work because, because %(commit) can contain the wrong SHA for when
returning from a blame view to a stash view, see test/blame/stash-test.

Sadly, stashes don't contain commit lines, so we need to find out
their SHAs in another way. Once we fix %(commit), we can use it,
but for now this commit adds a workaround to use the old
"view->vid", which seems to be set in the cases where display
the output from "git stash show".

Also, when blame is requested on a context line, use the current
commit rather than its parent.  This does not change behavior since
a context line can never be blamed on the current commit, but this
is consistent with the stage view, which only uses HEAD for deletions
and the index for others.

[ja: add test and commit message]

Co-authored-by: Thomas Koutcher <thomas.koutcher@online.fr>
  • Loading branch information
krobelus and koutcher authored Oct 4, 2021
1 parent 5a23214 commit 874ee11
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 21 deletions.
39 changes: 18 additions & 21 deletions src/diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,8 @@ diff_get_lineno(struct view *view, struct line *line, bool old)
static enum request
diff_trace_origin(struct view *view, struct line *line)
{
struct line *commit_line = find_prev_line_by_type(view, line, LINE_COMMIT);
char id[SIZEOF_REV];
struct line *diff = find_prev_line_by_type(view, line, LINE_DIFF_HEADER);
struct line *chunk = find_prev_line_by_type(view, line, LINE_DIFF_CHUNK);
const char *chunk_data;
Expand All @@ -645,7 +647,7 @@ diff_trace_origin(struct view *view, struct line *line)
struct blame_header header;
struct blame_commit commit;

if (!diff || !chunk || chunk == line) {
if (!diff || !chunk || chunk == line || diff < commit_line) {
report("The line to trace must be inside a diff chunk");
return REQ_NONE;
}
Expand Down Expand Up @@ -686,27 +688,25 @@ diff_trace_origin(struct view *view, struct line *line)
}
}

if (chunk_marker == '+')
string_copy(ref, view->vid);
if (commit_line)
string_copy_rev_from_commit_line(id, box_text(commit_line));
else
string_format(ref, "%s^", view->vid);
string_copy(id, view->vid);

if (string_rev_is_null(ref)) {
string_ncopy(view->env->file, file, strlen(file));
string_copy(view->env->ref, "");
view->env->goto_lineno = lineno - 1;

} else {
if (!diff_blame_line(ref, file, lineno, &header, &commit)) {
report("Failed to read blame data");
return REQ_NONE;
}
if (chunk_marker == '-')
string_format(ref, "%s^", id);
else
string_copy(ref, id);

string_ncopy(view->env->file, commit.filename, strlen(commit.filename));
string_copy(view->env->ref, header.id);
view->env->goto_lineno = header.orig_lineno - 1;
if (!diff_blame_line(ref, file, lineno, &header, &commit)) {
report("Failed to read blame data");
return REQ_NONE;
}

string_ncopy(view->env->file, commit.filename, strlen(commit.filename));
string_copy(view->env->ref, header.id);
view->env->goto_lineno = header.orig_lineno - 1;

return REQ_VIEW_BLAME;
}

Expand Down Expand Up @@ -791,10 +791,7 @@ diff_request(struct view *view, enum request request, struct line *line)
return diff_common_enter(view, request, line);

case REQ_REFRESH:
if (string_rev_is_null(view->vid))
refresh_view(view);
else
reload_view(view);
reload_view(view);
return REQ_NONE;

default:
Expand Down
28 changes: 28 additions & 0 deletions test/blame/initial-diff-test
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/bin/sh

. libtest.sh
. libgit.sh

steps '
:save-display initial-diff.screen
:20 # Move to a deleted line.
:view-blame
:scroll-right
:save-display blame-deleted-line.screen
'

in_work_dir create_repo_from_tgz "$base_dir/files/scala-js-benchmarks.tgz"

test_tig show a1dcf1a

sed -i.old -n /asInstanceOf/p *.screen

assert_equals 'initial-diff.screen' <<EOF
- val global = js.Dynamic.global.asInstanceOf[js.Dictionary]
+ val global = js.Dynamic.global.asInstanceOf[js.Dictionary[js.Any]]
EOF

# Make sure that we find the commit that introduce the deleted line.
assert_equals 'blame-deleted-line.screen' <<EOF
0500 17x val global = js.Dynamic.global.asInstanceOf[js.Dictionary]
EOF
49 changes: 49 additions & 0 deletions test/blame/stash-test
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#!/bin/sh

. libtest.sh
. libgit.sh

tigrc <<EOF
set blame-view = text
EOF

steps '
:view-stash
:view-diff
:move-last-line
:move-up
:save-display diff.screen
:view-blame
:save-display blame1.screen
:view-close
:view-blame
:save-display blame2.screen
'

git_init

test_setup_work_dir()
{
echo "original line" > file
git add file
git commit -m "Initial commit"
echo "changed line" > file
git stash
}

LINES=3 test_tig

assert_equals 'diff.screen' <<EOF
-original line
[diff] Changes to 'file' - line 9 of 10 90%
EOF

assert_equals 'blame1.screen' <<EOF
original line
[blame] file - line 1 of 1 100%
EOF

assert_equals 'blame2.screen' <<EOF
original line
[blame] file - line 1 of 1 100%
EOF

0 comments on commit 874ee11

Please sign in to comment.