diff --git a/internal/fileTree/fileTree.go b/internal/fileTree/fileTree.go index 6a060cd..d51f788 100644 --- a/internal/fileTree/fileTree.go +++ b/internal/fileTree/fileTree.go @@ -9,7 +9,10 @@ import ( ) func CreateTree() (*Node, error) { - home, _ := os.UserHomeDir() + home, err := os.UserHomeDir() + if err != nil { + return nil, err + } root, err := buildTree(nil, filepath.Join(home, ".toney"), 0) if err != nil { @@ -36,10 +39,18 @@ func buildTree(parent *Node, path string, depth int) (*Node, error) { } if node.IsDirectory { - files, _ := os.ReadDir(path) + files, err := os.ReadDir(path) + if err != nil { + return nil, err + } + for _, file := range files { childPath := filepath.Join(path, file.Name()) - child, _ := buildTree(&node, childPath, depth+1) + child, err := buildTree(&node, childPath, depth+1) + if err != nil { + return nil, err + } + node.Children = append(node.Children, child) } } diff --git a/internal/fileTree/flatten.go b/internal/fileTree/flatten.go index a71b192..2e6328c 100644 --- a/internal/fileTree/flatten.go +++ b/internal/fileTree/flatten.go @@ -1,17 +1,21 @@ package filetree func FlattenVisibleTree(root *Node) []*Node { + if root == nil { + return []*Node{} + } + var result []*Node - var walk func(n *Node, depth int) + var walk func(n *Node) // Removed unusued 'depth int', 0 references or implementations found for this across the project. - walk = func(n *Node, depth int) { + walk = func(n *Node) { result = append(result, n) if n.IsExpanded { for _, child := range n.Children { - walk(child, depth+1) + walk(child) } } } - walk(root, 0) + walk(root) return result } diff --git a/internal/models/fileExplorer/fileExplorer.go b/internal/models/fileExplorer/fileExplorer.go index 1de49b8..97c356a 100644 --- a/internal/models/fileExplorer/fileExplorer.go +++ b/internal/models/fileExplorer/fileExplorer.go @@ -76,6 +76,12 @@ func (m *FileExplorer) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.CurrentNode = m.VisibleNodes[m.CurrentIndex] return m, m.SelectionChanged(m.CurrentNode) case "enter": + // It's possible for currentNode to be nil if the file explorer is empty + // This check prevents the application from crashing if the user presses enter in that state + if m.CurrentNode == nil { + return m, nil + } + if m.CurrentNode.IsDirectory { m.CurrentNode.IsExpanded = !m.CurrentNode.IsExpanded m.VisibleNodes = filetree.FlattenVisibleTree(m.Root) @@ -137,7 +143,13 @@ func (m FileExplorer) View() string { w := (m.Width / 4) - 1 h := m.Height - 2 - s := filetree.BuildNodeTree(m.Root, "", len(m.Root.Children) == 0, m.CurrentNode) + var s string + // A check is added to endure m.Root is not nil before trying to build thre tree. + // This prevents a panic if the View is rendered after the initial CreateTree failed + // or if the tree becomes empty after a refresh + if m.Root != nil { + s = filetree.BuildNodeTree(m.Root, "", len(m.Root.Children) == 0, m.CurrentNode) + } return style.Width(w).Height(h).MarginTop(1).Render(s) } @@ -148,6 +160,12 @@ func (m *FileExplorer) Resize(w int, h int) { } func (m *FileExplorer) SelectionChanged(node *filetree.Node) tea.Cmd { + // Added a guard clause to if this function is ever called with a nil node. + // Makes function more robust against unexpected states + if node == nil { + return nil + } + path := filepopup.GetPath(node) if node.IsDirectory || m.LastSelection == path { return nil @@ -163,24 +181,58 @@ func (m *FileExplorer) SelectionChanged(node *filetree.Node) tea.Cmd { } func (m *FileExplorer) Refresh() { - newRoot, _ := filetree.CreateTree() + newRoot, err := filetree.CreateTree() + + // From last state this function was a bit of a *respectful mess* lol. So we're going to try to clean it up a little bit at a time. + + // The OG code ignored the error from CreateTree, (e.g due to a permissions issue), NewRoot would be nil. We now handle the error by aborting the refresh, keeping app stable. - filepopup.MapExpanded(newRoot, m.Root) + if err != nil { + // Log or handlme the error appropriately instead of ignorin g it + // FOr now, we'll just abort the refresh + return + } + + // Check ensures we don't try to map expanded states from a nil m.Root, which could happen in rare cases as such. It's small change but added safety. + + if m.Root != nil { + filepopup.MapExpanded(newRoot, m.Root) + } m.Root = newRoot m.VisibleNodes = filetree.FlattenVisibleTree(newRoot) + // After a refresh (e.g deleting the last file in a dir), the list of visible nodes might be empty. Graceful handling added. Resetting the selection from accessing an empty slice. + if len(m.VisibleNodes) == 0 { + m.CurrentIndex = 0 + m.CurrentNode = nil + return + } + idx := -1 - for i, val := range m.VisibleNodes { - if val.Name == m.CurrentNode.Name && filepopup.GetPath(val) == filepopup.GetPath(m.CurrentNode) { - idx = i + // we only search for the old node if one was actually selected. This avoids nil pointer deference on m.CurrentNode if it was nil before the refresh. + if m.CurrentNode != nil { + currentPath := filepopup.GetPath(m.CurrentNode) + for i, val := range m.VisibleNodes { + if val.Name == m.CurrentNode.Name && filepopup.GetPath(val) == currentPath { + idx = i + // break out of the loop once we find the node + break + } } } + // Old logic for finding index was buggy and could lead to an index of -1. Added more robustness. + if idx == -1 { - if m.CurrentIndex != 0 { + // If the previously selected node is gone (e.g. deleted), + // attempt to select the previous index, but ensure it's within bounds. + if m.CurrentIndex > 0 && m.CurrentIndex-1 < len(m.VisibleNodes) { idx = m.CurrentIndex - 1 + } else { + // Default to the first item if the old index is invalid. + idx = 0 } }