-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Continue/abort a conflicted cherry-pick or revert #4441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Continue/abort a conflicted cherry-pick or revert #4441
Conversation
253b2f7
to
82ebc80
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 preferencesFootnotes
|
82ebc80
to
0badb30
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. One thing
pkg/commands/types/enums/enums.go
Outdated
REBASE_MODE_REBASING | ||
REBASE_MODE_MERGING | ||
// this means we're neither rebasing nor merging, cherry-picking, or reverting | ||
WORKING_TREE_STATE_NONE WorkingTreeState = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like how we're using the same type (WorkingTreeState
) for the bits as for the bitfield. Naively I would expect that I could directly check if workingTreeState == WORKING_TREE_STATE_CHERRY_PICKING
but that's really only going to return true if we're only cherry picking and not also merging. I also can't do if workingTreeState == WORKING_TREE_STATE_CHERRY_PICKING || workingTreeState == WORKING_TREE_STATE_REBASING
to ask if it's either cherry picking or rebasing.
So I'm wondering, how about we just have a struct of booleans:
type WorkingTreeState struct {
rebasing bool
merging bool
cherryPicking bool
reverting bool
}
And then define methods on that struct. We can still have the .Effective()
method which returns the enum value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0badb30
to
5d957ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
5d957ce
to
2d897e1
Compare
2d897e1
to
a7ba781
Compare
When debugging an integration test that involves some behind-the-scenes rebasing, the daemon lazygit would also wait for a debugger to attach. This is very confusing because the test seems to hang; once you figured out what's going on, it's inconvenient because you need to attach a debugger to the daemon every time you debug the test. Now, it would sometimes be useful to be able to debug the daemon itself (whether inside an integration test, or during normal usage), and I have often wished to be able to do that. We might introduce an additional env var (and command-line option?) to enable this; but that's out of scope here.
All test cases set it to enums.REBASE_MODE_NONE, so we can simplify things a little bit by hard-coding that. This makes the changes in the following commits a little easier.
We want to get rid of RebaseMode, and using the more general WorkingTreeState will later allow us to also show cherry-pick or revert todos.
- Remove REBASE_MODE_NORMAL. It is not the "normal" mode anyway, rather a legacy mode; we have removed support for it in eb0f7e3, so there's no point in representing it in the enum. - Remove distinction between REBASE_MODE_REBASING and REBASE_MODE_INTERACTIVE; these are the same now. - Rename StatusCommands.IsInInteractiveRebase to IsInRebase. - Remove StatusCommands.RebaseMode; use StatusCommands.IsInRebase instead.
We're about to add more possible values (reverting and cherry-picking), so working tree state seems like a more suitable name.
The situation is that you perform a rebase, and one of the commits becomes empty during the rebase, in a way that couldn't be predicted before the rebase started. To explain: git rebase has some logic where it immediately discards commits if it can tell that they already exist in the branch being rebased onto; it does this by looking at the patch ids of the commits to work out which commits already exist upstream. But for those commits that become empty even though there isn't a corresponding commit upstream, git stops with an error, and lazygit detects this (in CheckMergeOrRebaseWithRefreshOptions) and automatically continues the rebase. This all works fine; I'm adding this test because I almost broke this during development of this branch, so I'm adding it to guard against regressions.
It looks like enums.go was supposed to be file that collects a bunch of enums, but actually there's only one in there, and since it has methods, it deserves to be in a file of its own, named after the type.
When you are in the middle of a rebase, and you cherry-pick a commit which conflicts, it helps to be clear on whether you are prompted to continue the cherry-pick or the rebase.
This works already (except that the "you are here" marker is not right yet, but that's an issue for another PR), just add a test that verifies it.
This way we get the usual menu for viewing the conflicts or aborting, like for rebases.
a7ba781
to
90db796
Compare
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.48.0` -> `v0.50.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary> ### [`v0.50.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.50.0) [Compare Source](jesseduffield/lazygit@v0.49.0...v0.50.0) <!-- Release notes generated using configuration in .github/release.yml at v0.50.0 --> #### What's Changed ##### Enhancements 🔥 - Continue/abort a conflicted cherry-pick or revert by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4441 - Show todo items for pending cherry-picks and reverts by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4442 - Use "git cherry-pick" for implementing copy/paste of commits by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4443 - Allow reverting a range of commits by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4444 - Section headers for rebase todos and actual commits by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4463 - Focus the main view by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4429 - Auto-forward main branches after fetching by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4493 - Add new command "Move commits to new branch" by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3876 - Strip the '+' and '-' characters when copying parts of a diff to the clipboard by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4519 - Reduce memory consumption of graph (pipe sets) by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4498 ##### Fixes 🔧 - Fix truncation of branches when scrolling branches panel to the left by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4483 - Fix nvim-remote commands for fish shell by [@​SavingFrame](https://github.com/SavingFrame) in jesseduffield/lazygit#4506 - Disallow creating custom patches when the diff context size is 0 by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4522 - Remove double space between rebase todo and author columns by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4520 ##### Maintenance ⚙️ - Allow closing issues via github actions by [@​jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4515 ##### Docs 📖 - Add Debian installation instructions alongside Ubuntu by [@​jmkim](https://github.com/jmkim) in jesseduffield/lazygit#4501 - fix wording of random tip by [@​dawedawe](https://github.com/dawedawe) in jesseduffield/lazygit#4488 #### New Contributors - [@​jmkim](https://github.com/jmkim) made their first contribution in jesseduffield/lazygit#4501 - [@​SavingFrame](https://github.com/SavingFrame) made their first contribution in jesseduffield/lazygit#4506 - [@​dawedawe](https://github.com/dawedawe) made their first contribution in jesseduffield/lazygit#4488 **Full Changelog**: jesseduffield/lazygit@v0.49.0...v0.50.0 ### [`v0.49.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.49.0) [Compare Source](jesseduffield/lazygit@v0.48.0...v0.49.0) <!-- Release notes generated using configuration in .github/release.yml at v0.49.0 --> #### What's Changed ##### Enhancements 🔥 - Support fish when running shell command by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4350 - Add acme editor preset by [@​rakoo](https://github.com/rakoo) in jesseduffield/lazygit#4360 - Support home and end as alternatives to '<' and '>' by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4396 - Drop the git config cache when getting focus by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4376 - Add a "Content of selected file" entry to the copy menu for commit files by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4341 - Add root node in file tree by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4346 - \[FEAT] Add Recursive Bulk Initialize and Update for Submodules by [@​cesarandr](https://github.com/cesarandr) in jesseduffield/lazygit#4259 - Commit without pre-commit hooks action is independent on prefix by [@​kschweiger](https://github.com/kschweiger) in jesseduffield/lazygit#4374 - Let users define custom icons and color for files on the config file by [@​hasecilu](https://github.com/hasecilu) in jesseduffield/lazygit#4395 - Add "Absolute path" item to the file view's copy menu by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4410 - Allow range drop stashes by [@​gaogao-qwq](https://github.com/gaogao-qwq) in jesseduffield/lazygit#4451 - More navigation keybindings for confirmation panel by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4404 - Resolve non-inline merge conflicts by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4431 - Add `runCommand` function to Go template syntax + add support for templates in git `branchPrefix` setting by [@​ruudk](https://github.com/ruudk) in jesseduffield/lazygit#4438 - Show "(hooks disabled)" in title bar of commit message editor by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4467 - Add a command to select all commits of the current branch by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4448 ##### Fixes 🔧 - Use a waiting status for rewording a non-head commit by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4343 - Fix layout of options view for non-english languages by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4359 - Fix changing language while lazygit is running by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4361 - URL encode gitlab brackets to make consistent with branch names by [@​ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4336 - Fix commitPrefix setting with empty pattern by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4381 - Commit only tracked files in tracked filter view by [@​parthokunda](https://github.com/parthokunda) in jesseduffield/lazygit#4386 - Revert [#​4313](jesseduffield/lazygit#4313) (Skip post-checkout hook when discarding changes) by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4407 - Enhance support for GPG signed tags by [@​ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4394 - Fix checking out a file from a range selection of commits by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4423 - Fix discarding submodule changes in nested folders by [@​brandondong](https://github.com/brandondong) in jesseduffield/lazygit#4317 - Better support for shell aliases by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4385 - Fix hyperlinks in last line of confirmation popups by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4454 - Fix wrong main view content after pressing `e` in a stack of branches by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4450 - fix: update vscode color to logo color by [@​PeterCardenas](https://github.com/PeterCardenas) in jesseduffield/lazygit#4459 - Fix crash with drag selecting and staging by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4480 - Escape special characters in filenames when git-ignoring files by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4475 ##### Maintenance ⚙️ - Fix linter warnings by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4352 - Fix release schedule again by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4364 - Update to go 1.24 by [@​radsaq](https://github.com/radsaq) in jesseduffield/lazygit#4377 - Add an integration test for a config with a negative refspec by [@​radsaq](https://github.com/radsaq) in jesseduffield/lazygit#4382 - Specify a go release minor version by [@​radsaq](https://github.com/radsaq) in jesseduffield/lazygit#4393 - Fix flaky integration test by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4432 - Some code cleanup by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4449 - Bump the minimum required git version to 2.22 by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4439 - Bump go-git, and get rid of github.com/imdario/mergo by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4460 - Skip date check when release worfklow is manually invoked by [@​jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4484 ##### Docs 📖 - Include empty arrays and maps in config docs by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4413 - Filter out deprecated user config fields from generated Config.md by [@​karimkhaleel](https://github.com/karimkhaleel) in jesseduffield/lazygit#4416 - Corrected interactive rebase keybind example in README.md by [@​NewtonChutney](https://github.com/NewtonChutney) in jesseduffield/lazygit#4426 - Update kidpix link in README to active url by [@​ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4425 ##### I18n 🌎 - Update translation files from Crowdin by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4473 #### New Contributors - [@​rakoo](https://github.com/rakoo) made their first contribution in jesseduffield/lazygit#4360 - [@​radsaq](https://github.com/radsaq) made their first contribution in jesseduffield/lazygit#4377 - [@​cesarandr](https://github.com/cesarandr) made their first contribution in jesseduffield/lazygit#4259 - [@​kschweiger](https://github.com/kschweiger) made their first contribution in jesseduffield/lazygit#4374 - [@​NewtonChutney](https://github.com/NewtonChutney) made their first contribution in jesseduffield/lazygit#4426 - [@​gaogao-qwq](https://github.com/gaogao-qwq) made their first contribution in jesseduffield/lazygit#4451 - [@​ruudk](https://github.com/ruudk) made their first contribution in jesseduffield/lazygit#4438 **Full Changelog**: jesseduffield/lazygit@v0.48.0...v0.49.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC4xMS4yIiwidXBkYXRlZEluVmVyIjoiNDAuMTEuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
This is part one of a four part series of PRs that improve the cherry-pick and revert experience.
With this first PR we make it possible to continue or abort a cherry-pick or revert operation, in the same way you can continue or abort a rebase or merge. Currently this is only relevant for revert, because lazygit doesn't use git cherry-pick for copying/pasting commits. This will change in a later PR in this series though, so here we are already preparing for that.
Fixes #1807
go generate ./...
)