From e5c39d5401055d01453a6220f3e62720dd41d2cb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Jul 2025 15:39:27 +0200 Subject: [PATCH 1/7] Fix visual regression when exiting filtering mode When exiting filtering mode while the focus is not in the local commits panel, the call to HandleFocus would render the selection in the commits panel as if the panel had the focus. The call to HandleFocus was introduced recently in 4981419ba95d in an attempt to rerender the main view correctly, but it should only have been called when local commits is the focused context. But instead, we can use PostRefreshUpdate, which handles this distinction. --- pkg/gui/controllers/helpers/mode_helper.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index f90519e4d..a5960697a 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -178,7 +178,8 @@ func (self *ModeHelper) ClearFiltering() error { // before we entered filtering self.c.Contexts().LocalCommits.SelectCommitByHash(self.c.Modes().Filtering.GetSelectedCommitHash()) } - self.c.Contexts().LocalCommits.HandleFocus(types.OnFocusOpts{}) + + self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits) }, }) return nil From 53079998749fce2e5146fd7311a62bb1fb9e5739 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Jul 2025 15:07:35 +0200 Subject: [PATCH 2/7] Allow passing SUB_COMMITS scope to Refresh when no subcommits view is showing We don't currently do this, but will in the next commit; it's a reasonable thing to want to do, and it shouldn't crash. --- pkg/gui/controllers/helpers/refresh_helper.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 2682d8296..dc396be3f 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -345,6 +345,10 @@ func (self *RefreshHelper) refreshCommitsWithLimit() error { } func (self *RefreshHelper) refreshSubCommitsWithLimit() error { + if self.c.Contexts().SubCommits.GetRef() == nil { + return nil + } + self.c.Mutexes().SubCommitsMutex.Lock() defer self.c.Mutexes().SubCommitsMutex.Unlock() From 5e65e8e0eaf61085dc8f2001c94531417c27a276 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Jul 2025 15:18:40 +0200 Subject: [PATCH 3/7] Refresh all affected scopes when entering/exiting filtering Since filtering switches to half-screen mode in the local commits panel, most people probably didn't notice, but we do also filter those other views. So when leaving half-screen mode (but not filtering), you could switch to sub-commits or stashes, and those would show the filtered view only after the next refresh (e.g. after a background fetch). It's worse when leaving filtering, because this goes back to normal screen mode, and you would often see an empty stashes panel after that (until the next background fetch), which is quite confusing. I also find it questionable to always switch focus to the commits panel when entering filtering. If it is initiated from subcommits, reflog, or stashes, maybe we want to stay there. I'm not changing this now since I'm unsure how much people rely on the current behavior. --- pkg/gui/controllers/filtering_menu_action.go | 3 ++- pkg/gui/controllers/helpers/mode_helper.go | 13 ++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/gui/controllers/filtering_menu_action.go b/pkg/gui/controllers/filtering_menu_action.go index 3a109a887..2ed072676 100644 --- a/pkg/gui/controllers/filtering_menu_action.go +++ b/pkg/gui/controllers/filtering_menu_action.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -122,7 +123,7 @@ func (self *FilteringMenuAction) setFiltering() error { self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.COMMITS}, Then: func() { + self.c.Refresh(types.RefreshOptions{Scope: helpers.ScopesToRefreshWhenFilteringModeChanges(), Then: func() { self.c.Contexts().LocalCommits.SetSelection(0) self.c.Contexts().LocalCommits.HandleFocus(types.OnFocusOpts{}) }}) diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index a5960697a..702825c58 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -168,7 +168,7 @@ func (self *ModeHelper) ClearFiltering() error { } self.c.Refresh(types.RefreshOptions{ - Scope: []types.RefreshableView{types.COMMITS}, + Scope: ScopesToRefreshWhenFilteringModeChanges(), Then: func() { // Find the commit that was last selected in filtering mode, and select it again after refreshing if !self.c.Contexts().LocalCommits.SelectCommitByHash(selectedCommitHash) { @@ -185,6 +185,17 @@ func (self *ModeHelper) ClearFiltering() error { return nil } +// Stashes really only need to be refreshed when filtering by path, not by author, but it's too much +// work to distinguish this, and refreshing stashes is fast, so we don't bother +func ScopesToRefreshWhenFilteringModeChanges() []types.RefreshableView { + return []types.RefreshableView{ + types.COMMITS, + types.SUB_COMMITS, + types.REFLOG, + types.STASH, + } +} + func (self *ModeHelper) SetSuppressRebasingMode(value bool) { self.suppressRebasingMode = value } From ea84d5ee96e7fb7574deb34f49d3163a29d4cc4c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Jul 2025 16:55:09 +0200 Subject: [PATCH 4/7] Add integration test for filtering the stashes list by file path The test shows that it doesn't work, the list is empty both when filtering by file and by folder. I could have added test cases for these to the unit tests in stash_loader_test.go instead, but I don't find tests that need to mock git's output very valuable, so I opted for an integration test even though it takes much longer to run. --- pkg/integration/tests/stash/filter_by_path.go | 66 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 67 insertions(+) create mode 100644 pkg/integration/tests/stash/filter_by_path.go diff --git a/pkg/integration/tests/stash/filter_by_path.go b/pkg/integration/tests/stash/filter_by_path.go new file mode 100644 index 000000000..8f151134b --- /dev/null +++ b/pkg/integration/tests/stash/filter_by_path.go @@ -0,0 +1,66 @@ +package stash + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FilterByPath = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Filter the stash list by path", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("initial commit") + shell.CreateFileAndAdd("file1", "content") + shell.Stash("file1") + shell.CreateDir("subdir") + shell.CreateFileAndAdd("subdir/file2", "content") + shell.Stash("subdir/file2") + shell.CreateFileAndAdd("file1", "other content") + shell.Stash("file1 again") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + filterBy := func(path string) { + t.GlobalPress(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(path). + Confirm() + } + + t.Views().Stash(). + Lines( + Contains("file1 again"), + Contains("subdir/file2"), + Contains("file1"), + ) + + filterBy("file1") + + t.Views().Stash(). + /* EXPECTED: + Lines( + Contains("file1 again"), + Contains("file1"), + ) + ACTUAL: */ + IsEmpty() + + t.GlobalPress(keys.Universal.Return) + filterBy("subdir") + + t.Views().Stash(). + /* EXPECTED: + Lines( + Contains("subdir/file2"), + ) + ACTUAL: */ + IsEmpty() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index d5f4f5c21..4a8754284 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -352,6 +352,7 @@ var tests = []*components.IntegrationTest{ stash.CreateBranch, stash.Drop, stash.DropMultiple, + stash.FilterByPath, stash.Pop, stash.PreventDiscardingFileChanges, stash.Rename, From 0dfa07865345750bd2c99a77bf7f3dc0a6b2b6ac Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Jul 2025 16:19:29 +0200 Subject: [PATCH 5/7] Revert "chore: use null char as a stash entries divider during loading" This was not a good idea, and it seems it has never been tested: the --name-only and -z options don't work well together. Specifically, -z seems to simply cancel --name-only. This is not enough to make it work, we'll need more fixes in the next commit. This reverts commit 50044dd5e0ad7d00ea1dbbf0b7c8c97a32bf3af2. --- pkg/commands/git_commands/stash_loader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/commands/git_commands/stash_loader.go b/pkg/commands/git_commands/stash_loader.go index d0a290507..5ec5d4ed4 100644 --- a/pkg/commands/git_commands/stash_loader.go +++ b/pkg/commands/git_commands/stash_loader.go @@ -32,14 +32,14 @@ func (self *StashLoader) GetStashEntries(filterPath string) []*models.StashEntry return self.getUnfilteredStashEntries() } - cmdArgs := NewGitCmd("stash").Arg("list", "-z", "--name-only", "--pretty=%ct|%gs").ToArgv() + cmdArgs := NewGitCmd("stash").Arg("list", "--name-only", "--pretty=%ct|%gs").ToArgv() rawString, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput() if err != nil { return self.getUnfilteredStashEntries() } stashEntries := []*models.StashEntry{} var currentStashEntry *models.StashEntry - lines := utils.SplitNul(rawString) + lines := utils.SplitLines(rawString) isAStash := func(line string) bool { return strings.HasPrefix(line, "stash@{") } re := regexp.MustCompile(`stash@\{(\d+)\}`) From 09cbaf2cba13536d08428d8eb3f6d747f5af7891 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Jul 2025 16:23:22 +0200 Subject: [PATCH 6/7] Make stash loading work in filtering mode This broke with the introduction of the age in the stashes list in bc330b8ff3ef (which was included in 0.41, so 12 minor versions ago). This makes it work again when filtering by file, but it still doesn't work when filtering by folder (and never has). We'll fix that next. --- pkg/commands/git_commands/stash_loader.go | 10 +++++----- pkg/integration/tests/stash/filter_by_path.go | 3 --- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/commands/git_commands/stash_loader.go b/pkg/commands/git_commands/stash_loader.go index 5ec5d4ed4..243be807d 100644 --- a/pkg/commands/git_commands/stash_loader.go +++ b/pkg/commands/git_commands/stash_loader.go @@ -32,7 +32,7 @@ func (self *StashLoader) GetStashEntries(filterPath string) []*models.StashEntry return self.getUnfilteredStashEntries() } - cmdArgs := NewGitCmd("stash").Arg("list", "--name-only", "--pretty=%ct|%gs").ToArgv() + cmdArgs := NewGitCmd("stash").Arg("list", "--name-only", "--pretty=%gd:%ct|%gs").ToArgv() rawString, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput() if err != nil { return self.getUnfilteredStashEntries() @@ -41,19 +41,19 @@ func (self *StashLoader) GetStashEntries(filterPath string) []*models.StashEntry var currentStashEntry *models.StashEntry lines := utils.SplitLines(rawString) isAStash := func(line string) bool { return strings.HasPrefix(line, "stash@{") } - re := regexp.MustCompile(`stash@\{(\d+)\}`) + re := regexp.MustCompile(`^stash@\{(\d+)\}:(.*)$`) outer: for i := 0; i < len(lines); i++ { - if !isAStash(lines[i]) { + match := re.FindStringSubmatch(lines[i]) + if match == nil { continue } - match := re.FindStringSubmatch(lines[i]) idx, err := strconv.Atoi(match[1]) if err != nil { return self.getUnfilteredStashEntries() } - currentStashEntry = stashEntryFromLine(lines[i], idx) + currentStashEntry = stashEntryFromLine(match[2], idx) for i+1 < len(lines) && !isAStash(lines[i+1]) { i++ if lines[i] == filterPath { diff --git a/pkg/integration/tests/stash/filter_by_path.go b/pkg/integration/tests/stash/filter_by_path.go index 8f151134b..7881e9225 100644 --- a/pkg/integration/tests/stash/filter_by_path.go +++ b/pkg/integration/tests/stash/filter_by_path.go @@ -44,13 +44,10 @@ var FilterByPath = NewIntegrationTest(NewIntegrationTestArgs{ filterBy("file1") t.Views().Stash(). - /* EXPECTED: Lines( Contains("file1 again"), Contains("file1"), ) - ACTUAL: */ - IsEmpty() t.GlobalPress(keys.Universal.Return) filterBy("subdir") From ec82e7c1e7be4607b129b6e7c43842f7b7d73a3d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 11 Jul 2025 16:35:29 +0200 Subject: [PATCH 7/7] Make stash filtering work when filtering on a folder --- pkg/commands/git_commands/stash_loader.go | 2 +- pkg/integration/tests/stash/filter_by_path.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/commands/git_commands/stash_loader.go b/pkg/commands/git_commands/stash_loader.go index 243be807d..67c3fb317 100644 --- a/pkg/commands/git_commands/stash_loader.go +++ b/pkg/commands/git_commands/stash_loader.go @@ -56,7 +56,7 @@ outer: currentStashEntry = stashEntryFromLine(match[2], idx) for i+1 < len(lines) && !isAStash(lines[i+1]) { i++ - if lines[i] == filterPath { + if strings.HasPrefix(lines[i], filterPath) { stashEntries = append(stashEntries, currentStashEntry) continue outer } diff --git a/pkg/integration/tests/stash/filter_by_path.go b/pkg/integration/tests/stash/filter_by_path.go index 7881e9225..ea397d70e 100644 --- a/pkg/integration/tests/stash/filter_by_path.go +++ b/pkg/integration/tests/stash/filter_by_path.go @@ -53,11 +53,8 @@ var FilterByPath = NewIntegrationTest(NewIntegrationTestArgs{ filterBy("subdir") t.Views().Stash(). - /* EXPECTED: Lines( Contains("subdir/file2"), ) - ACTUAL: */ - IsEmpty() }, })