Closed emk closed 4 years ago
OK, this should be ready for code review! I feel like this is only at about 90% of my usual standard, because it took longer than I had planned and I'm feeling a bit out of it this morning. So please don't hesitate to suggest to improvements! I think it's probably still possible to share more code between snap::read::FrameDecoder
and snap::write::FrameEncoder
, but this morning I can't see any way to do it without adding a whole pile of potentially-clunky abstractions that feel a little forced. I was able to share the core of the framing code between write
and read
, so that's nice.
Thank you very much for considering this patch. And also for all your other awesome Rust stuff; we actually use several of your tools at Faraday and we're always delighted by the quality.
Please do not feel guilty about how long it took! I also maintain many projects, though not nearly as many as you, and I understand completely.
Please make any changes that you think are appropriate. I fear that any insight I might have today, after a long week, is no special value.
Thank you once again for so many excellent crates! I've used so much of your Rust code over the years, and it's one of the things that I really enjoy about Rust.
Implements https://github.com/BurntSushi/rust-snappy/issues/17.
This patch reorganizes the API to work more like
flate2
and adds a newsnap::read::FrameEncoder
which can be used to compress aRead
object while reading from it. The new interfaces are:snap::Reader
becomessnap::read::FrameDecoder
.snap::Writer
becomessnap::write::FrameEncoder
.snap::read::FrameEncoder
is new.snap::write::FrameDecoder
yet, but it could be added.As for tests:
szip
crate tests are now run as part of CI.snap::write::FrameEncoder
andsnap::read::FrameEncoder
produce matching compressed streams.snap::read::FrameEncoder
is the same with very small and very large buffers (because it uses a faster code path for larger buffers).I was able to refactor out the core of
snap::write::FrameEncoder
and re-use it, but the the twosnap::read
implementations do not yet share code, because it didn't seem to help the clarity very much.