Show todo items for pending cherry-picks and reverts #4442
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stefanhaller
force-pushed
the
continue-and-abort-for-revert-and-cherrypick
branch
from
9d95aca to
b3745fa
Compare
stefanhaller
force-pushed
the
show-revert-and-cherry-pick-todos
branch
from
aa23d4d to
cd2e83f
Compare
stefanhaller
force-pushed
the
continue-and-abort-for-revert-and-cherrypick
branch
2 times, most recently
from
253b2f7 to
82ebc80
Compare
stefanhaller
force-pushed
the
show-revert-and-cherry-pick-todos
branch
from
cd2e83f to
a8eefbc
Compare
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
stefanhaller
force-pushed
the
continue-and-abort-for-revert-and-cherrypick
branch
from
82ebc80 to
0badb30
Compare
stefanhaller
force-pushed
the
show-revert-and-cherry-pick-todos
branch
from
a8eefbc to
d88dd37
Compare
|
Wait I forgot I was looking at only a single commit. One moment while I review the rest |
stefanhaller
force-pushed
the
continue-and-abort-for-revert-and-cherrypick
branch
from
0badb30 to
5d957ce
Compare
stefanhaller
force-pushed
the
show-revert-and-cherry-pick-todos
branch
from
d88dd37 to
57d7c9e
Compare
|
Okay LGTM for real this time |
…basing This is equivalent in the current state of the code, but it will no longer be after the next commit, because we will introduce a new status value StatusConflicted. And in a later PR we might add yet another value StatusCherryPicking to distinguish rebase todos from cherry-pick todos; using commit.IsTODO is a safer way to check whether a commit is any of these.
…CT` instead It is useful to see if the conflicted commit was a "pick" or an "edit". What's more, we're about to add support for showing cherry-picks and reverts, and seeing that a conflicted commit was a revert is important because its diff is backwards compared to the diff of the conflicting files in the Files panel.
MergeRebasingCommits already merges the rebasing commits into the commits slice that is passed in, so it doesn't make sense to append the result to commits again. It isn't a problem, but only because commits is always empty.
This works correctly, we are only adding this as a regression test to verify that the change later in this branch doesn't break it.
…rebase This problem can't happen inside of lazygit itself right now, but this will change in the future. It will only happen when you stopped in an interactive rebase on an "edit" entry, and then you perform a revert or cherry-pick consisting of more than one commit; in this situation lazygit will show a conflict although there is none. This is not possible with lazygit yet, as we don't support range-select for reverting, and we don't use `git cherry-pick` for cherry-picking. Both will change in the future, so it's good to fix this bug.
What happens here is that when stopping on an "edit" todo entry, we rely on the assumption that if the .git/rebase-merge/amend file exists, the command was successful, and if it doesn't, there was a conflict. The problem is that when you stop on an edit command, and then run a multi-commit cherry-pick or rebase, this will delete the amend file. You may or may not consider this a bug in git; to work around it, we also check the existence of the rebase-merge/message file, which will be deleted as well by the cherry-pick or revert.
…ting We treat the .git/sequencer/todo file as read-only. Technically it seems it would be possible to treat it as modifiable in the same way as .git/rebase-merge/git-rebase-todo, effectively turning a cherry-pick or revert that stops at a conflict into an interactive rebase; however, git itself doesn't allow this (there is no "git cherry-pick --edit-todo"), so it seems safer not to rely on it. Theoretically it would be possible to allow modifying the rebase todos when a cherry-pick or revert conflicts in the middle of a rebase. However, it would introduce a bit of complexity to support this, as we would have to be able to distinguish between rebasing todos and cherry-picking/reverting todos, which we currently can't; it could also be a bit error-prone as far as edge cases are concerned. And it's really a pretty uncommon situation, so it doesn't seem worth it, and we just forbid all modifications to todos whenever we are cherry-picking or reverting.
stefanhaller
force-pushed
the
continue-and-abort-for-revert-and-cherrypick
branch
from
5d957ce to
2d897e1
Compare
stefanhaller
force-pushed
the
show-revert-and-cherry-pick-todos
branch
from
57d7c9e to
c1143a2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is part two of a four part series of PRs that improve the cherry-pick and revert experience.
With this PR we include pending cherry-picks and reverts in the commit list (like rebase todos) when a cherry-pick or revert stops with conflicts; also, we show the conflicting item in the list like we do with conflicting rebase todos.
As with the previous PR, this is not really very useful yet because you can't revert a range of commits, and we don't use git cherry-pick for copy/paste. Both of these will change in later PRs in this series, so again this is preparation for that.
go generate ./...)