Add support for skip method / More population triggers
#144
Conversation
| # filepath contents | ||
| if buffer.io.size == 0 | ||
| write(buffer.io, read(buffer.path)) | ||
| seekstart(buffer.io) |
There was a problem hiding this comment.
I could definitely clean up Base.read(buffer::FileBuffer, ::Type{UInt8}) but we should probably leave the other _read(...); seekstart(...) calls alone as they would rewind the stream on each call after the FileBuffer was populated
There was a problem hiding this comment.
Yeah, I think when I initially wrote this the automatic rewinding seemed helpful for my use case. In retrospect, it seems like it has caused more problems with the IO API. I'm a little surprised this didn't break any of the existing tests though.
Codecov Report
@@ Coverage Diff @@
## master #144 +/- ##
==========================================
+ Coverage 91.70% 91.77% +0.07%
==========================================
Files 12 12
Lines 1182 1180 -2
==========================================
- Hits 1084 1083 -1
+ Misses 98 97 -1
Continue to review full report at Codecov.
|
rofinn
left a comment
rofinn
left a comment
There was a problem hiding this comment.
Seems fine to me. I think the existing tests covered my use, so if those aren't changing, then it should be fine. Please bump the patch release when you get a chance.
| # filepath contents | ||
| if buffer.io.size == 0 | ||
| write(buffer.io, read(buffer.path)) | ||
| seekstart(buffer.io) |
There was a problem hiding this comment.
Yeah, I think when I initially wrote this the automatic rewinding seemed helpful for my use case. In retrospect, it seems like it has caused more problems with the IO API. I'm a little surprised this didn't break any of the existing tests though.
2 participants
Fixes #143. Also, made it so more methods trigger the
FileBufferto become populated to avoid erroneous behaviour.I ended up using the short-form function syntax with two statements which I normally avoid but here it seemed cleaner. More than willing to revise that.