Windows path issues
Dids opened this issue · 5 comments
I'm using path.normalize() basically everywhere, but yazl is throwing me the following error:
Error: invalid characters in path: fonts\fontawesome-webfont.eot
Is yazl cross-platform/compatible with Windows?
I've created pull request #19 that fixes this (tested on Windows only, but I don't see why it wouldn't also work on Linux and OS X).
Yes, yazl is compatible with Windows, but the zip file format does not allow native Windows paths, as I'm sure you've discovered. This is documented in yazl's API "A valid metadataPath...".
#19 adds a convenience layer of translation for the metadataPath similar to what we're already doing for appending a trailing / for directory entries. it converts illegal \ characters into legal / characters. This translation seems pretty safe. I like it.
However, #19 also calls path.normalize() which doesn't seem right. If path.normalize() changes something, that seems like there really was a problem with the input. path.normalize() is only required for certain types of applications that build paths using complex logic or user input. In these cases, it seems like it's the calller's responsibility to normalize the path before passing it in as an argument. Furthermore, yazl throwing an exception on non-normalized input might be helpful to catch some kinds of errors.
In general, I don't want yazl's API to be too high-level, but translating Windows paths to Unix paths seems ok.
I'll take some action on this when i get home later today.
I've previously used path.* helpers to abstract away any platform specific tasks, including normalizing the path for any OS that might be running the app. That said, I agree that it shouldn't be necessary here, and I only included it due to old habits dying hard.
Simply doing the slash conversion should work just fine. I'll be happy with any solution, as long as this gets addressed at some point. :)
I'll make a release when I get #6 implemented as well.
I'll make a release when I get #6 implemented as well.
Never mind. 2.3.0 is released now with this change.