From 59a937ee7a30d654f60735eac08748de1e222cd5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Oct 2024 10:53:11 +0200 Subject: [PATCH 1/3] Get rid of error return value of PostRefreshUpdate and a few related ones I missed these in https://github.com/jesseduffield/lazygit/pull/3890. --- .../controllers/commits_files_controller.go | 10 +-- pkg/gui/controllers/files_controller.go | 14 ++-- .../controllers/helpers/cherry_pick_helper.go | 14 ++-- .../helpers/merge_and_rebase_helper.go | 3 +- .../helpers/patch_building_helper.go | 3 +- pkg/gui/controllers/helpers/refresh_helper.go | 69 ++++++++----------- pkg/gui/controllers/helpers/search_helper.go | 4 +- .../controllers/helpers/sub_commits_helper.go | 5 +- .../controllers/local_commits_controller.go | 10 +-- .../controllers/patch_building_controller.go | 3 +- pkg/gui/controllers/quit_actions.go | 3 +- pkg/gui/controllers/remotes_controller.go | 4 +- pkg/gui/controllers/staging_controller.go | 3 +- .../controllers/workspace_reset_controller.go | 5 +- pkg/gui/gui_common.go | 4 +- pkg/gui/menu_panel.go | 2 +- pkg/gui/types/common.go | 2 +- pkg/gui/view_helpers.go | 4 +- 18 files changed, 69 insertions(+), 93 deletions(-) diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index a675307312e..462b9c3ee5a 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/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.Log.Error(err) - } + self.c.PostRefreshUpdate(self.context()) 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 diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index 4169c911652..5380c890975 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/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 { - return err - } + self.c.PostRefreshUpdate(self.c.Contexts().Files) } 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.Log.Error(err) - } + self.c.PostRefreshUpdate(self.c.Contexts().Files) 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 { diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index 93637073e89..e5488556e83 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -57,7 +57,8 @@ func (self *CherryPickHelper) CopyRange(commitsList []*models.Commit, context ty } } - return self.rerender() + self.rerender() + return nil } // HandlePasteCommits begins a cherry-pick rebase with the commits the user has copied. @@ -120,7 +121,8 @@ 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 +138,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 { - return err - } + self.c.PostRefreshUpdate(context) } - - return nil } diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 5b3b91772c2..2fb30372b84 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/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 } diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index df107ed9b6e..b238d252c29 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/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) { diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 2a0c93f52e5..46965bddd25 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/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.c.Log.Error(err) - } + self.refreshView(self.c.Contexts().Worktrees) } 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.c.Log.Error(err) - } + self.refreshView(self.c.Contexts().Branches) // 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.c.Log.Error(err) - } - - if err := self.refreshView(self.c.Contexts().Files); err != nil { - self.c.Log.Error(err) - } - + self.refreshView(self.c.Contexts().Submodules) + self.refreshView(self.c.Contexts().Files) 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 { - return err - } - - if err := self.refreshView(self.c.Contexts().RemoteBranches); err != nil { - return err - } - + self.refreshView(self.c.Contexts().Remotes) + self.refreshView(self.c.Contexts().RemoteBranches) 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 { - return err - } - - return self.refreshView(self.c.Contexts().Worktrees) + self.refreshView(self.c.Contexts().Branches) + 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 } diff --git a/pkg/gui/controllers/helpers/search_helper.go b/pkg/gui/controllers/helpers/search_helper.go index 74467b02cfb..e685df34a96 100644 --- a/pkg/gui/controllers/helpers/search_helper.go +++ b/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: diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index a7f9cec8a02..a38acd2a77b 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/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) - if err != nil { - return err - } + self.c.PostRefreshUpdate(self.c.Contexts().SubCommits) self.c.Context().Push(self.c.Contexts().SubCommits) return nil diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 937a4b1d494..8d2d31700bc 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/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 { - return err - } - return self.c.PostRefreshUpdate(self.c.Contexts().SubCommits) + self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits) + self.c.PostRefreshUpdate(self.c.Contexts().SubCommits) + return nil } } 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 { diff --git a/pkg/gui/controllers/patch_building_controller.go b/pkg/gui/controllers/patch_building_controller.go index 9b6568016dd..ea181cd054d 100644 --- a/pkg/gui/controllers/patch_building_controller.go +++ b/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() diff --git a/pkg/gui/controllers/quit_actions.go b/pkg/gui/controllers/quit_actions.go index c85ab069c68..000fe1792e9 100644 --- a/pkg/gui/controllers/quit_actions.go +++ b/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 } } diff --git a/pkg/gui/controllers/remotes_controller.go b/pkg/gui/controllers/remotes_controller.go index dbae0e0b10d..6942e11a86a 100644 --- a/pkg/gui/controllers/remotes_controller.go +++ b/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 { - return err - } + self.c.PostRefreshUpdate(remoteBranchesContext) self.c.Context().Push(remoteBranchesContext) return nil diff --git a/pkg/gui/controllers/staging_controller.go b/pkg/gui/controllers/staging_controller.go index ca3cf20f72a..769cba68a23 100644 --- a/pkg/gui/controllers/staging_controller.go +++ b/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() diff --git a/pkg/gui/controllers/workspace_reset_controller.go b/pkg/gui/controllers/workspace_reset_controller.go index 7e9d115a63b..3f3ddf47cf0 100644 --- a/pkg/gui/controllers/workspace_reset_controller.go +++ b/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) - if err != nil { - self.c.Log.Error(err) - } + self.c.PostRefreshUpdate(self.c.Contexts().Files) }) } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 08103e09a61..dfa4be52f3d 100644 --- a/pkg/gui/gui_common.go +++ b/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 { - return self.gui.postRefreshUpdate(context) +func (self *guiCommon) PostRefreshUpdate(context types.Context) { + self.gui.postRefreshUpdate(context) } func (self *guiCommon) RunSubprocessAndRefresh(cmdObj oscommands.ICmdObj) error { diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go index 5c2f8643f58..c9783db69f8 100644 --- a/pkg/gui/menu_panel.go +++ b/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) diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 4dcafa0053b..9213cfc2443 100644 --- a/pkg/gui/types/common.go +++ b/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) diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 6226797ce22..f0f2bf88e26 100644 --- a/pkg/gui/view_helpers.go +++ b/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 } From f473d23d6596a4379d4acc87a318c4b59e24a2bf Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Oct 2024 11:22:43 +0200 Subject: [PATCH 2/3] Wrap an overly long line --- pkg/gui/modes/cherrypicking/cherry_picking.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/gui/modes/cherrypicking/cherry_picking.go b/pkg/gui/modes/cherrypicking/cherry_picking.go index 4e76cbd094e..40b9ce287de 100644 --- a/pkg/gui/modes/cherrypicking/cherry_picking.go +++ b/pkg/gui/modes/cherrypicking/cherry_picking.go @@ -9,7 +9,8 @@ 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 } From 85523402d698d0cc0242c688f194f237eae256bc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Oct 2024 10:40:06 +0200 Subject: [PATCH 3/3] Allow pasting commits more than once After pasting commits once, we hide the cherry-picking status (as if it had been reset), and no longer paint the copied commits with blue hashes; however, we still allow pasting them again. This can be useful e.g. to backport a bugfix to multiple major version release branches. --- .../controllers/helpers/cherry_pick_helper.go | 7 +++-- pkg/gui/modes/cherrypicking/cherry_picking.go | 12 +++++++++ .../tests/cherry_pick/cherry_pick.go | 27 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index e5488556e83..c21e7037a5d 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -57,6 +57,8 @@ func (self *CherryPickHelper) CopyRange(commitsList []*models.Commit, context ty } } + self.getData().DidPaste = false + self.rerender() return nil } @@ -103,7 +105,8 @@ func (self *CherryPickHelper) Paste() error { return err } if !isInRebase { - return self.Reset() + self.getData().DidPaste = true + self.rerender() } return nil }) @@ -114,7 +117,7 @@ func (self *CherryPickHelper) Paste() error { } func (self *CherryPickHelper) CanPaste() bool { - return self.getData().Active() + return self.getData().CanPaste() } func (self *CherryPickHelper) Reset() error { diff --git a/pkg/gui/modes/cherrypicking/cherry_picking.go b/pkg/gui/modes/cherrypicking/cherry_picking.go index 40b9ce287de..d84cec39ace 100644 --- a/pkg/gui/modes/cherrypicking/cherry_picking.go +++ b/pkg/gui/modes/cherrypicking/cherry_picking.go @@ -12,6 +12,10 @@ type CherryPicking struct { // 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 { @@ -22,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 }) diff --git a/pkg/integration/tests/cherry_pick/cherry_pick.go b/pkg/integration/tests/cherry_pick/cherry_pick.go index c49e5cf3886..eefe7369281 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick.go +++ b/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"), + ) }, })