From 6976d3858604f20296a79a1ff732a5d09e81f778 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Jul 2025 14:13:28 +0200 Subject: [PATCH 1/3] Inline the OutsideFilterMode guard into every binding The reason for doing this is that we want to remove the guard from some of these, but we don't want to change the order. --- .../controllers/local_commits_controller.go | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index c4c9fba44..8bbd4a4da 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/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, @@ -84,7 +84,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ }, { Key: opts.GetKey(opts.Config.Commits.RenameCommit), - Handler: self.withItem(self.reword), + Handler: opts.Guards.OutsideFilterMode(self.withItem(self.reword)), GetDisabledReason: self.require( self.singleItemSelected(self.rewordEnabled), ), @@ -95,7 +95,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ }, { Key: opts.GetKey(opts.Config.Commits.RenameCommitWithEditor), - Handler: self.withItem(self.rewordEditor), + Handler: opts.Guards.OutsideFilterMode(self.withItem(self.rewordEditor)), GetDisabledReason: self.require( self.singleItemSelected(self.rewordEnabled), ), @@ -103,7 +103,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ }, { Key: opts.GetKey(opts.Config.Universal.Remove), - Handler: self.withItemsRange(self.drop), + Handler: opts.Guards.OutsideFilterMode(self.withItemsRange(self.drop)), GetDisabledReason: self.require( self.itemRangeSelected( self.canDropCommits, @@ -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 } From bc936e8d1b1c6a03266c16b37a6126ba53e4c64b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Jul 2025 19:35:18 +0200 Subject: [PATCH 2/3] Extract helper function for integration test We are going to reuse it in two other tests that we are going to add in the next commit. --- .../keep_same_commit_selected_on_exit.go | 22 +++--------------- .../tests/filter_by_path/shared.go | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/pkg/integration/tests/filter_by_path/keep_same_commit_selected_on_exit.go b/pkg/integration/tests/filter_by_path/keep_same_commit_selected_on_exit.go index 76f35650a..a657bdedb 100644 --- a/pkg/integration/tests/filter_by_path/keep_same_commit_selected_on_exit.go +++ b/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`), diff --git a/pkg/integration/tests/filter_by_path/shared.go b/pkg/integration/tests/filter_by_path/shared.go index dd2538a9c..015a11f97 100644 --- a/pkg/integration/tests/filter_by_path/shared.go +++ b/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'")) From 52be6964ed9a47a4919b0df18f742910b957e488 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Jul 2025 14:18:08 +0200 Subject: [PATCH 3/3] Allow rewording and dropping commits in filtering mode There's no reason not to allow these. 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. --- .../controllers/local_commits_controller.go | 6 +-- .../drop_commit_in_filtering_mode.go | 43 +++++++++++++++++ .../reword_commit_in_filtering_mode.go | 46 +++++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 4 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 pkg/integration/tests/filter_by_path/drop_commit_in_filtering_mode.go create mode 100644 pkg/integration/tests/filter_by_path/reword_commit_in_filtering_mode.go diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 8bbd4a4da..b70a4ecb9 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -84,7 +84,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ }, { Key: opts.GetKey(opts.Config.Commits.RenameCommit), - Handler: opts.Guards.OutsideFilterMode(self.withItem(self.reword)), + Handler: self.withItem(self.reword), GetDisabledReason: self.require( self.singleItemSelected(self.rewordEnabled), ), @@ -95,7 +95,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ }, { Key: opts.GetKey(opts.Config.Commits.RenameCommitWithEditor), - Handler: opts.Guards.OutsideFilterMode(self.withItem(self.rewordEditor)), + Handler: self.withItem(self.rewordEditor), GetDisabledReason: self.require( self.singleItemSelected(self.rewordEnabled), ), @@ -103,7 +103,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ }, { Key: opts.GetKey(opts.Config.Universal.Remove), - Handler: opts.Guards.OutsideFilterMode(self.withItemsRange(self.drop)), + Handler: self.withItemsRange(self.drop), GetDisabledReason: self.require( self.itemRangeSelected( self.canDropCommits, diff --git a/pkg/integration/tests/filter_by_path/drop_commit_in_filtering_mode.go b/pkg/integration/tests/filter_by_path/drop_commit_in_filtering_mode.go new file mode 100644 index 000000000..7498730ef --- /dev/null +++ b/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(), + ) + }, +}) diff --git a/pkg/integration/tests/filter_by_path/reword_commit_in_filtering_mode.go b/pkg/integration/tests/filter_by_path/reword_commit_in_filtering_mode.go new file mode 100644 index 000000000..8c28a7eac --- /dev/null +++ b/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(), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 1c5f5d75f..32fab95ab 100644 --- a/pkg/integration/tests/test_list.go +++ b/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,