Handle UNC paths in windows (fixes #23)
@sokra Please review this, as it affects the logic outside of just pathToArray (normalisation).
needs test cases for normalization
i. e. fs.normalize("\\\\a\\b/../c\\d\\..").should.be.eql("\\\\a\\c");
Codecov Report
Merging #28 into master will increase coverage by
0.47%. The diff coverage is100%.
@@ Coverage Diff @@
## master #28 +/- ##
==========================================
+ Coverage 98.78% 99.25% +0.47%
==========================================
Files 3 3
Lines 246 268 +22
Branches 46 49 +3
==========================================
+ Hits 243 266 +23
+ Misses 3 2 -1
| Impacted Files | Coverage Δ | |
|---|---|---|
| lib/normalize.js | 100% <100%> (ø) |
:white_check_mark: |
| lib/join.js | 100% <100%> (ø) |
:white_check_mark: |
| lib/MemoryFileSystem.js | 99.1% <100%> (+0.58%) |
:white_check_mark: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 0a12b5c...3ccfa79. Read the comment docs.
@sokra are you certain regarding addition/support of this test case?
fs.normalize("\\\\a\\b/../c\\d\\..").should.be.eql("\\\\a\\c");
It would make the code quite ugly. That is, I had a few dabs at it, and reverted. The fact that in *nix a file/ directory name can contain a backslash (such as in the case of this test), makes it particularly ugly.
Besides, none of the existing test cases would support an equivalent mix of *nix and windows DS, e.g. these will fail:
"/a/b/c/..\\.."
// AssertionError: expected '/a/b/c/..\\..' to equal '/a'
"C:\\a\\b\\\c\\../.."
// AssertionError: expected 'C:\\a\\b\\c\\../..' to equal 'C:\\a'
am I overlooking something?
oh yeah, I was wrong. the normalization does only care for . and ... The path joining corrects the slashes. Here the absoluteWinRegexp need to include UNC paths.
Here the absoluteWinRegexp need to include UNC paths.
Whats the need for the addition?
What test case am I missing?
https://github.com/webpack/memory-fs/pull/28/commits/407053104022c5cf90b640efe04bdd21073d8349
As it is, all scenarios are covered, as far as my testing goes.
@sokra I had a second look into this. I don't see what you want to achieve by adding the UNC path support to absoluteWinRegexp. UNC paths are already handled as expected, except in cases when there is a mix of nix and windows DS, which does not work with the current test suite anyway.
I am happy to add the support for mixing windows/nix DS, though as I have mentioned earlier, it will require some refactoring and it is not going to work in all cases, since nix paths can contain backwards slashes in file/directory names.
mixed windows/nix paths will occur. i. e. when resolving ./relative/path in \\srv\share\src.
But windows style path separators will not appear in linux paths.
So \\srv\share\src/relative/path is possible, but /root/src\relative\path is not possible (it's possible but \ is not a path separator in this case). Because of this lucky circumstance we do not have a problem with backwards slashes in linux paths.
oh yeah, I was wrong. the normalization does only care for . and ... The path joining corrects the slashes. Here the absoluteWinRegexp need to include UNC paths.
3ccfa79
So \srv\share\src/relative/path is possible, but /root/src\relative\path is not possible (it's possible but \ is not a path separator in this case). Because of this lucky circumstance we do not have a problem with backwards slashes in linux paths.
I have fixed the refactoring issue and fixed the join function for use with UNC paths. However, I am leaving the normalisation of a mix of path to issue open. I don't see how it can be solved without major refactoring of normalise.js.