Add comment support

[dduponchel]

This commit adds the ability to read and generate comments on the zip file and/or on each zip entry, see #134.
The entry comment is set on ZipObject#comment and the zip file main comment is set on JSZip#comment. For example :

var zip = new JSZip(data);
zip.comment // contains the comment of the loaded zip
zip.generate({type:"blob", comment: "other comment"}); // generate a zip with a new comment

zip.file("Hello.txt").comment // contains the comment of the loaded entry, if any
zip.file("World.txt", "content", {comment : "comment here"}); // create a new file with a comment

I also had issues with saucelabs tests and I resolved them with an upgrade of the grunt plugin (I also upgraded other dev dependencies).

[Stuk]

I think the .comment property should be directly on the JSZip object (i.e. zip.comment). It doesn't seem like an option, but actually some data associated with it.

I'm not so sure about the file/ZipObject. It still seems like it should be directly on the object, because it's not an option like base64 is. However we have precedent of other data like date being under .options. I don't know what's more important: consistency with .options.date or consistency of having .comment either both under .options or not.

Perhaps promote date up to the ZipObject too.

What do you think?

[dduponchel]

I added JSZip#options to match ZipObject#options but you're right, JSZip#comment makes more sense.
I think that we have 3 types of informations in ZipObject#options :

• informations about the data format : base64 and binary (and sometimes optimizedBinaryString)
• metadata : comment, dir and date
• hints on the result we want when generating the zip file : compression

The first type is not really useful (for the user) and we could even remove them from the public API.
options is clearly not the right namespace for the second type. Putting them directly on ZipObject seems OK (we would get zipFile.name, zipFile.date, zipFile.comment, etc).

If we don't deprecate and move half of the attributes, I prefer a consistent date / comment under options. zip.comment and zipFile.options.comment are not consistent but they are on two different entities.

What do you think ?

[Stuk]

That's a great analysis of the different types we have.

I would prefer the later option of deprecating/moving the properties around. zipFile.name, zipFile.date, zipFile.comment all look much better than being under options.

[dduponchel]

Done : now the only attribute that's not deprecated in ZipObject#options is compression.
date, dir and comment are now attributes of ZipObject while the other (base64 and binary) are deprecated without replacement.

[zenorocha]

Awesome! Thanks @dduponchel :)

[brunocoelho]

👍

[dduponchel]

@Stuk I think we can merge this one if you are ok with my last commit.

[henvic]

+1

[Stuk]

The changes look good. I'm slightly concerned by back-compat issues. How likely is it that people are setting ZipObject#options.date = ... and expecting it to work?

[dduponchel]

The file(name) method returns a copy of ZipObject, updating it won't change the generated zip file.
The only way to get the issue would be to use the (undocumented until this PR) JSZip#files attribute to get a direct access to the ZipObject and change the date. I'm sure our users are nice people who don't use undocumented features :)

Nice catch, I'm updating the pull request.

[Stuk]

Great, thanks!

[zenorocha]

@Stuk can you release a new version of it?

[Stuk]

@dduponchel is there anything you want to get in before another release? I'm thinking #141 could wait until the release after.

[dduponchel]

Yes, #141 can wait. We already have a fix for #142 (dduponchel/jszip@7364428 and dduponchel/jszip@629b96e from #141), I will create a pull request to include them quickly. After that, a release seems OK. 2.3 ?

[Stuk]

Cool, 2.3 sounds good.