From ff4ae4a54454e6ed0abbce5b9dd53d72c0c1e24d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 21 Jan 2025 21:00:24 +0100 Subject: [PATCH] Fix possible crash when deleting a branch while filtering is active The code that tries to reselect the same branch again uses GetItems, which in case of filtering is the filtered list. After replacing the branches slice with a new one, the filtered list is no longer up to date, so we must reapply the filter before working with it. It so happens that refreshView does that, so simply call that before setting the selection again; I don't think the order matters in this case. Otherwise we'd have to insert another call to ReApplyFilter before the call to GetItems, which we can avoid this way. Note that this doesn't actually make anything work better in the case of deleting a branch, since we can't reselect the deleted branch anyway of course. But it avoids a possible crash if the branch that was deleted was the last one in the unfiltered list. --- pkg/gui/controllers/helpers/refresh_helper.go | 4 +- .../tests/branch/delete_while_filtering.go | 49 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 pkg/integration/tests/branch/delete_while_filtering.go diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index b9a90af606a..a5be655a1b1 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -490,6 +490,8 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele self.refreshView(self.c.Contexts().Worktrees) } + self.refreshView(self.c.Contexts().Branches) + if !keepBranchSelectionIndex && prevSelectedBranch != nil { _, idx, found := lo.FindIndexOf(self.c.Contexts().Branches.GetItems(), func(b *models.Branch) bool { return b.Name == prevSelectedBranch.Name }) @@ -498,8 +500,6 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele } } - self.refreshView(self.c.Contexts().Branches) - // Need to re-render the commits view because the visualization of local // branch heads might have changed self.c.Mutexes().LocalCommitsMutex.Lock() diff --git a/pkg/integration/tests/branch/delete_while_filtering.go b/pkg/integration/tests/branch/delete_while_filtering.go new file mode 100644 index 00000000000..d967ff40fd2 --- /dev/null +++ b/pkg/integration/tests/branch/delete_while_filtering.go @@ -0,0 +1,49 @@ +package branch + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// Regression test for deleting the last branch in the unfiltered list while +// filtering is on. This used to cause a segfault. +var DeleteWhileFiltering = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Delete a local branch while there's a filter in the branches panel", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetAppState().LocalBranchSortOrder = "alphabetic" + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("one") + shell.NewBranch("branch1") + shell.NewBranch("branch2") + shell.Checkout("master") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("master").IsSelected(), + Contains("branch1"), + Contains("branch2"), + ). + FilterOrSearch("branch"). + Lines( + Contains("branch1").IsSelected(), + Contains("branch2"), + ). + SelectNextItem(). + Press(keys.Universal.Remove). + Tap(func() { + t.ExpectPopup(). + Menu(). + Title(Equals("Delete branch 'branch2'?")). + Select(Contains("Delete local branch")). + Confirm() + }). + Lines( + Contains("branch1").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 4557b8afdbe..b9b480e910c 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -45,6 +45,7 @@ var tests = []*components.IntegrationTest{ branch.DeleteMultiple, branch.DeleteRemoteBranchWithCredentialPrompt, branch.DeleteRemoteBranchWithDifferentName, + branch.DeleteWhileFiltering, branch.DetachedHead, branch.NewBranchAutostash, branch.NewBranchFromRemoteTrackingDifferentName,