From 843e12286f83ba9002ec7edbebcb675388605091 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 1 Sep 2023 18:49:33 +0200 Subject: [PATCH] Improve prompts when amending commits This fixes two minor problems with the prompts: 1. When pressing shift-A in the local commits view, it would first prompt whether to stage all files, and then it would prompt whether to amend the commit at all. This doesn't make sense, it needs to be the other way round. 2. When pressing shift-A on the head commit in an interactive rebase, we would ask whether they want to amend the last commit, like when pressing shift-A in the files view. While this is technically correct, the fact that we're amending the head commit in this case is just an implementation detail, and from the user's point of view it's better to use the same prompt as we do for any other commit. To fix these, we remove the confirmation panel from AmendHelper.AmendHead() and instead add it at the two call sites, so that we have more control over this. --- pkg/gui/controllers/files_controller.go | 16 +++++++++++----- pkg/gui/controllers/helpers/amend_helper.go | 16 +++------------- pkg/gui/controllers/local_commits_controller.go | 16 +++++++++++----- .../amend_head_commit_during_rebase.go | 4 ++-- .../tests/interactive_rebase/amend_merge.go | 4 ++-- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index b13c1557b..aae88fe79 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -615,12 +615,18 @@ func (self *FilesController) refresh() error { } func (self *FilesController) handleAmendCommitPress() error { - return self.c.Helpers().WorkingTree.WithEnsureCommitableFiles(func() error { - if len(self.c.Model().Commits) == 0 { - return self.c.ErrorMsg(self.c.Tr.NoCommitToAmend) - } + return self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.AmendLastCommitTitle, + Prompt: self.c.Tr.SureToAmend, + HandleConfirm: func() error { + return self.c.Helpers().WorkingTree.WithEnsureCommitableFiles(func() error { + 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() + }) + }, }) } diff --git a/pkg/gui/controllers/helpers/amend_helper.go b/pkg/gui/controllers/helpers/amend_helper.go index 0e5216f83..c3e8a8118 100644 --- a/pkg/gui/controllers/helpers/amend_helper.go +++ b/pkg/gui/controllers/helpers/amend_helper.go @@ -1,9 +1,5 @@ package helpers -import ( - "github.com/jesseduffield/lazygit/pkg/gui/types" -) - type AmendHelper struct { c *HelperCommon gpg *GpgHelper @@ -20,13 +16,7 @@ func NewAmendHelper( } func (self *AmendHelper) AmendHead() error { - return self.c.Confirm(types.ConfirmOpts{ - Title: self.c.Tr.AmendLastCommitTitle, - Prompt: self.c.Tr.SureToAmend, - HandleConfirm: func() error { - cmdObj := self.c.Git().Commit.AmendHeadCmdObj() - self.c.LogAction(self.c.Tr.Actions.AmendCommit) - return self.gpg.WithGpgHandling(cmdObj, self.c.Tr.AmendingStatus, nil) - }, - }) + cmdObj := self.c.Git().Commit.AmendHeadCmdObj() + self.c.LogAction(self.c.Tr.Actions.AmendCommit) + return self.gpg.WithGpgHandling(cmdObj, self.c.Tr.AmendingStatus, nil) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index ecc937bac..397cdb808 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -568,11 +568,17 @@ func (self *LocalCommitsController) moveUp(commit *models.Commit) error { func (self *LocalCommitsController) amendTo(commit *models.Commit) error { if self.isHeadCommit() { - 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}) + return self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.AmendCommitTitle, + Prompt: self.c.Tr.AmendCommitPrompt, + HandleConfirm: func() error { + 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}) + }) + }, }) } diff --git a/pkg/integration/tests/interactive_rebase/amend_head_commit_during_rebase.go b/pkg/integration/tests/interactive_rebase/amend_head_commit_during_rebase.go index 16e34e160..10e7f0639 100644 --- a/pkg/integration/tests/interactive_rebase/amend_head_commit_during_rebase.go +++ b/pkg/integration/tests/interactive_rebase/amend_head_commit_during_rebase.go @@ -43,8 +43,8 @@ var AmendHeadCommitDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{ Press(keys.Commits.AmendToCommit). Tap(func() { t.ExpectPopup().Confirmation(). - Title(Equals("Amend last commit")). - Content(Contains("Are you sure you want to amend last commit?")). + Title(Equals("Amend commit")). + Content(Contains("Are you sure you want to amend this commit with your staged files?")). Confirm() }). Lines( diff --git a/pkg/integration/tests/interactive_rebase/amend_merge.go b/pkg/integration/tests/interactive_rebase/amend_merge.go index 13662ce54..3f01688ff 100644 --- a/pkg/integration/tests/interactive_rebase/amend_merge.go +++ b/pkg/integration/tests/interactive_rebase/amend_merge.go @@ -42,8 +42,8 @@ var AmendMerge = NewIntegrationTest(NewIntegrationTestArgs{ Press(keys.Commits.AmendToCommit) t.ExpectPopup().Confirmation(). - Title(Equals("Amend last commit")). - Content(Contains("Are you sure you want to amend last commit?")). + Title(Equals("Amend commit")). + Content(Contains("Are you sure you want to amend this commit with your staged files?")). Confirm() // assuring we haven't added a brand new commit