Allow pasting commits multiple times (#3983)

- **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

pkg/gui/controllers/commits_files_controller.go

@@ -303,7 +303,8 @@ func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.Comm
 				self.c.Git().Patch.PatchBuilder.Reset()
 			}
 
-			return self.c.PostRefreshUpdate(self.context())
+			self.c.PostRefreshUpdate(self.context())
+			return nil
 		})
 	}
 
@@ -387,9 +388,7 @@ func (self *CommitFilesController) enterCommitFile(node *filetree.CommitFileNode
 func (self *CommitFilesController) handleToggleCommitFileDirCollapsed(node *filetree.CommitFileNode) error {
 	self.context().CommitFileTreeViewModel.ToggleCollapsed(node.GetPath())
 
-	if err := self.c.PostRefreshUpdate(self.context()); err != nil {
+	self.c.PostRefreshUpdate(self.context())
-		self.c.Log.Error(err)
-	}
 
 	return nil
 }
@@ -398,7 +397,8 @@ func (self *CommitFilesController) handleToggleCommitFileDirCollapsed(node *file
 func (self *CommitFilesController) toggleTreeView() error {
 	self.context().CommitFileTreeViewModel.ToggleShowTree()
 
-	return self.c.PostRefreshUpdate(self.context())
+	self.c.PostRefreshUpdate(self.context())
+	return nil
 }
 
 // NOTE: these functions are identical to those in files_controller.go (except for types) and

pkg/gui/controllers/files_controller.go

@@ -373,9 +373,7 @@ func (self *FilesController) optimisticChange(nodes []*filetree.FileNode, optimi
 	}
 
 	if rerender {
-		if err := self.c.PostRefreshUpdate(self.c.Contexts().Files); err != nil {
+		self.c.PostRefreshUpdate(self.c.Contexts().Files)
-			return err
-		}
 	}
 
 	return nil
@@ -710,7 +708,8 @@ func (self *FilesController) handleStatusFilterPressed() error {
 
 func (self *FilesController) setStatusFiltering(filter filetree.FileTreeDisplayFilter) error {
 	self.context().FileTreeViewModel.SetStatusFilter(filter)
-	return self.c.PostRefreshUpdate(self.context())
+	self.c.PostRefreshUpdate(self.context())
+	return nil
 }
 
 func (self *FilesController) edit(nodes []*filetree.FileNode) error {
@@ -949,9 +948,7 @@ func (self *FilesController) handleToggleDirCollapsed() error {
 
 	self.context().FileTreeViewModel.ToggleCollapsed(node.GetPath())
 
-	if err := self.c.PostRefreshUpdate(self.c.Contexts().Files); err != nil {
+	self.c.PostRefreshUpdate(self.c.Contexts().Files)
-		self.c.Log.Error(err)
-	}
 
 	return nil
 }
@@ -959,7 +956,8 @@ func (self *FilesController) handleToggleDirCollapsed() error {
 func (self *FilesController) toggleTreeView() error {
 	self.context().FileTreeViewModel.ToggleShowTree()
 
-	return self.c.PostRefreshUpdate(self.context())
+	self.c.PostRefreshUpdate(self.context())
+	return nil
 }
 
 func (self *FilesController) handleStashSave(stashFunc func(message string) error, action string) error {

pkg/gui/controllers/helpers/cherry_pick_helper.go

@@ -57,7 +57,10 @@ func (self *CherryPickHelper) CopyRange(commitsList []*models.Commit, context ty
 		}
 	}
 
-	return self.rerender()
+	self.getData().DidPaste = false
+
+	self.rerender()
+	return nil
 }
 
 // HandlePasteCommits begins a cherry-pick rebase with the commits the user has copied.
@@ -102,7 +105,8 @@ func (self *CherryPickHelper) Paste() error {
 					return err
 				}
 				if !isInRebase {
-					return self.Reset()
+					self.getData().DidPaste = true
+					self.rerender()
 				}
 				return nil
 			})
@@ -113,14 +117,15 @@ func (self *CherryPickHelper) Paste() error {
 }
 
 func (self *CherryPickHelper) CanPaste() bool {
-	return self.getData().Active()
+	return self.getData().CanPaste()
 }
 
 func (self *CherryPickHelper) Reset() error {
 	self.getData().ContextKey = ""
 	self.getData().CherryPickedCommits = nil
 
-	return self.rerender()
+	self.rerender()
+	return nil
 }
 
 // you can only copy from one context at a time, because the order and position of commits matter
@@ -136,16 +141,12 @@ func (self *CherryPickHelper) resetIfNecessary(context types.Context) error {
 	return nil
 }
 
-func (self *CherryPickHelper) rerender() error {
+func (self *CherryPickHelper) rerender() {
 	for _, context := range []types.Context{
 		self.c.Contexts().LocalCommits,
 		self.c.Contexts().ReflogCommits,
 		self.c.Contexts().SubCommits,
 	} {
-		if err := self.c.PostRefreshUpdate(context); err != nil {
+		self.c.PostRefreshUpdate(context)
-			return err
-		}
 	}
-
-	return nil
 }

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

@@ -487,5 +487,6 @@ func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranch
 
 func (self *MergeAndRebaseHelper) ResetMarkedBaseCommit() error {
 	self.c.Modes().MarkedBaseCommit.Reset()
-	return self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)
+	self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)
+	return nil
 }

pkg/gui/controllers/helpers/patch_building_helper.go

@@ -52,7 +52,8 @@ func (self *PatchBuildingHelper) Reset() error {
 	}
 
 	// refreshing the current context so that the secondary panel is hidden if necessary.
-	return self.c.PostRefreshUpdate(self.c.Context().Current())
+	self.c.PostRefreshUpdate(self.c.Context().Current())
+	return nil
 }
 
 func (self *PatchBuildingHelper) RefreshPatchBuildingPanel(opts types.OnFocusOpts) {

pkg/gui/controllers/helpers/refresh_helper.go

@@ -157,7 +157,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error {
 		}
 
 		if scopeSet.Includes(types.STASH) {
-			refresh("stash", func() { _ = self.refreshStashEntries() })
+			refresh("stash", func() { self.refreshStashEntries() })
 		}
 
 		if scopeSet.Includes(types.TAGS) {
@@ -169,7 +169,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error {
 		}
 
 		if scopeSet.Includes(types.WORKTREES) && !includeWorktreesWithBranches {
-			refresh("worktrees", func() { _ = self.refreshWorktrees() })
+			refresh("worktrees", func() { self.refreshWorktrees() })
 		}
 
 		if scopeSet.Includes(types.STAGING) {
@@ -343,7 +343,8 @@ func (self *RefreshHelper) refreshCommitsWithLimit() error {
 	self.c.Model().WorkingTreeStateAtLastCommitRefresh = self.c.Git().Status.WorkingTreeState()
 	self.c.Model().CheckedOutBranch = checkedOutBranchName
 
-	return self.refreshView(self.c.Contexts().LocalCommits)
+	self.refreshView(self.c.Contexts().LocalCommits)
+	return nil
 }
 
 func (self *RefreshHelper) refreshSubCommitsWithLimit() error {
@@ -368,7 +369,8 @@ func (self *RefreshHelper) refreshSubCommitsWithLimit() error {
 	self.c.Model().SubCommits = commits
 	self.RefreshAuthors(commits)
 
-	return self.refreshView(self.c.Contexts().SubCommits)
+	self.refreshView(self.c.Contexts().SubCommits)
+	return nil
 }
 
 func (self *RefreshHelper) RefreshAuthors(commits []*models.Commit) {
@@ -397,7 +399,8 @@ func (self *RefreshHelper) refreshCommitFilesContext() error {
 	self.c.Model().CommitFiles = files
 	self.c.Contexts().CommitFiles.CommitFileTreeViewModel.SetTree()
 
-	return self.refreshView(self.c.Contexts().CommitFiles)
+	self.refreshView(self.c.Contexts().CommitFiles)
+	return nil
 }
 
 func (self *RefreshHelper) refreshRebaseCommits() error {
@@ -411,7 +414,8 @@ func (self *RefreshHelper) refreshRebaseCommits() error {
 	self.c.Model().Commits = updatedCommits
 	self.c.Model().WorkingTreeStateAtLastCommitRefresh = self.c.Git().Status.WorkingTreeState()
 
-	return self.refreshView(self.c.Contexts().LocalCommits)
+	self.refreshView(self.c.Contexts().LocalCommits)
+	return nil
 }
 
 func (self *RefreshHelper) refreshTags() error {
@@ -422,7 +426,8 @@ func (self *RefreshHelper) refreshTags() error {
 
 	self.c.Model().Tags = tags
 
-	return self.refreshView(self.c.Contexts().Tags)
+	self.refreshView(self.c.Contexts().Tags)
+	return nil
 }
 
 func (self *RefreshHelper) refreshStateSubmoduleConfigs() error {
@@ -482,9 +487,7 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele
 
 	if refreshWorktrees {
 		self.loadWorktrees()
-		if err := self.refreshView(self.c.Contexts().Worktrees); err != nil {
+		self.refreshView(self.c.Contexts().Worktrees)
-			self.c.Log.Error(err)
-		}
 	}
 
 	if !keepBranchSelectionIndex && prevSelectedBranch != nil {
@@ -495,9 +498,7 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele
 		}
 	}
 
-	if err := self.refreshView(self.c.Contexts().Branches); err != nil {
+	self.refreshView(self.c.Contexts().Branches)
-		self.c.Log.Error(err)
-	}
 
 	// Need to re-render the commits view because the visualization of local
 	// branch heads might have changed
@@ -525,14 +526,8 @@ func (self *RefreshHelper) refreshFilesAndSubmodules() error {
 	}
 
 	self.c.OnUIThread(func() error {
-		if err := self.refreshView(self.c.Contexts().Submodules); err != nil {
+		self.refreshView(self.c.Contexts().Submodules)
-			self.c.Log.Error(err)
+		self.refreshView(self.c.Contexts().Files)
-		}
-
-		if err := self.refreshView(self.c.Contexts().Files); err != nil {
-			self.c.Log.Error(err)
-		}
-
 		return nil
 	})
 
@@ -653,7 +648,8 @@ func (self *RefreshHelper) refreshReflogCommits() error {
 		model.FilteredReflogCommits = model.ReflogCommits
 	}
 
-	return self.refreshView(self.c.Contexts().ReflogCommits)
+	self.refreshView(self.c.Contexts().ReflogCommits)
+	return nil
 }
 
 func (self *RefreshHelper) refreshRemotes() error {
@@ -677,14 +673,8 @@ func (self *RefreshHelper) refreshRemotes() error {
 		}
 	}
 
-	if err := self.refreshView(self.c.Contexts().Remotes); err != nil {
+	self.refreshView(self.c.Contexts().Remotes)
-		return err
+	self.refreshView(self.c.Contexts().RemoteBranches)
-	}
-
-	if err := self.refreshView(self.c.Contexts().RemoteBranches); err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -698,23 +688,20 @@ func (self *RefreshHelper) loadWorktrees() {
 	self.c.Model().Worktrees = worktrees
 }
 
-func (self *RefreshHelper) refreshWorktrees() error {
+func (self *RefreshHelper) refreshWorktrees() {
 	self.loadWorktrees()
 
 	// need to refresh branches because the branches view shows worktrees against
 	// branches
-	if err := self.refreshView(self.c.Contexts().Branches); err != nil {
+	self.refreshView(self.c.Contexts().Branches)
-		return err
+	self.refreshView(self.c.Contexts().Worktrees)
-	}
-
-	return self.refreshView(self.c.Contexts().Worktrees)
 }
 
-func (self *RefreshHelper) refreshStashEntries() error {
+func (self *RefreshHelper) refreshStashEntries() {
 	self.c.Model().StashEntries = self.c.Git().Loaders.StashLoader.
 		GetStashEntries(self.c.Modes().Filtering.GetPath())
 
-	return self.refreshView(self.c.Contexts().Stash)
+	self.refreshView(self.c.Contexts().Stash)
 }
 
 // never call this on its own, it should only be called from within refreshCommits()
@@ -754,12 +741,12 @@ func (self *RefreshHelper) refForLog() string {
 	return bisectInfo.GetStartHash()
 }
 
-func (self *RefreshHelper) refreshView(context types.Context) error {
+func (self *RefreshHelper) refreshView(context types.Context) {
 	// Re-applying the filter must be done before re-rendering the view, so that
 	// the filtered list model is up to date for rendering.
 	self.searchHelper.ReApplyFilter(context)
 
-	err := self.c.PostRefreshUpdate(context)
+	self.c.PostRefreshUpdate(context)
 
 	self.c.AfterLayout(func() error {
 		// Re-applying the search must be done after re-rendering the view though,
@@ -773,6 +760,4 @@ func (self *RefreshHelper) refreshView(context types.Context) error {
 		self.searchHelper.ReApplySearch(context)
 		return nil
 	})
-
-	return err
 }

pkg/gui/controllers/helpers/search_helper.go

@@ -213,7 +213,7 @@ func (self *SearchHelper) Cancel() {
 	switch context := state.Context.(type) {
 	case types.IFilterableContext:
 		context.ClearFilter()
-		_ = self.c.PostRefreshUpdate(context)
+		self.c.PostRefreshUpdate(context)
 	case types.ISearchableContext:
 		context.ClearSearchString()
 		context.GetView().ClearSearch()
@@ -231,7 +231,7 @@ func (self *SearchHelper) OnPromptContentChanged(searchString string) {
 		context.SetSelection(0)
 		context.GetView().SetOriginY(0)
 		context.SetFilter(searchString, self.c.UserConfig().Gui.UseFuzzySearch())
-		_ = self.c.PostRefreshUpdate(context)
+		self.c.PostRefreshUpdate(context)
 	case types.ISearchableContext:
 		// do nothing
 	default:

pkg/gui/controllers/helpers/sub_commits_helper.go

@@ -67,10 +67,7 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error {
 	subCommitsContext.GetView().ClearSearch()
 	subCommitsContext.GetView().TitlePrefix = opts.Context.GetView().TitlePrefix
 
-	err = self.c.PostRefreshUpdate(self.c.Contexts().SubCommits)
+	self.c.PostRefreshUpdate(self.c.Contexts().SubCommits)
-	if err != nil {
-		return err
-	}
 
 	self.c.Context().Push(self.c.Contexts().SubCommits)
 	return nil

pkg/gui/controllers/local_commits_controller.go

@@ -1177,10 +1177,9 @@ func (self *LocalCommitsController) handleOpenLogMenu() error {
 						return func() error {
 							self.c.GetAppState().GitLogShowGraph = value
 							self.c.SaveAppStateAndLogError()
-							if err := self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits); err != nil {
+							self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)
-								return err
+							self.c.PostRefreshUpdate(self.c.Contexts().SubCommits)
-							}
+							return nil
-							return self.c.PostRefreshUpdate(self.c.Contexts().SubCommits)
 						}
 					}
 					return self.c.Menu(types.CreateMenuOptions{
@@ -1286,7 +1285,8 @@ func (self *LocalCommitsController) markAsBaseCommit(commit *models.Commit) erro
 	} else {
 		self.c.Modes().MarkedBaseCommit.SetHash(commit.Hash)
 	}
-	return self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)
+	self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits)
+	return nil
 }
 
 func (self *LocalCommitsController) isHeadCommit(idx int) bool {

pkg/gui/controllers/patch_building_controller.go

@@ -160,7 +160,8 @@ func (self *PatchBuildingController) Escape() error {
 
 	if state.SelectingRange() || state.SelectingHunk() {
 		state.SetLineSelectMode()
-		return self.c.PostRefreshUpdate(context)
+		self.c.PostRefreshUpdate(context)
+		return nil
 	}
 
 	self.c.Helpers().PatchBuilding.Escape()

pkg/gui/controllers/quit_actions.go

@@ -58,7 +58,8 @@ func (self *QuitActions) Escape() error {
 	if listContext, ok := currentContext.(types.IListContext); ok {
 		if listContext.GetList().IsSelectingRange() {
 			listContext.GetList().CancelRangeSelect()
-			return self.c.PostRefreshUpdate(listContext)
+			self.c.PostRefreshUpdate(listContext)
+			return nil
 		}
 	}
 

pkg/gui/controllers/remotes_controller.go

@@ -127,9 +127,7 @@ func (self *RemotesController) enter(remote *models.Remote) error {
 	remoteBranchesContext.SetParentContext(self.Context())
 	remoteBranchesContext.GetView().TitlePrefix = self.Context().GetView().TitlePrefix
 
-	if err := self.c.PostRefreshUpdate(remoteBranchesContext); err != nil {
+	self.c.PostRefreshUpdate(remoteBranchesContext)
-		return err
-	}
 
 	self.c.Context().Push(remoteBranchesContext)
 	return nil

pkg/gui/controllers/staging_controller.go

@@ -168,7 +168,8 @@ func (self *StagingController) EditFile() error {
 func (self *StagingController) Escape() error {
 	if self.context.GetState().SelectingRange() || self.context.GetState().SelectingHunk() {
 		self.context.GetState().SetLineSelectMode()
-		return self.c.PostRefreshUpdate(self.context)
+		self.c.PostRefreshUpdate(self.context)
+		return nil
 	}
 
 	self.c.Context().Pop()

pkg/gui/controllers/workspace_reset_controller.go

@@ -162,10 +162,7 @@ func (self *FilesController) createResetMenu() error {
 
 func (self *FilesController) animateExplosion() {
 	self.Explode(self.c.Views().Files, func() {
-		err := self.c.PostRefreshUpdate(self.c.Contexts().Files)
+		self.c.PostRefreshUpdate(self.c.Contexts().Files)
-		if err != nil {
-			self.c.Log.Error(err)
-		}
 	})
 }
 

pkg/gui/gui_common.go

@@ -29,8 +29,8 @@ func (self *guiCommon) Refresh(opts types.RefreshOptions) error {
 	return self.gui.helpers.Refresh.Refresh(opts)
 }
 
-func (self *guiCommon) PostRefreshUpdate(context types.Context) error {
+func (self *guiCommon) PostRefreshUpdate(context types.Context) {
-	return self.gui.postRefreshUpdate(context)
+	self.gui.postRefreshUpdate(context)
 }
 
 func (self *guiCommon) RunSubprocessAndRefresh(cmdObj oscommands.ICmdObj) error {

pkg/gui/menu_panel.go

@@ -57,7 +57,7 @@ func (gui *Gui) createMenu(opts types.CreateMenuOptions) error {
 		return err
 	}
 
-	_ = gui.c.PostRefreshUpdate(gui.State.Contexts.Menu)
+	gui.c.PostRefreshUpdate(gui.State.Contexts.Menu)
 
 	// TODO: ensure that if we're opened a menu from within a menu that it renders correctly
 	gui.c.Context().Push(gui.State.Contexts.Menu)

pkg/gui/modes/cherrypicking/cherry_picking.go

@@ -9,8 +9,13 @@ import (
 type CherryPicking struct {
 	CherryPickedCommits []*models.Commit
 
-	// we only allow cherry picking from one context at a time, so you can't copy a commit from the local commits context and then also copy a commit in the reflog context
+	// we only allow cherry picking from one context at a time, so you can't copy a commit from
+	// the local commits context and then also copy a commit in the reflog context
 	ContextKey string
+
+	// keep track of whether the currently copied commits have been pasted already. If so, we hide
+	// the mode and the blue display of the commits, but we still allow pasting them again.
+	DidPaste bool
 }
 
 func New() *CherryPicking {
@@ -21,10 +26,18 @@ func New() *CherryPicking {
 }
 
 func (self *CherryPicking) Active() bool {
+	return self.CanPaste() && !self.DidPaste
+}
+
+func (self *CherryPicking) CanPaste() bool {
 	return len(self.CherryPickedCommits) > 0
 }
 
 func (self *CherryPicking) SelectedHashSet() *set.Set[string] {
+	if self.DidPaste {
+		return set.New[string]()
+	}
+
 	hashes := lo.Map(self.CherryPickedCommits, func(commit *models.Commit, _ int) string {
 		return commit.Hash
 	})

pkg/gui/types/common.go

@@ -33,7 +33,7 @@ type IGuiCommon interface {
 	// we call this when we've changed something in the view model but not the actual model,
 	// e.g. expanding or collapsing a folder in a file view. Calling 'Refresh' in this
 	// case would be overkill, although refresh will internally call 'PostRefreshUpdate'
-	PostRefreshUpdate(Context) error
+	PostRefreshUpdate(Context)
 
 	// renders string to a view without resetting its origin
 	SetViewContent(view *gocui.View, content string)

pkg/gui/view_helpers.go

@@ -126,7 +126,7 @@ func (gui *Gui) render() {
 // postRefreshUpdate is to be called on a context after the state that it depends on has been refreshed
 // if the context's view is set to another context we do nothing.
 // if the context's view is the current view we trigger a focus; re-selecting the current item.
-func (gui *Gui) postRefreshUpdate(c types.Context) error {
+func (gui *Gui) postRefreshUpdate(c types.Context) {
 	t := time.Now()
 	defer func() {
 		gui.Log.Infof("postRefreshUpdate for %s took %s", c.GetKey(), time.Since(t))
@@ -137,6 +137,4 @@ func (gui *Gui) postRefreshUpdate(c types.Context) error {
 	if gui.currentViewName() == c.GetViewName() {
 		c.HandleFocus(types.OnFocusOpts{})
 	}
-
-	return nil
 }

pkg/integration/tests/cherry_pick/cherry_pick.go

@@ -79,5 +79,32 @@ var CherryPick = NewIntegrationTest(NewIntegrationTestArgs{
 				Contains("one"),
 				Contains("base"),
 			)
+
+		// Even though the cherry-picking mode has been reset, it's still possible to paste the copied commits again:
+		t.Views().Branches().
+			Focus().
+			NavigateToLine(Contains("master")).
+			PressPrimaryAction()
+
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("base").IsSelected(),
+			).
+			Press(keys.Commits.PasteCommits).
+			Tap(func() {
+				t.ExpectPopup().Alert().
+					Title(Equals("Cherry-pick")).
+					Content(Contains("Are you sure you want to cherry-pick the copied commits onto this branch?")).
+					Confirm()
+			}).
+			Tap(func() {
+				t.Views().Information().Content(DoesNotContain("commits copied"))
+			}).
+			Lines(
+				Contains("four"),
+				Contains("three"),
+				Contains("base"),
+			)
 	},
 })