search

c9 / vfs-local

A VFS implementation for the local file-system.
MIT License
32 stars 39 forks source link

Slash handling fix for Windows #5

Closed davidebbo closed 12 years ago

davidebbo commented 12 years ago

I'm now using path.normalize("/") to get the separator since path.sep is not available in Node 0.6.

creationix commented 12 years ago

What about people who include the drive letter in their root? Does this assume paths start with \ on windows?

davidebbo commented 12 years ago

Right now, I'm simply omitting that check on Windows:

if (pathSep == "/" && root[0] !== "/") throw new Error("root path must start in /");

This basically says that this extra error check only happens on non-Windows platforms. Ideally, we'd call a path.isrooted() API is there was one, but I don't see one. We could also hand write that logic ourselves (check for c:\bar or \bar or \some\unc), but then we're adding too much Windows specific logic.

So I figured omitting the check is the lesser evil, and it unblocks the use of this module on Windows.

davidebbo commented 12 years ago

Do you need more info about this? Note that I've tested it on Windows with driver letter path, slash path, and UNC path, and it all seems to work fine.

creationix commented 12 years ago

No, I was just busy. Sorry for the delay.

davidebbo commented 12 years ago

Thanks! Didn't mean to rush you; just not familiar with the process.

Would I push my luck if I asked for the NPM package to be refreshed with this?

creationix commented 12 years ago

Published as 0.3.10. Thanks for the patch!

davidebbo commented 12 years ago

That's great, thanks Tim!