Skip to content

Commit 1bae135

Browse files
committed
fix(tree): Use consistent forward slash path separator for Windows compatibility
The walker normalizes all paths to forward slashes via filepath.ToSlash(), but display and tree building code used os.PathSeparator which is "\" on Windows. This caused path matching failures (e.g., HasPrefix, Contains) resulting in a flattened directory tree on Windows/Git Bash/WSL. Changes: - Replace os.PathSeparator with "/" in display.go and view.go - Use path.Join instead of filepath.Join in tree.go for consistent "/" - Add regression test for deeply nested directory structures Fixes #19
1 parent 977887a commit 1bae135

6 files changed

Lines changed: 73 additions & 10 deletions

File tree

internal/generator/tree.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package generator
33
import (
44
"fmt"
55
"os"
6+
"path"
67
"path/filepath"
78
"runtime"
89
"sort"
@@ -50,12 +51,12 @@ func (g *Generator) buildTree() *Node {
5051
}
5152
sort.Strings(paths)
5253
dirSet := make(map[string]bool)
53-
for _, path := range paths {
54-
parts := strings.Split(path, string(os.PathSeparator))
54+
for _, p := range paths {
55+
parts := strings.Split(p, "/")
5556
current := root
5657
fullPath := ""
5758
for i, part := range parts {
58-
fullPath = filepath.Join(fullPath, part)
59+
fullPath = path.Join(fullPath, part)
5960
isLast := i == len(parts)-1
6061
found := false
6162
isDir := !isLast || dirSet[fullPath]

internal/generator/tree_benchmark_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ func createTestTree(rootPath string) *Node {
168168
}
169169

170170
// Build path segments
171-
parts := strings.Split(relPath, string(os.PathSeparator))
171+
parts := strings.Split(filepath.ToSlash(relPath), "/")
172172
current := root
173173

174174
for i, part := range parts {

internal/generator/tree_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ import (
77
"testing"
88
)
99

10+
// TestBuildTree tests tree building from selected files.
11+
// Note: All internal paths use forward slashes ("/") for cross-platform compatibility.
12+
// The walker normalizes paths with filepath.ToSlash(), so tree building must use
13+
// path.Join (not filepath.Join) to maintain consistency.
1014
func TestBuildTree(t *testing.T) {
1115
tempDir, err := os.MkdirTemp("", "tree-test")
1216
if err != nil {

internal/model/display.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package model
22

33
import (
4-
"os"
54
"path/filepath"
65
"sort"
76
"strings"
@@ -51,14 +50,14 @@ func (m *Model) buildDisplayNodes() {
5150
m.displayNodes = append(m.displayNodes, node)
5251

5352
if item.IsDir && !m.collapsed[item.Path] {
54-
prefix := item.Path + string(os.PathSeparator)
53+
prefix := item.Path + "/"
5554
var children []filesystem.FileItem
5655
directChildren := make(map[string]filesystem.FileItem)
5756

5857
for _, f := range m.files {
5958
if strings.HasPrefix(f.Path, prefix) {
6059
sub := strings.TrimPrefix(f.Path, prefix)
61-
if !strings.Contains(sub, string(os.PathSeparator)) {
60+
if !strings.Contains(sub, "/") {
6261
directChildren[f.Path] = f
6362
}
6463
}
@@ -82,7 +81,7 @@ func (m *Model) buildDisplayNodes() {
8281
}
8382

8483
for _, item := range m.files {
85-
if !strings.Contains(item.Path, string(os.PathSeparator)) {
84+
if !strings.Contains(item.Path, "/") {
8685
rootItems = append(rootItems, item)
8786
}
8887
}

internal/model/display_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import (
77
"github.com/epilande/codegrab/internal/filesystem"
88
)
99

10+
// TestBuildDisplayNodes tests the display node building.
11+
// Note: All paths use forward slashes ("/") for cross-platform compatibility.
12+
// The walker normalizes all paths to use "/" regardless of OS.
1013
func TestBuildDisplayNodes(t *testing.T) {
1114
m := Model{
1215
selected: make(map[string]bool),
@@ -91,3 +94,60 @@ func TestBuildDisplayNodes(t *testing.T) {
9194
t.Errorf("Expected dir1/file1.txt to be included in display nodes when dir1 is expanded")
9295
}
9396
}
97+
98+
// TestBuildDisplayNodesDeeplyNested tests that deeply nested paths work correctly.
99+
// This is important for cross-platform compatibility since paths must use "/"
100+
// consistently (not os.PathSeparator which would be "\" on Windows).
101+
func TestBuildDisplayNodesDeeplyNested(t *testing.T) {
102+
m := Model{
103+
selected: make(map[string]bool),
104+
deselected: make(map[string]bool),
105+
collapsed: make(map[string]bool),
106+
isDependency: make(map[string]bool),
107+
files: []filesystem.FileItem{
108+
{Path: "a", IsDir: true, Level: 0},
109+
{Path: "a/b", IsDir: true, Level: 1},
110+
{Path: "a/b/c", IsDir: true, Level: 2},
111+
{Path: "a/b/c/d", IsDir: true, Level: 3},
112+
{Path: "a/b/c/d/file.txt", IsDir: false, Level: 4},
113+
{Path: "a/b/other.txt", IsDir: false, Level: 2},
114+
},
115+
}
116+
117+
m.buildDisplayNodes()
118+
119+
// With all directories expanded (default), we should see all nodes
120+
expectedPaths := []string{
121+
"a",
122+
"a/b",
123+
"a/b/c",
124+
"a/b/c/d",
125+
"a/b/c/d/file.txt",
126+
"a/b/other.txt",
127+
}
128+
129+
if len(m.displayNodes) != len(expectedPaths) {
130+
t.Fatalf("Expected %d display nodes, got %d", len(expectedPaths), len(m.displayNodes))
131+
}
132+
133+
for i, expectedPath := range expectedPaths {
134+
if m.displayNodes[i].Path != expectedPath {
135+
t.Errorf("Node %d: expected path %q, got %q", i, expectedPath, m.displayNodes[i].Path)
136+
}
137+
}
138+
139+
// Collapse a/b and verify children are hidden
140+
m.collapsed["a/b"] = true
141+
m.buildDisplayNodes()
142+
143+
// Should only show a and a/b (children of a/b are hidden)
144+
if len(m.displayNodes) != 2 {
145+
t.Fatalf("Expected 2 display nodes when a/b is collapsed, got %d", len(m.displayNodes))
146+
}
147+
if m.displayNodes[0].Path != "a" {
148+
t.Errorf("Expected first node to be 'a', got %q", m.displayNodes[0].Path)
149+
}
150+
if m.displayNodes[1].Path != "a/b" {
151+
t.Errorf("Expected second node to be 'a/b', got %q", m.displayNodes[1].Path)
152+
}
153+
}

internal/model/view.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package model
22

33
import (
44
"fmt"
5-
"os"
65
"path/filepath"
76
"strings"
87

@@ -627,7 +626,7 @@ func (m *Model) getSelectedFileCount() int {
627626
effectiveSelection[item.Path] = true
628627
} else {
629628
// For selected directories, count all non-deselected files
630-
prefix := item.Path + string(os.PathSeparator)
629+
prefix := item.Path + "/"
631630
for _, child := range m.files {
632631
if !child.IsDir && strings.HasPrefix(child.Path, prefix) && !m.deselected[child.Path] {
633632
if isFilteringBySearch && !searchResultPaths[child.Path] {

0 commit comments

Comments
 (0)