Skip to content

Commit 2fa40c3

Browse files
committed
Address Copilot review on pathutil filters
- FilterPaths/ListEntries: feed slash-separated paths into runFilters via filepath.ToSlash so BaseFilter/ExtensionFilter/ComponentFilter work on Windows. Returned paths stay OS-native. - BySortablePathComponents.Less: tie-break on AbsPth then Pth after the base-name comparison so same-depth same-base entries sort deterministically (sort.Sort is not stable). - ComponentWithExtensionFilter: case-insensitive (strings.EqualFold) to match BaseFilter/ExtensionFilter. - sortable_path_test: POSIX-only assertions (filepath.Abs differs on Windows). Skip on GOOS=windows rather than try to make path-mechanics tests platform-neutral; CI already runs Linux/macOS only. RegexpFilter.MustCompile is left as-is: callers pass literal patterns at construction time, so an invalid pattern surfaces in their own unit tests, not in production.
1 parent afdb4f9 commit 2fa40c3

4 files changed

Lines changed: 46 additions & 6 deletions

File tree

pathutil/path_filter.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,12 @@ func ComponentFilter(component string, allowed bool) FilterFunc {
7979
}
8080

8181
// ComponentWithExtensionFilter matches entries whose path has at least one
82-
// component with the given extension.
82+
// component with the given extension (case-insensitive, leading dot
83+
// included, e.g. ".xcodeproj").
8384
func ComponentWithExtensionFilter(ext string, allowed bool) FilterFunc {
8485
return func(pth string, _ fs.DirEntry) (bool, error) {
8586
found := slices.ContainsFunc(strings.Split(pth, "/"), func(c string) bool {
86-
return path.Ext(c) == ext
87+
return strings.EqualFold(path.Ext(c), ext)
8788
})
8889
return allowed == found, nil
8990
}

pathutil/path_filter_compat.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func FilterPaths(paths []string, filters ...FilterFunc) ([]string, error) {
3131
return nil, err
3232
}
3333

34-
keep, err := runFilters(pth, d, filters)
34+
keep, err := runFilters(filepath.ToSlash(pth), d, filters)
3535
if err != nil {
3636
return nil, err
3737
}
@@ -61,7 +61,7 @@ func ListEntries(dir string, filters ...FilterFunc) ([]string, error) {
6161
var filtered []string
6262
for _, entry := range entries {
6363
pth := filepath.Join(absDir, entry.Name())
64-
keep, err := runFilters(pth, entry, filters)
64+
keep, err := runFilters(filepath.ToSlash(pth), entry, filters)
6565
if err != nil {
6666
return nil, err
6767
}

pathutil/sortable_path.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ func NewSortablePath(pth string) (SortablePath, error) {
3939
}
4040

4141
// BySortablePathComponents sorts SortablePath values by component depth
42-
// (shallowest first), breaking ties alphabetically on the base name.
42+
// (shallowest first), breaking ties alphabetically on the base name and
43+
// falling back to the absolute and original paths to keep the order
44+
// deterministic when same-depth same-base entries exist.
4345
type BySortablePathComponents []SortablePath
4446

4547
func (s BySortablePathComponents) Len() int { return len(s) }
@@ -48,7 +50,14 @@ func (s BySortablePathComponents) Less(i, j int) bool {
4850
if len(s[i].Components) != len(s[j].Components) {
4951
return len(s[i].Components) < len(s[j].Components)
5052
}
51-
return filepath.Base(s[i].AbsPth) < filepath.Base(s[j].AbsPth)
53+
baseI, baseJ := filepath.Base(s[i].AbsPth), filepath.Base(s[j].AbsPth)
54+
if baseI != baseJ {
55+
return baseI < baseJ
56+
}
57+
if s[i].AbsPth != s[j].AbsPth {
58+
return s[i].AbsPth < s[j].AbsPth
59+
}
60+
return s[i].Pth < s[j].Pth
5261
}
5362

5463
// SortPathsByComponents returns paths sorted by directory depth.

pathutil/sortable_path_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,26 @@ package pathutil
33
import (
44
"os"
55
"path/filepath"
6+
"runtime"
67
"testing"
78

89
"github.com/stretchr/testify/require"
910
)
1011

12+
// These tests assume POSIX-style absolute paths (no drive letter, "/" as
13+
// separator). filepath.Abs and component splitting differ on Windows, so
14+
// the bare-string assertions would not hold there. CI for go-utils runs on
15+
// Linux and macOS only; skip on Windows to keep the test honest.
16+
func skipOnWindows(t *testing.T) {
17+
t.Helper()
18+
if runtime.GOOS == "windows" {
19+
t.Skip("POSIX-only: filepath.Abs and component split differ on Windows")
20+
}
21+
}
22+
1123
func TestNewSortablePath(t *testing.T) {
24+
skipOnWindows(t)
25+
1226
sp, err := NewSortablePath("/a/b/c.txt")
1327
require.NoError(t, err)
1428
require.Equal(t, "/a/b/c.txt", sp.Pth)
@@ -17,6 +31,8 @@ func TestNewSortablePath(t *testing.T) {
1731
}
1832

1933
func TestSortPathsByComponents(t *testing.T) {
34+
skipOnWindows(t)
35+
2036
input := []string{
2137
"/a/b/c/deep.txt",
2238
"/x.txt",
@@ -35,13 +51,27 @@ func TestSortPathsByComponents(t *testing.T) {
3551
}
3652

3753
func TestSortPathsByComponents_tiebreakAlphabetic(t *testing.T) {
54+
skipOnWindows(t)
55+
3856
input := []string{"/a/zeta.txt", "/a/alpha.txt", "/a/mu.txt"}
3957

4058
got, err := SortPathsByComponents(input)
4159
require.NoError(t, err)
4260
require.Equal(t, []string{"/a/alpha.txt", "/a/mu.txt", "/a/zeta.txt"}, got)
4361
}
4462

63+
func TestSortPathsByComponents_tiebreakSameBaseDifferentDir(t *testing.T) {
64+
skipOnWindows(t)
65+
66+
// Same depth, same base name — without the AbsPth tie-breaker the
67+
// resulting order would be nondeterministic (sort.Sort is not stable).
68+
input := []string{"/b/x.txt", "/a/x.txt"}
69+
70+
got, err := SortPathsByComponents(input)
71+
require.NoError(t, err)
72+
require.Equal(t, []string{"/a/x.txt", "/b/x.txt"}, got)
73+
}
74+
4575
func TestListPathInDirSortedByComponents(t *testing.T) {
4676
root := t.TempDir()
4777
mustWrite(t, filepath.Join(root, "top.txt"), "x")

0 commit comments

Comments
 (0)