search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Correctly normalize UNC paths #5517

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by iwehrman Tuesday Nov 19, 2013 at 00:39 GMT Originally opened as https://github.com/adobe/brackets/pull/6041

Add support for normalization of UNC paths in the filesystem. _normalizePath is now a method of the FileSystem class. Its behavior w.r.t. UNC paths is controlled by a new FileSystemImpl flag, normalizeUNCPaths. If set, paths like ///server//foo/bar are normalized to //server/foo/bar/ instead of /server/foo/bar/. AppshellFileSystemImpl.normalizeUNCPaths is set at runtime for Windows only.

Addresses #6028 and #6029.

CC@gruehle@peterflynn

iwehrman included the following code: https://github.com/adobe/brackets/pull/6041/commits

core-ai-bot commented 3 years ago

Comment by iwehrman Tuesday Nov 19, 2013 at 01:23 GMT

Pushed one fix for the RE naming and JSDoc. I'm happy to throw an exception in case _normalizePath is called before init(). I didn't do that initially because it seemed potentially disruptive and probably not useful. (It's also kind of hard to grep through extensions to see if this is common.)

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 :)

core-ai-bot commented 3 years ago

Comment by iwehrman Tuesday Nov 19, 2013 at 01:36 GMT

I now think that, yes, we should throw if you try to normalize a path before init. Will fix tomorrow.

core-ai-bot commented 3 years ago

Comment by iwehrman Tuesday Nov 19, 2013 at 18:15 GMT

The fixing is taking a little longer than I expected. I'm in require.js hell.

core-ai-bot commented 3 years ago

Comment by peterflynn Tuesday Nov 19, 2013 at 18:24 GMT

@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.

core-ai-bot commented 3 years ago

Comment by iwehrman Tuesday Nov 19, 2013 at 19:34 GMT

Ready for re-review.

core-ai-bot commented 3 years ago

Comment by iwehrman Tuesday Nov 19, 2013 at 20:19 GMT

Impl documentation updated accordingly.

core-ai-bot commented 3 years ago

Comment by gruehle Tuesday Nov 19, 2013 at 20:35 GMT

Looks good. Merging.