lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Clear copied commits selection after pasting (cherry-pick)

Open jesseduffield opened this issue 1 year ago • 3 comments

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:

  1. clear the selection upon paste
  2. don't clear the selection, but stop telling the user that there is a selection i.e. hide but don't clear

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. clear the selection anyway
  2. wait until the rebase has successfully completed before clearing the selection
  3. leave it to the user to clear the selection

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

  • Do we want a concept of a 'hidden' selection for those who want to paste to multiple destinations?
  • What do we do with the selection when we encounter conflicts?

What do people think?

jesseduffield avatar Jan 05 '24 10:01 jesseduffield

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.

stefanhaller avatar Jan 05 '24 10:01 stefanhaller

There are two ways to solve this problem:

1. clear the selection upon paste

2. don't clear the selection, but stop telling the user that there is a selection i.e. hide but don't clear

I'd like to add a third option:

  • clear the selection upon paste, but provide the option to reselect the previous commits

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.

mark2185 avatar Jan 07 '24 11:01 mark2185

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

jesseduffield avatar Jan 11 '24 12:01 jesseduffield