From 7c4d8045f9dbcab76c8f627d3c9735de56af37ed Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 5 Jul 2026 17:12:42 +0200 Subject: [PATCH] Clamp the commit-file tree selection when the tree is rebuilt CommitFileTreeViewModel embedded the low-level tree's SetTree, which rebuilds the node list without touching the cursor. So after a shrinking rebuild (e.g. moving a patch out into the index removes a file), the selection index could be left past the end of the tree. GetSelectedItems then indexes out of range and returns a nil node, which segfaults callers such as canEditFiles when the options map is rendered during layout. Override SetTree to ClampSelection after the rebuild. Unlike FileTreeViewModel we deliberately don't also re-find the selected node by path: that walk lands on the containing directory when a file is removed from a dir that then collapses, whereas keeping the clamped index lands on the sibling file (see discard_old_file_changes). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../filetree/commit_file_tree_view_model.go | 16 ++++++++ .../commit_file_tree_view_model_test.go | 40 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 pkg/gui/filetree/commit_file_tree_view_model_test.go diff --git a/pkg/gui/filetree/commit_file_tree_view_model.go b/pkg/gui/filetree/commit_file_tree_view_model.go index e33316788..a58f7d93e 100644 --- a/pkg/gui/filetree/commit_file_tree_view_model.go +++ b/pkg/gui/filetree/commit_file_tree_view_model.go @@ -142,6 +142,22 @@ func (self *CommitFileTreeViewModel) GetSelectedPath() string { return node.GetPath() } +// SetTree rebuilds the tree and clamps the selection so it stays in range. The +// embedded tree's SetTree only rebuilds the node list and doesn't touch the +// cursor, so after a shrinking rebuild (e.g. moving a patch out into the index) +// the selection index could be left past the end of the tree; GetSelectedItems +// would then return a nil node and crash callers such as canEditFiles when the +// options map is rendered during layout. +// +// Unlike FileTreeViewModel.SetTree we don't re-find the selected node by path +// afterwards: that walk lands on the containing directory when a file is removed +// from a dir that then collapses, whereas keeping the (clamped) index lands on +// the sibling file, which is what we want here. +func (self *CommitFileTreeViewModel) SetTree() { + self.ICommitFileTree.SetTree() + self.ClampSelection() +} + // duplicated from file_tree_view_model.go. Generics will help here func (self *CommitFileTreeViewModel) ToggleShowTree() { selectedNode := self.GetSelected() diff --git a/pkg/gui/filetree/commit_file_tree_view_model_test.go b/pkg/gui/filetree/commit_file_tree_view_model_test.go new file mode 100644 index 000000000..c8862f6f9 --- /dev/null +++ b/pkg/gui/filetree/commit_file_tree_view_model_test.go @@ -0,0 +1,40 @@ +package filetree + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/common" + "github.com/stretchr/testify/assert" +) + +// When the tree shrinks under the selection - e.g. moving a patch out into the +// index removes a file - SetTree must keep the selection in range. Otherwise +// GetSelectedItems returns a nil node, which crashes callers such as +// canEditFiles when the options map is rendered during layout. +func TestCommitFileTreeViewModelSetTreeClampsSelectionOnShrink(t *testing.T) { + files := []*models.CommitFile{ + {Path: "file1"}, + {Path: "file2"}, + {Path: "file3"}, + } + viewModel := NewCommitFileTreeViewModel( + func() []*models.CommitFile { return files }, + common.NewDummyCommon(), + false, // flat list + ) + viewModel.SetTree() + viewModel.SetSelectedLineIdx(viewModel.Len() - 1) + + // The file under the cursor goes away and the tree shrinks. + files = []*models.CommitFile{{Path: "file1"}} + viewModel.SetTree() + + assert.Less(t, viewModel.GetSelectedLineIdx(), viewModel.Len()) + assert.NotNil(t, viewModel.GetSelected()) + items, _, _ := viewModel.GetSelectedItems() + assert.NotEmpty(t, items) + for _, item := range items { + assert.NotNil(t, item) + } +}