diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index cd6033b3915..01bf190cd7f 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -285,15 +285,14 @@ func (self *CommitCommands) ShowFileContentCmdObj(hash string, filePath string) return self.cmd.New(cmdArgs).DontLog() } -// Revert reverts the selected commit by hash -func (self *CommitCommands) Revert(hash string) error { - cmdArgs := NewGitCmd("revert").Arg(hash).ToArgv() - - return self.cmd.New(cmdArgs).Run() -} - -func (self *CommitCommands) RevertMerge(hash string, parentNumber int) error { - cmdArgs := NewGitCmd("revert").Arg(hash, "-m", fmt.Sprintf("%d", parentNumber)). +// Revert reverts the selected commits by hash. If isMerge is true, we'll pass -m 1 +// to say we want to revert the first parent of the merge commit, which is the one +// people want in 99.9% of cases. In current git versions we could unconditionally +// pass -m 1 even for non-merge commits, but older versions of git choke on it. +func (self *CommitCommands) Revert(hashes []string, isMerge bool) error { + cmdArgs := NewGitCmd("revert"). + ArgIf(isMerge, "-m", "1"). + Arg(hashes...). ToArgv() return self.cmd.New(cmdArgs).Run() diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 9564827204f..16cdab3ecc8 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -1,7 +1,6 @@ package controllers import ( - "fmt" "strings" "github.com/go-errors/errors" @@ -249,8 +248,8 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ }, { Key: opts.GetKey(opts.Config.Commits.RevertCommit), - Handler: self.withItem(self.revert), - GetDisabledReason: self.require(self.singleItemSelected()), + Handler: self.withItemsRange(self.revert), + GetDisabledReason: self.require(self.itemRangeSelected()), Description: self.c.Tr.Revert, Tooltip: self.c.Tr.RevertCommitTooltip, }, @@ -858,26 +857,34 @@ func (self *LocalCommitsController) addCoAuthor(start, end int) error { return nil } -func (self *LocalCommitsController) revert(commit *models.Commit) error { - if commit.IsMerge() { - return self.createRevertMergeCommitMenu(commit) +func (self *LocalCommitsController) revert(commits []*models.Commit, start, end int) error { + var promptText string + if len(commits) == 1 { + promptText = utils.ResolvePlaceholderString( + self.c.Tr.ConfirmRevertCommit, + map[string]string{ + "selectedCommit": commits[0].ShortHash(), + }) + } else { + promptText = self.c.Tr.ConfirmRevertCommitRange } + hashes := lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash }) + isMerge := lo.SomeBy(commits, func(c *models.Commit) bool { return c.IsMerge() }) self.c.Confirm(types.ConfirmOpts{ - Title: self.c.Tr.Actions.RevertCommit, - Prompt: utils.ResolvePlaceholderString( - self.c.Tr.ConfirmRevertCommit, - map[string]string{ - "selectedCommit": commit.ShortHash(), - }), + Title: self.c.Tr.Actions.RevertCommit, + Prompt: promptText, HandleConfirm: func() error { self.c.LogAction(self.c.Tr.Actions.RevertCommit) return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error { - result := self.c.Git().Commit.Revert(commit.Hash) + result := self.c.Git().Commit.Revert(hashes, isMerge) if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(result); err != nil { return err } - return self.afterRevertCommit() + self.context().MoveSelection(len(commits)) + return self.c.Refresh(types.RefreshOptions{ + Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.BRANCHES}, + }) }) }, }) @@ -885,39 +892,6 @@ func (self *LocalCommitsController) revert(commit *models.Commit) error { return nil } -func (self *LocalCommitsController) createRevertMergeCommitMenu(commit *models.Commit) error { - menuItems := make([]*types.MenuItem, len(commit.Parents)) - for i, parentHash := range commit.Parents { - message, err := self.c.Git().Commit.GetCommitMessageFirstLine(parentHash) - if err != nil { - return err - } - - menuItems[i] = &types.MenuItem{ - Label: fmt.Sprintf("%s: %s", utils.SafeTruncate(parentHash, 8), message), - OnPress: func() error { - parentNumber := i + 1 - self.c.LogAction(self.c.Tr.Actions.RevertCommit) - return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error { - if err := self.c.Git().Commit.RevertMerge(commit.Hash, parentNumber); err != nil { - return err - } - return self.afterRevertCommit() - }) - }, - } - } - - return self.c.Menu(types.CreateMenuOptions{Title: self.c.Tr.SelectParentCommitForMerge, Items: menuItems}) -} - -func (self *LocalCommitsController) afterRevertCommit() error { - self.context().MoveSelection(1) - return self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.BRANCHES}, - }) -} - func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) error { var disabledReasonWhenFilesAreNeeded *types.DisabledReason if len(self.c.Model().Files) == 0 { diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 5642e696f42..ced96cc8a01 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -727,7 +727,6 @@ type TranslationSet struct { FocusCommandLog string CommandLogHeader string RandomTip string - SelectParentCommitForMerge string ToggleWhitespaceInDiffView string ToggleWhitespaceInDiffViewTooltip string IgnoreWhitespaceDiffViewSubTitle string @@ -772,6 +771,7 @@ type TranslationSet struct { OpenCommitInBrowser string ViewBisectOptions string ConfirmRevertCommit string + ConfirmRevertCommitRange string RewordInEditorTitle string RewordInEditorPrompt string CheckoutAutostashPrompt string @@ -1795,7 +1795,6 @@ func EnglishTranslationSet() *TranslationSet { FocusCommandLog: "Focus command log", CommandLogHeader: "You can hide/focus this panel by pressing '%s'\n", RandomTip: "Random tip", - SelectParentCommitForMerge: "Select parent commit for merge", ToggleWhitespaceInDiffView: "Toggle whitespace", ToggleWhitespaceInDiffViewTooltip: "Toggle whether or not whitespace changes are shown in the diff view.", IgnoreWhitespaceDiffViewSubTitle: "(ignoring whitespace)", @@ -1839,6 +1838,7 @@ func EnglishTranslationSet() *TranslationSet { OpenCommitInBrowser: "Open commit in browser", ViewBisectOptions: "View bisect options", ConfirmRevertCommit: "Are you sure you want to revert {{.selectedCommit}}?", + ConfirmRevertCommitRange: "Are you sure you want to revert the selected commits?", RewordInEditorTitle: "Reword in editor", RewordInEditorPrompt: "Are you sure you want to reword this commit in your editor?", HardResetAutostashPrompt: "Are you sure you want to hard reset to '%s'? An auto-stash will be performed if necessary.", diff --git a/pkg/integration/tests/commit/revert_merge.go b/pkg/integration/tests/commit/revert_merge.go index 5d27970d0b6..1d45180869b 100644 --- a/pkg/integration/tests/commit/revert_merge.go +++ b/pkg/integration/tests/commit/revert_merge.go @@ -21,14 +21,9 @@ var RevertMerge = NewIntegrationTest(NewIntegrationTestArgs{ ). Press(keys.Commits.RevertCommit) - t.ExpectPopup().Menu(). - Title(Equals("Select parent commit for merge")). - Lines( - Contains("first change"), - Contains("second-change-branch unrelated change"), - Contains("Cancel"), - ). - Select(Contains("first change")). + t.ExpectPopup().Confirmation(). + Title(Equals("Revert commit")). + Content(MatchesRegexp(`Are you sure you want to revert \w+?`)). Confirm() t.Views().Commits().IsFocused(). diff --git a/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go b/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go index b4186d55a20..ad3761ba238 100644 --- a/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go +++ b/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go @@ -9,16 +9,7 @@ var RevertWithConflictMultipleCommits = NewIntegrationTest(NewIntegrationTestArg Description: "Reverts a range of commits, the first of which conflicts", ExtraCmdArgs: []string{}, Skip: false, - SetupConfig: func(cfg *config.AppConfig) { - // TODO: use our revert UI once we support range-select for reverts - cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ - { - Key: "X", - Context: "commits", - Command: "git -c core.editor=: revert HEAD^ HEAD^^", - }, - } - }, + SetupConfig: func(cfg *config.AppConfig) {}, SetupRepo: func(shell *Shell) { shell.CreateFileAndAdd("myfile", "") shell.Commit("add empty file") @@ -38,13 +29,18 @@ var RevertWithConflictMultipleCommits = NewIntegrationTest(NewIntegrationTestArg Contains("CI ◯ unrelated change"), Contains("CI ◯ add empty file"), ). - Press("X"). + SelectNextItem(). + Press(keys.Universal.RangeSelectDown). + Press(keys.Commits.RevertCommit). Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - // The exact error message is different on different git versions, - // but they all contain the word 'conflict' somewhere. - Content(Contains("conflict")). + t.ExpectPopup().Confirmation(). + Title(Equals("Revert commit")). + Content(Equals("Are you sure you want to revert the selected commits?")). + Confirm() + + t.ExpectPopup().Menu(). + Title(Equals("Conflicts!")). + Select(Contains("View conflicts")). Confirm() }). Lines( @@ -59,7 +55,7 @@ var RevertWithConflictMultipleCommits = NewIntegrationTest(NewIntegrationTestArg t.Views().Options().Content(Contains("View revert options: m")) t.Views().Information().Content(Contains("Reverting (Reset)")) - t.Views().Files().Focus(). + t.Views().Files().IsFocused(). Lines( Contains("UU myfile").IsSelected(), ). diff --git a/pkg/integration/tests/interactive_rebase/revert_during_rebase_when_stopped_on_edit.go b/pkg/integration/tests/interactive_rebase/revert_during_rebase_when_stopped_on_edit.go index 1db0f0370d9..7d49f06db5f 100644 --- a/pkg/integration/tests/interactive_rebase/revert_during_rebase_when_stopped_on_edit.go +++ b/pkg/integration/tests/interactive_rebase/revert_during_rebase_when_stopped_on_edit.go @@ -9,18 +9,10 @@ var RevertDuringRebaseWhenStoppedOnEdit = NewIntegrationTest(NewIntegrationTestA Description: "Revert a series of commits while stopped in a rebase", ExtraCmdArgs: []string{}, Skip: false, - SetupConfig: func(cfg *config.AppConfig) { - // TODO: use our revert UI once we support range-select for reverts - cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ - { - Key: "X", - Context: "commits", - Command: "git -c core.editor=: revert HEAD^ HEAD^^", - }, - } - }, + SetupConfig: func(cfg *config.AppConfig) {}, SetupRepo: func(shell *Shell) { - shell.EmptyCommit("master commit") + shell.EmptyCommit("master commit 1") + shell.EmptyCommit("master commit 2") shell.NewBranch("branch") shell.CreateNCommits(4) }, @@ -32,7 +24,8 @@ var RevertDuringRebaseWhenStoppedOnEdit = NewIntegrationTest(NewIntegrationTestA Contains("commit 03"), Contains("commit 02"), Contains("commit 01"), - Contains("master commit"), + Contains("master commit 2"), + Contains("master commit 1"), ). NavigateToLine(Contains("commit 03")). Press(keys.Universal.Edit). @@ -41,17 +34,27 @@ var RevertDuringRebaseWhenStoppedOnEdit = NewIntegrationTest(NewIntegrationTestA Contains("<-- YOU ARE HERE --- commit 03").IsSelected(), Contains("commit 02"), Contains("commit 01"), - Contains("master commit"), + Contains("master commit 2"), + Contains("master commit 1"), ). - Press("X"). + SelectNextItem(). + Press(keys.Universal.RangeSelectDown). + Press(keys.Commits.RevertCommit). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Revert commit")). + Content(MatchesRegexp(`Are you sure you want to revert \w+?`)). + Confirm() + }). Lines( Contains("pick").Contains("commit 04"), - Contains(`<-- YOU ARE HERE --- Revert "commit 01"`).IsSelected(), + Contains(`<-- YOU ARE HERE --- Revert "commit 01"`), Contains(`Revert "commit 02"`), Contains("commit 03"), - Contains("commit 02"), - Contains("commit 01"), - Contains("master commit"), + Contains("commit 02").IsSelected(), + Contains("commit 01").IsSelected(), + Contains("master commit 2"), + Contains("master commit 1"), ) }, }) diff --git a/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go b/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go index 2953e3c3a82..3d3aa42623c 100644 --- a/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go +++ b/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go @@ -9,16 +9,7 @@ var RevertMultipleCommitsInInteractiveRebase = NewIntegrationTest(NewIntegration Description: "Reverts a range of commits, the first of which conflicts, in the middle of an interactive rebase", ExtraCmdArgs: []string{}, Skip: false, - SetupConfig: func(cfg *config.AppConfig) { - // TODO: use our revert UI once we support range-select for reverts - cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ - { - Key: "X", - Context: "commits", - Command: "git -c core.editor=: revert HEAD^ HEAD^^", - }, - } - }, + SetupConfig: func(cfg *config.AppConfig) {}, SetupRepo: func(shell *Shell) { shell.CreateFileAndAdd("myfile", "") shell.Commit("add empty file") @@ -44,13 +35,18 @@ var RevertMultipleCommitsInInteractiveRebase = NewIntegrationTest(NewIntegration ). NavigateToLine(Contains("add second line")). Press(keys.Universal.Edit). - Press("X"). + SelectNextItem(). + Press(keys.Universal.RangeSelectDown). + Press(keys.Commits.RevertCommit). Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - // The exact error message is different on different git versions, - // but they all contain the word 'conflict' somewhere. - Content(Contains("conflict")). + t.ExpectPopup().Confirmation(). + Title(Equals("Revert commit")). + Content(Equals("Are you sure you want to revert the selected commits?")). + Confirm() + + t.ExpectPopup().Menu(). + Title(Equals("Conflicts!")). + Select(Contains("View conflicts")). Confirm() }). Lines( @@ -67,7 +63,7 @@ var RevertMultipleCommitsInInteractiveRebase = NewIntegrationTest(NewIntegration t.Views().Options().Content(Contains("View revert options: m")) t.Views().Information().Content(Contains("Reverting (Reset)")) - t.Views().Files().Focus(). + t.Views().Files().IsFocused(). Lines( Contains("UU myfile").IsSelected(), ).