From f561007fb72591dbab15b6631fd4d64ac40f3d82 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 22 Aug 2023 14:54:08 +0200 Subject: [PATCH 1/2] Extract a WithEnsureCommitableFiles function This encapsulates the logic to make sure we have something to commit; which is to - auto-stage all files if no files are staged and the SkipNoStagedFilesWarning config is on - otherwise, prompt the user whether they want to stage all files - error out if we don't have any files at all Of these, the first one was only done when committing with the built-in commit message panel; there's no reason why it shouldn't also be done when committing with the editor, or when amending, and now it is. --- pkg/gui/controllers/files_controller.go | 18 ++--- .../helpers/working_tree_helper.go | 72 +++++++++---------- 2 files changed, 42 insertions(+), 48 deletions(-) diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index b557c636f..b13c1557b 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -615,19 +615,13 @@ func (self *FilesController) refresh() error { } func (self *FilesController) handleAmendCommitPress() error { - if len(self.c.Model().Files) == 0 { - return self.c.ErrorMsg(self.c.Tr.NoFilesStagedTitle) - } + return self.c.Helpers().WorkingTree.WithEnsureCommitableFiles(func() error { + if len(self.c.Model().Commits) == 0 { + return self.c.ErrorMsg(self.c.Tr.NoCommitToAmend) + } - if !self.c.Helpers().WorkingTree.AnyStagedFiles() { - return self.c.Helpers().WorkingTree.PromptToStageAllAndRetry(self.handleAmendCommitPress) - } - - if len(self.c.Model().Commits) == 0 { - return self.c.ErrorMsg(self.c.Tr.NoCommitToAmend) - } - - return self.c.Helpers().AmendHelper.AmendHead() + return self.c.Helpers().AmendHelper.AmendHead() + }) } func (self *FilesController) handleStatusFilterPressed() error { diff --git a/pkg/gui/controllers/helpers/working_tree_helper.go b/pkg/gui/controllers/helpers/working_tree_helper.go index 4f7a6bd83..6f00c7479 100644 --- a/pkg/gui/controllers/helpers/working_tree_helper.go +++ b/pkg/gui/controllers/helpers/working_tree_helper.go @@ -84,29 +84,19 @@ func (self *WorkingTreeHelper) OpenMergeTool() error { } func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage string) error { - if err := self.prepareFilesForCommit(); err != nil { - return self.c.Error(err) - } - - if len(self.c.Model().Files) == 0 { - return self.c.ErrorMsg(self.c.Tr.NoFilesStagedTitle) - } - - if !self.AnyStagedFiles() { - return self.PromptToStageAllAndRetry(self.HandleCommitPress) - } - - return self.commitsHelper.OpenCommitMessagePanel( - &OpenCommitMessagePanelOpts{ - CommitIndex: context.NoCommitIndex, - InitialMessage: initialMessage, - SummaryTitle: self.c.Tr.CommitSummaryTitle, - DescriptionTitle: self.c.Tr.CommitDescriptionTitle, - PreserveMessage: true, - OnConfirm: self.handleCommit, - OnSwitchToEditor: self.switchFromCommitMessagePanelToEditor, - }, - ) + return self.WithEnsureCommitableFiles(func() error { + return self.commitsHelper.OpenCommitMessagePanel( + &OpenCommitMessagePanelOpts{ + CommitIndex: context.NoCommitIndex, + InitialMessage: initialMessage, + SummaryTitle: self.c.Tr.CommitSummaryTitle, + DescriptionTitle: self.c.Tr.CommitDescriptionTitle, + PreserveMessage: true, + OnConfirm: self.handleCommit, + OnSwitchToEditor: self.switchFromCommitMessagePanelToEditor, + }, + ) + }) } func (self *WorkingTreeHelper) handleCommit(summary string, description string) error { @@ -136,18 +126,12 @@ func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath str // HandleCommitEditorPress - handle when the user wants to commit changes via // their editor rather than via the popup panel func (self *WorkingTreeHelper) HandleCommitEditorPress() error { - if len(self.c.Model().Files) == 0 { - return self.c.ErrorMsg(self.c.Tr.NoFilesStagedTitle) - } - - if !self.AnyStagedFiles() { - return self.PromptToStageAllAndRetry(self.HandleCommitEditorPress) - } - - self.c.LogAction(self.c.Tr.Actions.Commit) - return self.c.RunSubprocessAndRefresh( - self.c.Git().Commit.CommitEditorCmdObj(), - ) + return self.WithEnsureCommitableFiles(func() error { + self.c.LogAction(self.c.Tr.Actions.Commit) + return self.c.RunSubprocessAndRefresh( + self.c.Git().Commit.CommitEditorCmdObj(), + ) + }) } func (self *WorkingTreeHelper) HandleWIPCommitPress() error { @@ -179,7 +163,23 @@ func (self *WorkingTreeHelper) HandleCommitPress() error { return self.HandleCommitPressWithMessage(message) } -func (self *WorkingTreeHelper) PromptToStageAllAndRetry(retry func() error) error { +func (self *WorkingTreeHelper) WithEnsureCommitableFiles(handler func() error) error { + if err := self.prepareFilesForCommit(); err != nil { + return self.c.Error(err) + } + + if len(self.c.Model().Files) == 0 { + return self.c.ErrorMsg(self.c.Tr.NoFilesStagedTitle) + } + + if !self.AnyStagedFiles() { + return self.promptToStageAllAndRetry(handler) + } + + return handler() +} + +func (self *WorkingTreeHelper) promptToStageAllAndRetry(retry func() error) error { return self.c.Confirm(types.ConfirmOpts{ Title: self.c.Tr.NoFilesStagedTitle, Prompt: self.c.Tr.NoFilesStagedPrompt, From 1ba8d3060b8b02f8255d818f319a97fb5f9a77f3 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 22 Aug 2023 16:33:38 +0200 Subject: [PATCH 2/2] Ensure committable files for "amend to" and "create fixup commit" These would previously fail with confusing error messages when no files were staged. The diff is best viewed with "ignore whitespace" turned on. --- .../controllers/local_commits_controller.go | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index b3ef598d2..ecc937bac 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -568,10 +568,12 @@ func (self *LocalCommitsController) moveUp(commit *models.Commit) error { func (self *LocalCommitsController) amendTo(commit *models.Commit) error { if self.isHeadCommit() { - if err := self.c.Helpers().AmendHelper.AmendHead(); err != nil { - return err - } - return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + return self.c.Helpers().WorkingTree.WithEnsureCommitableFiles(func() error { + if err := self.c.Helpers().AmendHelper.AmendHead(); err != nil { + return err + } + return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + }) } if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE { @@ -582,10 +584,12 @@ func (self *LocalCommitsController) amendTo(commit *models.Commit) error { Title: self.c.Tr.AmendCommitTitle, Prompt: self.c.Tr.AmendCommitPrompt, HandleConfirm: func() error { - return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func(gocui.Task) error { - self.c.LogAction(self.c.Tr.Actions.AmendCommit) - err := self.c.Git().Rebase.AmendTo(self.c.Model().Commits, self.context().GetView().SelectedLineIdx()) - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) + return self.c.Helpers().WorkingTree.WithEnsureCommitableFiles(func() error { + return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func(gocui.Task) error { + self.c.LogAction(self.c.Tr.Actions.AmendCommit) + err := self.c.Git().Rebase.AmendTo(self.c.Model().Commits, self.context().GetView().SelectedLineIdx()) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) + }) }) }, }) @@ -709,12 +713,14 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err Title: self.c.Tr.CreateFixupCommit, Prompt: prompt, HandleConfirm: func() error { - self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit) - if err := self.c.Git().Commit.CreateFixupCommit(commit.Sha); err != nil { - return self.c.Error(err) - } + return self.c.Helpers().WorkingTree.WithEnsureCommitableFiles(func() error { + self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit) + if err := self.c.Git().Commit.CreateFixupCommit(commit.Sha); err != nil { + return self.c.Error(err) + } - return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + }) }, }) }