Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented multi member gz decoder #43

Merged
merged 1 commit into from
Feb 2, 2017

Conversation

veldsla
Copy link
Contributor

@veldsla veldsla commented Jun 14, 2016

You mentioned in issue #41 that you were considering creating a separate type for the multistream gzip files. The nested loop method I used to parse these files in #41 (comment) becomes a bit convoluted when parsing multiple files.

Considering my rust beginner level I used it as an exercise. It works by calling finish_member instead of finish and checking if there are more bytes remaining in the buffer. If this is the case a new header read is attempted and the decoder is reset for the next member. Would this be what you had in mind?

Added a MultiDecoderReader and MultiDecoderReaderBuf to gz. I moved
the read header code to a separate function to avoid code duplication.

In order to keep the reader but reset the decoder a reset_data method
has been added to deflate.rs and a reset method to the CrcReader.

Tests for multimember gz files are in the test directory.
@alexcrichton
Copy link
Member

@veldsla I'm so sorry this sat here for so long! I had no idea this was here :(

In any case this looks excellent, thanks so much for implementing this! In the future feel free to ping me on any PR that sits for awhile!

@alexcrichton alexcrichton merged commit b36a942 into rust-lang:master Feb 2, 2017
@veldsla
Copy link
Contributor Author

veldsla commented Feb 2, 2017

Haha, that's great. You did reference this PR in #44, so I just assumed you didn't want to add more functionality. Oh well assumptions are...😄

@alexcrichton
Copy link
Member

Weird... I maybe accidentally archived the email?

@pmarks
Copy link

pmarks commented Apr 10, 2017

@alexcrichton - any chance we could get a crates.io release w/ this functionality? Would like to start encouraging downstream packages to use MultiGzDecoder. Many uses cases where MultiGz files are the norm (especially bioinformatics).

@veldsla
Copy link
Contributor Author

veldsla commented Apr 11, 2017

There has been a library evaluation which among other things raised a point about the documentation (#87). I also noticed a doc has been swapped between a multi/normal reader. I will submit a PR for this. I'm guessing the next release will come when #89 is done?

@alexcrichton
Copy link
Member

@pmarks certainly! I'll do a release right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants