diff --git a/pkg/gui/filetree/file_tree_view_model.go b/pkg/gui/filetree/file_tree_view_model.go index 68829b444..f6465e64b 100644 --- a/pkg/gui/filetree/file_tree_view_model.go +++ b/pkg/gui/filetree/file_tree_view_model.go @@ -98,22 +98,22 @@ func (self *FileTreeViewModel) GetSelectedPath() string { } func (self *FileTreeViewModel) SetTree() { - newFiles := self.GetAllFiles() selectedNode := self.GetSelected() - - // for when you stage the old file of a rename and the new file is in a collapsed dir - for _, file := range newFiles { - if selectedNode != nil && selectedNode.path != "" && file.PreviousPath == selectedNode.path { - self.ExpandToPath(file.Path) - } - } - prevNodes := self.GetAllItems() prevSelectedLineIdx := self.GetSelectedLineIdx() self.IFileTree.SetTree() if selectedNode != nil { + // If the selected file has become the old half of a rename, e.g. because + // its deletion was staged, make sure the rename is visible so that the + // selection can move to it. + for _, node := range self.GetRoot().GetLeaves() { + if node.File.PreviousPath == selectedNode.GetPath() { + self.ExpandToPath(node.GetInternalPath()) + } + } + newNodes := self.GetAllItems() newIdx := self.findNewSelectedIdx(prevNodes[prevSelectedLineIdx:], newNodes) if newIdx != -1 && newIdx != prevSelectedLineIdx { @@ -131,7 +131,16 @@ func (self *FileTreeViewModel) SetTree() { // nodes until we find one that exists in the new set of nodes, then move the cursor // to that. // prevNodes starts from our previously selected node because we don't need to consider anything above that +// +// A compressed directory node stands for every directory that was squished +// into it, so it matches any new node that stands for at least one of the same +// directories. When a compressed directory splits into several nodes because +// a file appeared in another of its subdirectories, the topmost of these nodes +// comes first in currNodes and takes over the selection; this keeps the cursor +// on the same line. func (self *FileTreeViewModel) findNewSelectedIdx(prevNodes []*FileNode, currNodes []*FileNode) int { + // Paths are compared as the user sees them, without the "./" prefix of the + // root item, so that they line up with the names of a rename. getPaths := func(node *FileNode) []string { if node == nil { return nil @@ -139,7 +148,7 @@ func (self *FileTreeViewModel) findNewSelectedIdx(prevNodes []*FileNode, currNod if node.File != nil && node.File.IsRename() { return node.File.Names() } - return []string{node.path} + return node.GetPaths() } for _, prevNode := range prevNodes { @@ -150,7 +159,7 @@ func (self *FileTreeViewModel) findNewSelectedIdx(prevNodes []*FileNode, currNod // If you started off with a rename selected, and now it's broken in two, we want you to jump to the new file, not the old file. // This is because the new should be in the same position as the rename was meaning less cursor jumping - foundOldFileInRename := prevNode.File != nil && prevNode.File.IsRename() && node.path == prevNode.File.PreviousPath + foundOldFileInRename := prevNode.File != nil && prevNode.File.IsRename() && node.GetPath() == prevNode.File.PreviousPath foundNode := utils.StringArraysOverlap(paths, selectedPaths) && !foundOldFileInRename if foundNode { return idx diff --git a/pkg/gui/filetree/file_tree_view_model_test.go b/pkg/gui/filetree/file_tree_view_model_test.go index c14c91ea8..088b2b859 100644 --- a/pkg/gui/filetree/file_tree_view_model_test.go +++ b/pkg/gui/filetree/file_tree_view_model_test.go @@ -5,6 +5,8 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/common" + "github.com/jesseduffield/lazygit/pkg/config" + "github.com/samber/lo" "github.com/stretchr/testify/assert" ) @@ -30,3 +32,174 @@ func TestSetStatusFilterPreservingSelection(t *testing.T) { assert.Equal(t, "file3", viewModel.GetSelectedPath()) assert.False(t, viewModel.IsSelectingRange()) } + +func TestSetTreeSelectsNewFileWhenSelectedRenameSplits(t *testing.T) { + scenarios := []struct { + name string + showRootItem bool + expectedPath string + }{ + { + name: "with root item", + showRootItem: true, + expectedPath: "dir/new.go", + }, + { + name: "without root item", + showRootItem: false, + expectedPath: "dir/new.go", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + userConfig := config.GetDefaultConfig() + userConfig.Gui.ShowRootItemInFileTree = s.showRootItem + cmn := common.NewDummyCommonWithUserConfigAndAppState(userConfig, nil) + + files := []*models.File{ + {Path: "dir/new.go", PreviousPath: "dir/old.go"}, + {Path: "other.go"}, + } + viewModel := NewFileTreeViewModel(func() []*models.File { return files }, cmn, true) + viewModel.SetTree() + idx, found := viewModel.GetIndexForPath(InternalTreePathForFilePath("dir/new.go", s.showRootItem)) + assert.True(t, found) + viewModel.SetSelection(idx) + + // the rename is split into its two halves, e.g. because it was unstaged + files = []*models.File{ + {Path: "dir/new.go"}, + {Path: "dir/old.go"}, + {Path: "other.go"}, + } + viewModel.SetTree() + + assert.Equal(t, s.expectedPath, viewModel.GetSelectedPath()) + }) + } +} + +func TestSetTreeFollowsRenameIntoCollapsedDir(t *testing.T) { + scenarios := []struct { + name string + showRootItem bool + expectedPath string + }{ + { + name: "with root item", + showRootItem: true, + expectedPath: "a/new.go", + }, + { + name: "without root item", + showRootItem: false, + expectedPath: "a/new.go", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + userConfig := config.GetDefaultConfig() + userConfig.Gui.ShowRootItemInFileTree = s.showRootItem + cmn := common.NewDummyCommonWithUserConfigAndAppState(userConfig, nil) + + files := []*models.File{ + {Path: "a/b.go"}, + {Path: "a/new.go"}, + {Path: "old.go"}, + } + viewModel := NewFileTreeViewModel(func() []*models.File { return files }, cmn, true) + viewModel.SetTree() + viewModel.ToggleCollapsed(InternalTreePathForFilePath("a", s.showRootItem)) + idx, found := viewModel.GetIndexForPath(InternalTreePathForFilePath("old.go", s.showRootItem)) + assert.True(t, found) + viewModel.SetSelection(idx) + + // staging the deletion of old.go turns it into the old half of a rename + files = []*models.File{ + {Path: "a/b.go"}, + {Path: "a/new.go", PreviousPath: "old.go"}, + } + viewModel.SetTree() + + assert.Equal(t, s.expectedPath, viewModel.GetSelectedPath()) + }) + } +} + +func TestSetTreeKeepsSelectionAcrossCompressionChanges(t *testing.T) { + scenarios := []struct { + name string + filesBefore []string + selectedPath string + filesAfter []string + expectedPath string + }{ + { + name: "compressed root directory splits", + filesBefore: []string{"pkg/gui/controllers/helpers/refresh_helper.go"}, + selectedPath: "pkg/gui/controllers/helpers", + filesAfter: []string{ + "pkg/gui/context/base_context.go", + "pkg/gui/controllers/helpers/refresh_helper.go", + }, + expectedPath: "pkg/gui", + }, + { + name: "compressed subdirectory splits", + filesBefore: []string{"a/b/c/file1", "file2"}, + selectedPath: "a/b/c", + filesAfter: []string{"a/b/c/file1", "a/b/d/file3", "file2"}, + expectedPath: "a/b", + }, + { + name: "file inside a compressed directory that splits", + filesBefore: []string{"pkg/gui/controllers/helpers/refresh_helper.go"}, + selectedPath: "pkg/gui/controllers/helpers/refresh_helper.go", + filesAfter: []string{ + "pkg/gui/context/base_context.go", + "pkg/gui/controllers/helpers/refresh_helper.go", + }, + expectedPath: "pkg/gui/controllers/helpers/refresh_helper.go", + }, + { + name: "directories merge into one compressed node", + filesBefore: []string{ + "pkg/gui/context/base_context.go", + "pkg/gui/controllers/helpers/refresh_helper.go", + }, + selectedPath: "pkg/gui", + filesAfter: []string{"pkg/gui/controllers/helpers/refresh_helper.go"}, + expectedPath: "pkg/gui/controllers/helpers", + }, + } + + toFiles := func(paths []string) []*models.File { + return lo.Map(paths, func(path string, _ int) *models.File { + return &models.File{Path: path} + }) + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + files := toFiles(s.filesBefore) + cmn := common.NewDummyCommon() + viewModel := NewFileTreeViewModel( + func() []*models.File { return files }, + cmn, + true, + ) + viewModel.SetTree() + showRootItem := cmn.UserConfig().Gui.ShowRootItemInFileTree + idx, found := viewModel.GetIndexForPath(InternalTreePathForFilePath(s.selectedPath, showRootItem)) + assert.True(t, found) + viewModel.SetSelection(idx) + + files = toFiles(s.filesAfter) + viewModel.SetTree() + + assert.Equal(t, s.expectedPath, viewModel.GetSelectedPath()) + }) + } +} diff --git a/pkg/gui/filetree/node.go b/pkg/gui/filetree/node.go index 143cdeeff..fc409d501 100644 --- a/pkg/gui/filetree/node.go +++ b/pkg/gui/filetree/node.go @@ -63,6 +63,20 @@ func (self *Node[T]) GetInternalPath() string { return self.path } +// This returns the logical paths of all the directories that this node stands +// for, from the user's point of view like GetPath. For most nodes that's just +// its own path. A compressed node (see CompressionLevel) also stands for the +// directories that were squished into it, so for "a/b/c" with a +// CompressionLevel of 2 this returns "a/b/c", "a/b" and "a". +func (self *Node[T]) GetPaths() []string { + splitPath := split(self.path) + paths := make([]string, 0, self.CompressionLevel+1) + for i := 0; i <= self.CompressionLevel; i++ { + paths = append(paths, strings.TrimPrefix(join(splitPath[:len(splitPath)-i]), "./")) + } + return paths +} + func (self *Node[T]) Sort(cmp func(a, b *Node[T]) int) { self.SortChildren(cmp)