Repository navigation
Support decompressor plugins - #427
Conversation
jdleesmiller
left a comment
There was a problem hiding this comment.
Thanks for the PR and sorry for the long turnaround. This looks great to me 👍. Just a few suggestions.
I think this could go out in a semver minor version bump, probably 2.2 (I am planning to release 2.1 shortly), because I don't see any breaking changes, except to classes marked with :nodoc:. Does that sound right?
Are you planning on releasing a plugin gem with bzip2 compression support? You can of course keep that under your own namespace, but if you would like it to live under the rubyzip org, we can ask @simonoff about setting that up.
|
|
||
| def produce_input | ||
| @decompressor.produce_input | ||
| @decompressor.read(CHUNK_SIZE) |
There was a problem hiding this comment.
There are quite a few CHUNK_SIZEs with the same value already. I wonder whether using Decompressor::CHUNK_SIZE would be appropriate here?
There was a problem hiding this comment.
I think the optimal CHUNK_SIZE for decompressors is dependant on compression method. Thus the Decompressor::CHUNK_SIZE should not exist and be replaced by an implementation dependant CHUCK_SIZE in Inflater.
Therefore, in InputStream, I think it is better to not depend on this Decompressor::CHUNK_SIZE.
Agreed.
Yes, I am planning to create a bzip2 compression plugin gem, after this pull request is accepted. When the plugin gem is ready, I would very much like to transfer control (and maintainership) to rubyzip.org. |
This aligns Inflater#produce_input with PassThruDecompresser#produce_input.
Now, STORED files can be decrypted, just like DEFLATED files.
6f90c6e to
0b9433c
Compare
I think developing a collection of plugins under the rubyzip org is a nice idea. It would also be an opportunity to widen the cohort of maintainers of the library and drive more participation. |
jdleesmiller
left a comment
There was a problem hiding this comment.
Great, thanks for the updates. Spotted one minor thing but I can fix that post-merge.
| retried += 1 | ||
| retry | ||
| end | ||
| rescue Zlib::Error => e |
There was a problem hiding this comment.
| rescue Zlib::Error => e | |
| rescue Zlib::Error |
|
Wallet |
As suggested in #425, this pull request makes it possible to easily extend rubyzip with additional decompressors.