Correctly normalize UNC paths #6041
Conversation
…y the FileSystemImpl.normalizeUNCPaths flag
|
|
||
| if (!FileSystem.isAbsolutePath(path)) { | ||
| throw new Error("Paths must be absolute: '" + path + "'"); // expect only absolute paths | ||
| } | ||
|
|
||
|
|
||
| var normalizeUNCPaths = this._impl && this._impl.normalizeUNCPaths, |
There was a problem hiding this comment.
Is it a bug if we get called while this._impl is undefined?
There was a problem hiding this comment.
There is at least one group of instances of this that didn't seem crazy to me: In JSUtils-tests.js, there are calls to FileSystem.getFileForPath within the module scope. I decided not to call this a bug, but I guess I could be convinced otherwise.
|
Pushed one fix for the RE naming and JSDoc. I'm happy to throw an exception in case Less excited about adding a Windows flag to trim out the single colon-in-paths case of normalization, but I don't really have strong feelings either way. Just let me know what you think is best and I'll push the keys. I'm also happy if you just merge because I think both decisions are perfectly fine :) |
|
I now think that, yes, we should throw if you try to normalize a path before init. Will fix tomorrow. |
|
The fixing is taking a little longer than I expected. I'm in require.js hell. |
|
@iwehrman Given that we want to sneak this in as a last-minute fix for EC, maybe we should just do the simple fix for now and clean up the "normalization before init" stuff in a subsequent (master-only) PR... It looks good to merge to me. |
…ize the singleton instance at module load
|
Ready for re-review. |
|
Impl documentation updated accordingly. |
|
Looks good. Merging. |
Add support for normalization of UNC paths in the filesystem.
_normalizePathis now a method of theFileSystemclass. Its behavior w.r.t. UNC paths is controlled by a newFileSystemImplflag,normalizeUNCPaths. If set, paths like///server//foo/barare normalized to//server/foo/bar/instead of/server/foo/bar/.AppshellFileSystemImpl.normalizeUNCPathsis set at runtime for Windows only.Addresses #6028 and #6029.
CC @gruehle @peterflynn