search

sindresorhus / gulp-zip

ZIP compress files
MIT License
270 stars 47 forks source link

Add `createSubFolders` option, default to `true` #30

Closed isochronous closed 10 years ago

isochronous commented 10 years ago

I've got a PR (https://github.com/Stuk/jszip/pull/157) in for JSZip to add a createSubFolders option that will fix #29. This pull request just sends that option (defaulting to true if not provided) along to JSZip in each call to .file.

isochronous commented 10 years ago

Well, the Travis CI build is failing because the npm dependency for jszip was set to ^2.3.1, which won't exist until the JSZip devs publish the new version. Either I can revert the jszip dependency to 2.3.0 (as the changes I made will be benign in old versions) and get the Travis CI build to pass, and then update the version number again after JSZip publishes, or I can leave it as it is, in which case the Travis build will start working again as soon as 2.3.1 comes out. Let me know which you'd prefer.

sindresorhus commented 10 years ago

Why would there need to be an option for this? Seems like it can just be on by default unless I'm missing something.

isochronous commented 10 years ago

It is on by default. However, there are apparently use cases in JSZip where the behavior is undesirable, which is why I left a way for users to disable it if they need to.

sindresorhus commented 10 years ago

Use-cases like?

isochronous commented 10 years ago

Like this: https://github.com/Stuk/jszip/issues/130

My pull request to JSZip has been accepted and merged into master, but I'm not sure when he'll re-publish the npm package.

isochronous commented 10 years ago

He's saying he'll re-publish the package later today, but he wants to change the name of the flag first, so let me update the code in this pull request real quick.

sindresorhus commented 10 years ago

Yeah, I don't really care about that usecase. Just enable it by default without an option.

isochronous commented 10 years ago

You da boss

isochronous commented 10 years ago

Okay, done - I also rebased all of the commits into one commit for easier merging and tracking

isochronous commented 10 years ago

Okay, done

isochronous commented 10 years ago

You were right about the JSZip version number, so I've updated the package.json file to require ^2.4.0 instead.

sindresorhus commented 10 years ago

@isochronous see https://github.com/sindresorhus/gulp-zip/pull/31

isochronous commented 10 years ago

Well, it's nice that it works without adding the option, but since the option is being added (stuk is going to publish it to npm as soon as he gets to a computer) it seems like that would be unnecessary complexity for something that can be addressed by just sending an option. But it's your call man, like I said before, you da boss.

isochronous commented 10 years ago

2.4.0 has been released