Start blame on the commit passed to a "tig show" commandline#1135
Start blame on the commit passed to a "tig show" commandline#1135koutcher merged 1 commit intojonas:masterfrom
Conversation
|
I found a better approach: get the commid id from the commit line in the diff. It also works when there are several commits in the diff view (e.g. |
b99ba09 to
5aba58a
Compare
|
Yeah that seems better. I used %(commit) which works the same way except it also works for stash objects. |
|
I don't think there is an option in I thought I did but actually it is not so good and neither is the use of view->env->commit because it doesn't work if you go backward in the diff view. |
5aba58a to
4dcb59b
Compare
|
Ok falling back to view-vid if there is no commit line seems to work. |
|
Yes and no. When we fix view->env->commit (let's put it on the todo list, we can reuse bits from the log view), we will still need to look for a commit line to know when we have to use view->vid as view->env->commit can be overwritten by other views. I noticed test/blame/initial-diff-test fails on my Mac because of |
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]
4dcb59b to
2c6029b
Compare
What is an example of that overwriting? Shouldn't view->env->commit (aka %(commit)) always be the commit of the thing at the cursor? |
|
This was typically what happened in your scenario with a stash. When the diff view is opened on the stash, view->env->commit is initialized from the stash view, but it is later overwritten in the blame view and when coming back to the diff view, there is nothing to set view->env->commit back as there is no commit line. |
|
Ah right, so we do just need to fix view->env->commit but that probably requires a commit line. |
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>
Fixes the first issue described in #1127 (comment)