fix: normalize path separators in CleanPath for cross-platform consistency#88
Open
fix: normalize path separators in CleanPath for cross-platform consistency#88
Conversation
…tency CleanPath was using filepath.Separator to split and join path components, which is '\' on Windows. This caused two problems: - Paths using '/' (e.g. 'foo/bar/baz') were not split on Windows - Paths using '\' (e.g. 'foo\bar\baz') were incorrectly split on Windows but the backslashes were stripped as invalid chars on Unix Fix: normalize both '\' and '/' to '/' before splitting, and always join with '/'. This produces consistent output on all platforms regardless of which separator style is used in the input. Also update the 'several dirs2' test case expected value from 'foobarbaz' to 'foo/bar/baz', since backslash-separated input now correctly splits into path components rather than having the backslashes stripped as invalid chars. Fixes #87 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes CleanPath so it treats both \ and / as path separators across platforms, preventing Windows from treating /-separated paths as a single filename segment and stripping slashes as invalid characters.
Changes:
- Normalize
\to/before splitting path components inCleanPath. - Always join cleaned path components with
/and remove the now-unusedpath/filepathimport. - Update the
several dirs2test expectation to reflect the corrected separator behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pathological.go |
Normalizes separators before splitting and always rejoins with /, removing platform-dependent behavior. |
pathological_test.go |
Updates a path test case to expect /-separated output for backslash-separated input. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CleanPathusedfilepath.Separatorto split and join path components. On Windows that separator is\, so paths written with/(e.g."foo/bar/baz") were never split -- the whole string was treated as a single filename component and the slashes were stripped as invalid characters. Fixes #87.What changed
CleanPathnow normalizes both\and/to/before splitting, and always joins with/. This means:"foo/bar/baz"->"foo/bar/baz"on every platform (was broken on Windows)"foo\bar\baz"->"foo/bar/baz"on every platform (backslashes treated as separators, not invalid chars)/-separated, matching the documented behavior in the READMEThe unused
path/filepathimport is removed.One test case (
several_dirs2) expected"foobarbaz"because it was written assuming backslashes would be stripped as invalid characters. That expectation is updated to"foo/bar/baz"to reflect the corrected, consistent behavior.All 57 tests pass on Windows.