Skip to content

Commit

Permalink
Fix possible crash when deleting a branch while filtering is active
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stefanhaller committed Feb 3, 2025
1 parent 49f8dc2 commit c20badb
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/gui/controllers/helpers/refresh_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand All @@ -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()
Expand Down
49 changes: 49 additions & 0 deletions pkg/integration/tests/branch/delete_while_filtering.go
Original file line number Diff line number Diff line change
@@ -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(),
)
},
})
1 change: 1 addition & 0 deletions pkg/integration/tests/test_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ var tests = []*components.IntegrationTest{
branch.DeleteMultiple,
branch.DeleteRemoteBranchWithCredentialPrompt,
branch.DeleteRemoteBranchWithDifferentName,
branch.DeleteWhileFiltering,
branch.DetachedHead,
branch.NewBranchAutostash,
branch.NewBranchFromRemoteTrackingDifferentName,
Expand Down

0 comments on commit c20badb

Please sign in to comment.