-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Clear copied commits selection after pasting (cherry-pick) #3198
Comments
This also bothers me every time I paste commits. I have never had the need to cherry-pick commits to more than one destination, so for me it would be fine to auto-clear on paste. Hiding the selection seems messy to me (both conceptually, and in the code). As for conflicts, I don't care too much, I could live with either 1. or 3. I'd probably vote for 1. as it seems like the most simple one. |
I'd like to add a third option:
It does mean people who want to paste to multiple destinations will still have to press an extra key or two, but it doesn't hide state. |
It sounds like the simplest approach is to just auto-clear on paste no matter what. I like the idea of being able to reselect previous commits, but I'm happy to wait and see if anybody actually needs it before implementing it |
It can be tedious after each cherry-pick opearation to clear the selection by pressing escape in order for lazygit to stop displaying info about copied commits. Also, it seems to be a rare case to cherry-pick commits to more than one destination. The simplest solution to address this issue is to clear the selection upon paste, including merge conflict scenario. Previously discussed in #3198.
I often need to cherry-pick same commits in multiple branches, e.g. backport patch to multiple major versions or multiple environments. Can you, please, add a way to reselect commits or to paste same commits again? |
We could do a thing where we keep track of the copied commits and if you go to re-paste again we can show a confirmation modal saying 'paste previously copied commits?' |
This is very similar to your suggestion 2. above ("don't clear the selection, but stop telling the user that there is a selection i.e. hide but don't clear"). This seems a little clearer to me, the effect should almost be the same. Here's a PR that does this: #3983. The subtle difference to your suggestion is that we get the same confirmation message again when pasting a second time. If you prefer to have the "paste previously copied commits?" one instead, this would be very easy to do of course. And just for completeness, two other options we could consider:
|
- **PR Description** After pasting commits, hide the cherry-pick status (i.e. remove the "x commits copied" status in the lower right corner, and hide the blue selection of the copied commits). However, keep the copied commits around so that it's possible to paste them again. This can be useful e.g. to backport a bugfix to multiple major version release branches. Discussed in #3198. - **Please check if the PR fulfills these requirements** * [x] Cheatsheets are up-to-date (run `go generate ./...`) * [x] Code has been formatted (see [here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting)) * [x] Tests have been added/updated (see [here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md) for the integration test guide) * [ ] Text is internationalised (see [here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation)) * [ ] If a new UserConfig entry was added, make sure it can be hot-reloaded (see [here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig)) * [ ] Docs have been updated if necessary * [x] You've read through your own file changes for silly mistakes etc
Is your feature request related to a problem? Please describe.
It's annoying that each time I cherry pick commits I need to manually clear the selection by pressing escape in order for lazygit to stop telling me that I've copied commits.
There are two ways to solve this problem:
The first option is obviously the easiest to implement, but the reason I initially required the selection to be manually cleared is that some users may need to paste a commit to multiple destination branches.
Now, if such a user has pasted a bespoke commit (e.g. that adds a new file) and the selection is automatically cleared, they can just copy the newly created commit and then go paste that in the other destination. But if it's a commit whose patch changes based due to the new base (or if it's a set of commits, one of which ends up being a no-op in the destination branch due to existing changes), they'll need to navigate back to the original branch to re-copy the original commits.
So the question is: how much do we care about that situation? I very rarely encounter it myself, but it is a real thing.
If we do want to support this use case, we could make it so that upon pasting, we hide the seletion without clearing it. So that means the commits will no longer be highlighted and the bottom right of the screen no longer mentions copied commits. We could tell the user when they confirm the cherry-pick 'Lazygit will remember the copied commits so you can still paste them again in another destination'. This honestly doesn't even sound that hard to implement: you'd just need a new boolean flag like 'isPasted' which, if true, hides all the UI stuff around cherry picking.
Putting aside the question of hide vs clear, we need to consider another scenario: merge conflicts, If we encounter conflicts when cherry-picking, do we:
1 is easy to implement but is jarring for users who want to retry the cherry pick after aborting. As for 2, it's not clear to me how to do this in a clean way: do you just say that any time any rebase finishes, we clear the selection? And how do you even know if a rebase was successful or not, e.g. if it happened outside of git (users often resolve merge conflicts outside of lazygit)? Option 3 is straightforward but inconsistent with the default behaviour of automatically clearing. Nonetheless, if conflicts happen rarely enough, that inconsistency could be tolerable.
Alternatives considered
We could add a config option for whether you clear on paste, but I don't like the idea because whether you want it cleared on paste is situation dependent, and you can't know ahead of time.
Open questions
What do people think?
The text was updated successfully, but these errors were encountered: