Skip to content

Conversation

@TWDickson
Copy link
Contributor

Windows compatibility failed due to hardcoded unix path separators in file tree generation and gitignore pattern matching. I have not tested on a Unix based system.

  • Normalize paths using filepath.ToSlash() in processor for cross-platform gitignore matching
  • Add defensive path normalization to filetree package using strings.FieldsFunc
  • Add comprehensive tests for mixed path separators and edge cases

- Normalize paths using filepath.ToSlash() in processor for cross-platform gitignore matching - Add defensive path normalization to filetree package using strings.FieldsFunc - Add comprehensive tests for mixed path separators and edge cases
@fredoliveira fredoliveira self-requested a review June 19, 2025 15:30
Copy link
Member

@fredoliveira fredoliveira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Taylor! Thanks for submitting this!

I tested this locally and found one small issue - the filepath.ToSlash() approach doesn't handle literal backslashes on Unix systems. I believe this will work across both Windows and unix-based systems:

// In filetree.go, replace the normalization logic with:
parts := strings.FieldsFunc(path, func(r rune) bool {
    return r == '/' || r == '\\'
})

This handles both separators directly without string replacement and removes the need for the filepath import (you can remove that too). All tests should pass with this change :)

@TWDickson
Copy link
Contributor Author

Updated and it looks like it's working!

Copy link
Member

@fredoliveira fredoliveira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TWDickson looks great! Thank you for updating the code. Merging!

@fredoliveira fredoliveira merged commit 08b716c into holonoms:main Jun 19, 2025
1 check passed
@TWDickson TWDickson deleted the fix/windows-path-compatibility branch June 19, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants