Allow rewording or dropping commits in filtering mode (#4756)

- **PR Description**

Rewording or dropping commits was disabled when filtering commits by
path or author. This used to be necessary a long time ago for technical
reasons, but these reasons went away with the merge of #2552, so enable
them now.

Technically we could enable a few more, but I chose not to because some
might be surprising or confusing in filtering mode. For example,
creating a fixup commit would work (shift-F), but the newly created
commit might not show up if it doesn't match the filter. Similarly,
pressing `f` to fixup a commit into its parent would work, but that
parent commit might not be visible, so users might expect to be fixing
up into the next visible commit.

Fixes #2554.

Author: stefanhaller

pkg/gui/controllers/local_commits_controller.go

@@ -55,10 +55,10 @@ func NewLocalCommitsController(
 func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) []*types.Binding {
 	editCommitKey := opts.Config.Universal.Edit
 
-	outsideFilterModeBindings := []*types.Binding{
+	bindings := []*types.Binding{
 		{
 			Key:     opts.GetKey(opts.Config.Commits.SquashDown),
-			Handler: self.withItemsRange(self.squashDown),
+			Handler: opts.Guards.OutsideFilterMode(self.withItemsRange(self.squashDown)),
 			GetDisabledReason: self.require(
 				self.itemRangeSelected(
 					self.midRebaseCommandEnabled,
@@ -71,7 +71,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:     opts.GetKey(opts.Config.Commits.MarkCommitAsFixup),
-			Handler: self.withItemsRange(self.fixup),
+			Handler: opts.Guards.OutsideFilterMode(self.withItemsRange(self.fixup)),
 			GetDisabledReason: self.require(
 				self.itemRangeSelected(
 					self.midRebaseCommandEnabled,
@@ -115,7 +115,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:     opts.GetKey(editCommitKey),
-			Handler: self.withItemsRange(self.edit),
+			Handler: opts.Guards.OutsideFilterMode(self.withItemsRange(self.edit)),
 			GetDisabledReason: self.require(
 				self.itemRangeSelected(self.midRebaseCommandEnabled),
 			),
@@ -129,7 +129,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 			// we're calling it 'quick-start interactive rebase' to differentiate it from
 			// when you manually select the base commit.
 			Key:               opts.GetKey(opts.Config.Commits.StartInteractiveRebase),
-			Handler:           self.quickStartInteractiveRebase,
+			Handler:           opts.Guards.OutsideFilterMode(self.quickStartInteractiveRebase),
 			GetDisabledReason: self.require(self.notMidRebase(self.c.Tr.AlreadyRebasing), self.canFindCommitForQuickStart),
 			Description:       self.c.Tr.QuickStartInteractiveRebase,
 			Tooltip: utils.ResolvePlaceholderString(self.c.Tr.QuickStartInteractiveRebaseTooltip, map[string]string{
@@ -138,7 +138,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:     opts.GetKey(opts.Config.Commits.PickCommit),
-			Handler: self.withItems(self.pick),
+			Handler: opts.Guards.OutsideFilterMode(self.withItems(self.pick)),
 			GetDisabledReason: self.require(
 				self.itemRangeSelected(self.pickEnabled),
 			),
@@ -147,7 +147,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:               opts.GetKey(opts.Config.Commits.CreateFixupCommit),
-			Handler:           self.withItem(self.createFixupCommit),
+			Handler:           opts.Guards.OutsideFilterMode(self.withItem(self.createFixupCommit)),
 			GetDisabledReason: self.require(self.singleItemSelected()),
 			Description:       self.c.Tr.CreateFixupCommit,
 			Tooltip: utils.ResolvePlaceholderString(
@@ -159,7 +159,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:     opts.GetKey(opts.Config.Commits.SquashAboveCommits),
-			Handler: self.squashFixupCommits,
+			Handler: opts.Guards.OutsideFilterMode(self.squashFixupCommits),
 			GetDisabledReason: self.require(
 				self.notMidRebase(self.c.Tr.AlreadyRebasing),
 			),
@@ -169,7 +169,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:     opts.GetKey(opts.Config.Commits.MoveDownCommit),
-			Handler: self.withItemsRange(self.moveDown),
+			Handler: opts.Guards.OutsideFilterMode(self.withItemsRange(self.moveDown)),
 			GetDisabledReason: self.require(self.itemRangeSelected(
 				self.midRebaseMoveCommandEnabled,
 				self.canMoveDown,
@@ -178,7 +178,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:     opts.GetKey(opts.Config.Commits.MoveUpCommit),
-			Handler: self.withItemsRange(self.moveUp),
+			Handler: opts.Guards.OutsideFilterMode(self.withItemsRange(self.moveUp)),
 			GetDisabledReason: self.require(self.itemRangeSelected(
 				self.midRebaseMoveCommandEnabled,
 				self.canMoveUp,
@@ -187,25 +187,18 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:               opts.GetKey(opts.Config.Commits.PasteCommits),
-			Handler:           self.paste,
+			Handler:           opts.Guards.OutsideFilterMode(self.paste),
 			GetDisabledReason: self.require(self.canPaste),
 			Description:       self.c.Tr.PasteCommits,
 			DisplayStyle:      &style.FgCyan,
 		},
 		{
 			Key:               opts.GetKey(opts.Config.Commits.MarkCommitAsBaseForRebase),
-			Handler:           self.withItem(self.markAsBaseCommit),
+			Handler:           opts.Guards.OutsideFilterMode(self.withItem(self.markAsBaseCommit)),
 			GetDisabledReason: self.require(self.singleItemSelected()),
 			Description:       self.c.Tr.MarkAsBaseCommit,
 			Tooltip:           self.c.Tr.MarkAsBaseCommitTooltip,
 		},
-	}
-
-	for _, binding := range outsideFilterModeBindings {
-		binding.Handler = opts.Guards.OutsideFilterMode(binding.Handler)
-	}
-
-	bindings := append(outsideFilterModeBindings, []*types.Binding{
 		// overriding this navigation keybinding because we might need to load
 		// more commits on demand
 		{
@@ -251,7 +244,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 			Tooltip:     self.c.Tr.OpenLogMenuTooltip,
 			OpensMenu:   true,
 		},
-	}...)
+	}
 
 	return bindings
 }

pkg/integration/tests/filter_by_path/drop_commit_in_filtering_mode.go

@@ -0,0 +1,43 @@
+package filter_by_path
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var DropCommitInFilteringMode = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Filter commits by file path, then drop a commit",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig: func(config *config.AppConfig) {
+	},
+	SetupRepo: func(shell *Shell) {
+		commonSetup(shell)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		filterByFilterFile(t, keys)
+
+		t.Views().Commits().
+			IsFocused().
+			Lines(
+				Contains(`both files`).IsSelected(),
+				Contains(`only filterFile`),
+			).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectPopup().Confirmation().
+					Title(Equals("Drop commit")).
+					Content(Equals("Are you sure you want to drop the selected commit(s)?")).
+					Confirm()
+			}).
+			Lines(
+				Contains(`only filterFile`).IsSelected(),
+			).
+			Press(keys.Universal.Return).
+			Lines(
+				Contains(`none of the two`),
+				Contains(`only otherFile`),
+				Contains(`only filterFile`).IsSelected(),
+			)
+	},
+})

pkg/integration/tests/filter_by_path/keep_same_commit_selected_on_exit.go

@@ -15,26 +15,10 @@ var KeepSameCommitSelectedOnExit = NewIntegrationTest(NewIntegrationTestArgs{
 		commonSetup(shell)
 	},
 	Run: func(t *TestDriver, keys config.KeybindingConfig) {
-		t.Views().Commits().
-			Focus().
-			Lines(
-				Contains(`none of the two`).IsSelected(),
-				Contains(`both files`),
-				Contains(`only otherFile`),
-				Contains(`only filterFile`),
-			).Press(keys.Universal.FilteringMenu).
-			Tap(func() {
-				t.ExpectPopup().Menu().
-					Title(Equals("Filtering")).
-					Select(Contains("Enter path to filter by")).
-					Confirm()
+		filterByFilterFile(t, keys)
 
-				t.ExpectPopup().Prompt().
-					Title(Equals("Enter path:")).
-					Type("filterF").
-					SuggestionLines(Equals("filterFile")).
-					ConfirmFirstSuggestion()
-			}).
+		t.Views().Commits().
+			IsFocused().
 			Lines(
 				Contains(`both files`).IsSelected(),
 				Contains(`only filterFile`),

pkg/integration/tests/filter_by_path/reword_commit_in_filtering_mode.go

@@ -0,0 +1,46 @@
+package filter_by_path
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var RewordCommitInFilteringMode = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Filter commits by file path, then reword a commit",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig: func(config *config.AppConfig) {
+	},
+	SetupRepo: func(shell *Shell) {
+		commonSetup(shell)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		filterByFilterFile(t, keys)
+
+		t.Views().Commits().
+			IsFocused().
+			Lines(
+				Contains(`both files`).IsSelected(),
+				Contains(`only filterFile`),
+			).
+			SelectNextItem().
+			Press(keys.Commits.RenameCommit).
+			Tap(func() {
+				t.ExpectPopup().CommitMessagePanel().
+					Clear().
+					Type("new message").
+					Confirm()
+			}).
+			Lines(
+				Contains(`both files`),
+				Contains(`new message`).IsSelected(),
+			).
+			Press(keys.Universal.Return).
+			Lines(
+				Contains(`none of the two`),
+				Contains(`both files`),
+				Contains(`only otherFile`),
+				Contains(`new message`).IsSelected(),
+			)
+	},
+})

pkg/integration/tests/filter_by_path/shared.go

@@ -1,6 +1,7 @@
 package filter_by_path
 
 import (
+	"github.com/jesseduffield/lazygit/pkg/config"
 	. "github.com/jesseduffield/lazygit/pkg/integration/components"
 )
 
@@ -17,6 +18,28 @@ func commonSetup(shell *Shell) {
 	shell.EmptyCommit("none of the two")
 }
 
+func filterByFilterFile(t *TestDriver, keys config.KeybindingConfig) {
+	t.Views().Commits().
+		Focus().
+		Lines(
+			Contains(`none of the two`).IsSelected(),
+			Contains(`both files`),
+			Contains(`only otherFile`),
+			Contains(`only filterFile`),
+		).
+		Press(keys.Universal.FilteringMenu)
+
+	t.ExpectPopup().Menu().
+		Title(Equals("Filtering")).
+		Select(Contains("Enter path to filter by")).
+		Confirm()
+	t.ExpectPopup().Prompt().
+		Title(Equals("Enter path:")).
+		Type("filterF").
+		SuggestionLines(Equals("filterFile")).
+		ConfirmFirstSuggestion()
+}
+
 func postFilterTest(t *TestDriver) {
 	t.Views().Information().Content(Contains("Filtering by 'filterFile'"))
 

pkg/integration/tests/test_list.go

@@ -233,7 +233,9 @@ var tests = []*components.IntegrationTest{
 	filter_by_author.SelectAuthor,
 	filter_by_author.TypeAuthor,
 	filter_by_path.CliArg,
+	filter_by_path.DropCommitInFilteringMode,
 	filter_by_path.KeepSameCommitSelectedOnExit,
+	filter_by_path.RewordCommitInFilteringMode,
 	filter_by_path.SelectFile,
 	filter_by_path.ShowDiffsForRenamedFile,
 	filter_by_path.TypeFile,