fix: handle Windows separators in recursive find paths#1255
Open
airforce000 wants to merge 1 commit into
Open
Conversation
e6eafde to
aa6bc28
Compare
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.
Summary
Fixes #1227.
This fixes recursive
find()/ls -Rbehavior on Windows when the input path uses native backslash separators, such as a path created bypath.join().The underlying issue is in
ls's recursive glob path construction.find()calls recursivels()for directory contents, and on Windows a native path likeproject\sourcewas being passed tofast-globas a mixed-separator pattern:fast-globexpects/separators in glob patterns, so the directory itself could be returned without the recursive contents. This patch normalizes the glob pattern to forward slashes on Windows before appending/using the recursive glob.Why this approach
ls()glob boundary, where the mixed-separator pattern is created.ls()/find(), so this keeps the existing output behavior.Test coverage
I added a regression test for
shell.find(path.join('test', 'resources', 'find')). This is the case that creates native Windows backslashes and reproduces the issue.There was an earlier closed PR (#1228) that added an absolute-path test, but it built the path with
/separators and did not include an implementation fix. This test covers the failing Windows-native input shape.Local verification
On Windows, before the fix, this minimal repro returned only the directory for
path.join()input:{"input":"project\\source","result":["project/source"],"length":1,"code":0}After the fix, the same repro returns the nested file as expected:
{"input":"project\\source","result":["project/source","project/source/index.js"],"length":2,"code":0}I also verified absolute Windows paths with backslashes recurse correctly.
Commands run locally:
Note: the full
test/find.jsrun on my Windows checkout hits the existing symlink fixture issue wheretest/resources/find/dir2_linkis checked out as a plain file rather than a symlink, so I excluded only that unrelated symlink-specific assertion for the local Windows run.