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

impl Drop #12

Open
goertzenator opened this issue Dec 22, 2017 · 10 comments
Open

impl Drop #12

goertzenator opened this issue Dec 22, 2017 · 10 comments

Comments

@goertzenator
Copy link

The Encoders should have impl Drop that finishes the compressed stream upon drop(). BufWriter does something similar (flushes its buffer upon drop()).

My use case is Write trait objects. Consider the following trait object that writes compressed output to stdout...

fn make_writer_object() -> Box<Write> {
    let stdout = io::stdout();
    let buf_stdout = io::BufWriter::new(stdout);
    let gzip = gzip::Encoder::new(buf_stdout).unwrap();
    Box::new(gzip)
}

This doesn't work because the last bit of compressed data gets cut off.

@sile
Copy link
Owner

sile commented Dec 24, 2017

Thanks for your advice. It seems useful.
I added impl Drop for encoders at the commit e7ec99b.
Because this change breaks compatibility with previous versions, I will publish this as the version 0.2.0 (in a few days).

@sile
Copy link
Owner

sile commented Dec 25, 2017

I reverted the above commit because implementing Drop trait directly for encoders seemed to make it difficult to handle errors.

Instead, I tried adding AutoFinish (and AutoFinshUnchecked) for this purpose ( 2fd7870 ).

It can be used as follows:

use libflate::finish::AutoFinish;

fn make_writer_object() -> Box<Write> {
    let stdout = io::stdout();
    let buf_stdout = io::BufWriter::new(stdout);
    let gzip = gzip::Encoder::new(buf_stdout).unwrap();
    Box::new(AutoFinish::new(gzip))
}

@goertzenator
Copy link
Author

For comparison I've also been using tar::Builder which has to solve problems similar to gzip encoder. In tar, if you care about errors you can explicitly call finish() or into_inner() to get a proper result, and if you don't care you can just drop() and ignore any errors. This is a simpler API in my opinion.

Anyway, are you interested in help API with development? I had some ideas about unifying blocking/non-blocking code and supporting concatenated streams. I'm not interested in the underlying compression details, but I do like fiddling with APIs.

@sile
Copy link
Owner

sile commented Jan 2, 2018

For comparison I've also been using tar::Builder which has to solve problems similar to gzip encoder.

Thank you for the information. It seems okay to implement the Drop trait if it follows Rust's practices.

Anyway, are you interested in help API with development? I had some ideas about unifying blocking/non-blocking code and supporting concatenated streams.

Great! Your contributions are welcome.
If your ideas are good, I would like to adopt them.

@DevQps
Copy link

DevQps commented Mar 13, 2019

Sorry for bumping this old issue! But did it eventually land somewhere inside a new version? I see that the 0.2.0 is not yet released right? I am not sure if it was incorporated in another version?

@sile
Copy link
Owner

sile commented Mar 16, 2019

@DevQps Thanks for your question. This feature has not been merged into master branch.

@DevQps
Copy link

DevQps commented Mar 16, 2019

@sile Alright! Thanks for informing us! If there is anything else I can do, feel free to contact!

@sile
Copy link
Owner

sile commented Mar 17, 2019

@DevQps To be honest, I forgot about this feature. If you need it, I will consider it again, but what about it?

@DevQps
Copy link

DevQps commented Mar 17, 2019

@sile No problem! I personally don't need it, so I am not sure whether it would be best to just leave it open for someone to potentially pick this up, or to just close it since it's so old. But I'll leave that up to you! :)

@sile
Copy link
Owner

sile commented Mar 17, 2019

@DevQps I see, thanks! I will keep this PR open.

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

No branches or pull requests

3 participants