From 64615b7fb2ffb31e254526d7bb80dafada12d636 Mon Sep 17 00:00:00 2001 From: John Claus Date: Mon, 15 Jun 2026 06:24:41 -0600 Subject: [PATCH] Fix file tree navigation while filter active Navigation evaluators required each node to match the filter regex, while rendering kept ancestor directories of matches visible. This desynced the cursor index from the rendered tree, so collapse and expand became a no op under an active filter. Derive cursor visibility from the Hidden flag that Update already computes instead. --- cmd/dive/cli/internal/ui/v1/view/filetree.go | 8 +- .../cli/internal/ui/v1/viewmodel/filetree.go | 34 +++----- .../internal/ui/v1/viewmodel/filetree_test.go | 83 ++++++++++++++++--- 3 files changed, 89 insertions(+), 36 deletions(-) diff --git a/cmd/dive/cli/internal/ui/v1/view/filetree.go b/cmd/dive/cli/internal/ui/v1/view/filetree.go index f106cb1f..74f1ef95 100644 --- a/cmd/dive/cli/internal/ui/v1/view/filetree.go +++ b/cmd/dive/cli/internal/ui/v1/view/filetree.go @@ -246,7 +246,7 @@ func (v *FileTree) CursorUp() error { // CursorLeft moves the cursor up until we reach the Parent Node or top of the tree func (v *FileTree) CursorLeft() error { - err := v.vm.CursorLeft(v.filterRegex) + err := v.vm.CursorLeft() if err != nil { return err } @@ -256,7 +256,7 @@ func (v *FileTree) CursorLeft() error { // CursorRight descends into directory expanding it if needed func (v *FileTree) CursorRight() error { - err := v.vm.CursorRight(v.filterRegex) + err := v.vm.CursorRight() if err != nil { return err } @@ -289,7 +289,7 @@ func (v *FileTree) PageUp() error { // ToggleCollapse will collapse/expand the selected FileNode. func (v *FileTree) toggleCollapse() error { - err := v.vm.ToggleCollapse(v.filterRegex) + err := v.vm.ToggleCollapse() if err != nil { return err } @@ -321,7 +321,7 @@ func (v *FileTree) toggleSortOrder() error { } func (v *FileTree) extractFile() error { - node := v.vm.CurrentNode(v.filterRegex) + node := v.vm.CurrentNode() for _, listener := range v.extractListeners { err := listener(node.Path()) if err != nil { diff --git a/cmd/dive/cli/internal/ui/v1/viewmodel/filetree.go b/cmd/dive/cli/internal/ui/v1/viewmodel/filetree.go index 48332d75..3b5152c8 100644 --- a/cmd/dive/cli/internal/ui/v1/viewmodel/filetree.go +++ b/cmd/dive/cli/internal/ui/v1/viewmodel/filetree.go @@ -163,17 +163,17 @@ func (vm *FileTreeViewModel) CursorDown() bool { return true } -func (vm *FileTreeViewModel) CurrentNode(filterRegex *regexp.Regexp) *filetree.FileNode { - return vm.getAbsPositionNode(filterRegex) +func (vm *FileTreeViewModel) CurrentNode() *filetree.FileNode { + return vm.getAbsPositionNode() } // CursorLeft moves the cursor up until we reach the Parent Node or top of the tree -func (vm *FileTreeViewModel) CursorLeft(filterRegex *regexp.Regexp) error { +func (vm *FileTreeViewModel) CursorLeft() error { var visitor func(*filetree.FileNode) error var evaluator func(*filetree.FileNode) bool var dfsCounter, newIndex int oldIndex := vm.TreeIndex - currentNode := vm.getAbsPositionNode(filterRegex) + currentNode := vm.getAbsPositionNode() if currentNode == nil { return nil @@ -189,12 +189,7 @@ func (vm *FileTreeViewModel) CursorLeft(filterRegex *regexp.Regexp) error { } evaluator = func(curNode *filetree.FileNode) bool { - regexMatch := true - if filterRegex != nil { - match := filterRegex.Find([]byte(curNode.Path())) - regexMatch = match != nil - } - return !curNode.Parent.Data.ViewInfo.Collapsed && !curNode.Data.ViewInfo.Hidden && regexMatch + return !curNode.Parent.Data.ViewInfo.Collapsed && !curNode.Data.ViewInfo.Hidden } err := vm.ModelTree.VisitDepthParentFirst(visitor, evaluator) @@ -218,8 +213,8 @@ func (vm *FileTreeViewModel) CursorLeft(filterRegex *regexp.Regexp) error { } // CursorRight descends into directory expanding it if needed -func (vm *FileTreeViewModel) CursorRight(filterRegex *regexp.Regexp) error { - node := vm.getAbsPositionNode(filterRegex) +func (vm *FileTreeViewModel) CursorRight() error { + node := vm.getAbsPositionNode() if node == nil { return nil } @@ -301,7 +296,9 @@ func (vm *FileTreeViewModel) PageUp() error { } // getAbsPositionNode determines the selected screen cursor's location in the file tree, returning the selected FileNode. -func (vm *FileTreeViewModel) getAbsPositionNode(filterRegex *regexp.Regexp) (node *filetree.FileNode) { +// Visibility is derived from the node state populated by Update (the Hidden flag already accounts for the active +// filter, including ancestor directories of matches), keeping cursor positioning in lockstep with what is rendered. +func (vm *FileTreeViewModel) getAbsPositionNode() (node *filetree.FileNode) { var visitor func(*filetree.FileNode) error var evaluator func(*filetree.FileNode) bool var dfsCounter int @@ -315,16 +312,11 @@ func (vm *FileTreeViewModel) getAbsPositionNode(filterRegex *regexp.Regexp) (nod } evaluator = func(curNode *filetree.FileNode) bool { - regexMatch := true - if filterRegex != nil { - match := filterRegex.Find([]byte(curNode.Path())) - regexMatch = match != nil - } parentCollapsed := false if curNode.Parent != nil { parentCollapsed = curNode.Parent.Data.ViewInfo.Collapsed } - return !parentCollapsed && !curNode.Data.ViewInfo.Hidden && regexMatch + return !parentCollapsed && !curNode.Data.ViewInfo.Hidden } err := vm.ModelTree.VisitDepthParentFirst(visitor, evaluator) @@ -336,8 +328,8 @@ func (vm *FileTreeViewModel) getAbsPositionNode(filterRegex *regexp.Regexp) (nod } // ToggleCollapse will collapse/expand the selected FileNode. -func (vm *FileTreeViewModel) ToggleCollapse(filterRegex *regexp.Regexp) error { - node := vm.getAbsPositionNode(filterRegex) +func (vm *FileTreeViewModel) ToggleCollapse() error { + node := vm.getAbsPositionNode() if node != nil && node.Data.FileInfo.IsDir { node.Data.ViewInfo.Collapsed = !node.Data.ViewInfo.Collapsed } diff --git a/cmd/dive/cli/internal/ui/v1/viewmodel/filetree_test.go b/cmd/dive/cli/internal/ui/v1/viewmodel/filetree_test.go index dd406ec4..e74eaa97 100644 --- a/cmd/dive/cli/internal/ui/v1/viewmodel/filetree_test.go +++ b/cmd/dive/cli/internal/ui/v1/viewmodel/filetree_test.go @@ -151,7 +151,7 @@ func TestFileTreeDirCollapse(t *testing.T) { assertPath(t, vm, "/bin", "before toggle of bin") // collapse /bin - err := vm.ToggleCollapse(nil) + err := vm.ToggleCollapse() checkError(t, err, "unable to collapse /bin") assertPath(t, vm, "/bin", "after toggle of bin") @@ -164,7 +164,7 @@ func TestFileTreeDirCollapse(t *testing.T) { assertPath(t, vm, "/etc", "down to etc") // collapse /etc - err = vm.ToggleCollapse(nil) + err = vm.ToggleCollapse() checkError(t, err, "unable to collapse /etc") assertPath(t, vm, "/etc", "after toggle of etc") @@ -173,7 +173,7 @@ func TestFileTreeDirCollapse(t *testing.T) { func assertPath(t *testing.T, vm *FileTreeViewModel, expected string, msg string) { t.Helper() - n := vm.CurrentNode(nil) + n := vm.CurrentNode() require.NotNil(t, n, "unable to get current node") assert.Equal(t, expected, n.Path(), msg) } @@ -199,7 +199,7 @@ func TestFileTreeSelectLayer(t *testing.T) { vm.ShowAttributes = true // collapse /bin - err := vm.ToggleCollapse(nil) + err := vm.ToggleCollapse() checkError(t, err, "unable to collapse /bin") // select the next layer, compareMode = layer @@ -218,7 +218,7 @@ func TestFileShowAggregateChanges(t *testing.T) { vm.ShowAttributes = true // collapse /bin - err := vm.ToggleCollapse(nil) + err := vm.ToggleCollapse() checkError(t, err, "unable to collapse /bin") // select the next layer, compareMode = layer @@ -280,7 +280,7 @@ func TestFileTreeDirCursorRight(t *testing.T) { vm.ShowAttributes = true // collapse /bin - err := vm.ToggleCollapse(nil) + err := vm.ToggleCollapse() checkError(t, err, "unable to collapse /bin") moved := vm.CursorDown() @@ -294,11 +294,11 @@ func TestFileTreeDirCursorRight(t *testing.T) { } // collapse /etc - err = vm.ToggleCollapse(nil) + err = vm.ToggleCollapse() checkError(t, err, "unable to collapse /etc") // expand /etc - err = vm.CursorRight(nil) + err = vm.CursorRight() checkError(t, err, "unable to cursor right") runTestCase(t, vm, width, height, nil) @@ -319,6 +319,67 @@ func TestFileTreeFilterTree(t *testing.T) { runTestCase(t, vm, width, height, regex) } +// TestFileTreeNavigateWithActiveFilter is a regression test for +// https://github.com/wagoodman/dive/issues/627: with a filter active, the cursor must +// still resolve to the visible ancestor directories of a match. Filtering for "network" +// keeps /etc visible (a descendant matches) even though /etc itself does not match, so +// /etc must remain selectable and collapsible. +func TestFileTreeNavigateWithActiveFilter(t *testing.T) { + vm := initializeTestViewModel(t) + + width, height := 100, 1000 + vm.Setup(0, height) + + regex := regexp.MustCompile("network") + err := vm.Update(regex, width, height) + require.NoError(t, err, "unable to update viewmodel") + + // the cursor starts at the top of the filtered tree, which is /etc. + node := vm.CurrentNode() + require.NotNil(t, node, "expected a selectable node under an active filter") + require.True(t, node.Data.FileInfo.IsDir, "expected the selected node to be a directory") + assert.Equal(t, "/etc", node.Path(), "expected /etc to be selectable under the filter") + + // the selected directory must be collapsible while the filter is active. + require.False(t, node.Data.ViewInfo.Collapsed, "expected /etc to start expanded") + err = vm.ToggleCollapse() + require.NoError(t, err, "unable to toggle collapse under filter") + assert.True(t, node.Data.ViewInfo.Collapsed, "expected /etc to collapse under the filter") +} + +// TestFileTreeNavigateCollapseAllThenFilter reproduces the exact sequence from +// https://github.com/wagoodman/dive/issues/627: collapse all directories, apply a +// filter, then expand a directory. Before the fix getAbsPositionNode returned nil here +// (no node matched the regex at the top level), making collapse/expand a no-op. +func TestFileTreeNavigateCollapseAllThenFilter(t *testing.T) { + vm := initializeTestViewModel(t) + + width, height := 100, 1000 + vm.Setup(0, height) + + // collapse all directories, then reset the cursor to the top (as the UI does). + err := vm.ToggleCollapseAll() + require.NoError(t, err, "unable to collapse all") + vm.ResetCursor() + + // apply the filter. + regex := regexp.MustCompile("network") + err = vm.Update(regex, width, height) + require.NoError(t, err, "unable to update viewmodel") + + // /etc is collapsed but still visible (it has a matching descendant); it must be + // selectable and expandable. + node := vm.CurrentNode() + require.NotNil(t, node, "expected a selectable node after collapse-all + filter") + assert.Equal(t, "/etc", node.Path(), "expected /etc to be selectable") + require.True(t, node.Data.ViewInfo.Collapsed, "expected /etc to be collapsed after collapse-all") + + // expand it via cursor-right. + err = vm.CursorRight() + require.NoError(t, err, "unable to expand directory under filter") + assert.False(t, node.Data.ViewInfo.Collapsed, "expected /etc to expand under the filter") +} + func TestFileTreeHideAddedRemovedModified(t *testing.T) { vm := initializeTestViewModel(t) @@ -327,7 +388,7 @@ func TestFileTreeHideAddedRemovedModified(t *testing.T) { vm.ShowAttributes = true // collapse /bin - err := vm.ToggleCollapse(nil) + err := vm.ToggleCollapse() checkError(t, err, "unable to collapse /bin") // select the 7th layer, compareMode = layer @@ -356,7 +417,7 @@ func TestFileTreeHideUnmodified(t *testing.T) { vm.ShowAttributes = true // collapse /bin - err := vm.ToggleCollapse(nil) + err := vm.ToggleCollapse() checkError(t, err, "unable to collapse /bin") // select the 7th layer, compareMode = layer @@ -379,7 +440,7 @@ func TestFileTreeHideTypeWithFilter(t *testing.T) { vm.ShowAttributes = true // collapse /bin - err := vm.ToggleCollapse(nil) + err := vm.ToggleCollapse() checkError(t, err, "unable to collapse /bin") // select the 7th layer, compareMode = layer