Make mark consistent with skip and IOBuffer (#201)

Author: nhz2

docs/src/reference.md

@@ -19,6 +19,7 @@ TranscodingStreams.unsafe_read
 TranscodingStreams.unread
 TranscodingStreams.unsafe_unread
 Base.position(stream::TranscodingStream)
+Base.skip
 ```
 
 Statistics

ext/TestExt.jl

@@ -88,11 +88,15 @@ function TranscodingStreams.test_chunked_read(Encoder, Decoder)
         ok = true
         for chunk in chunks
             stream = TranscodingStream(Decoder(), buffer, stop_on_end=true)
-            ok &= hash(read(stream)) == hash(chunk)
+            ok &= read(stream) == chunk
             ok &= eof(stream)
             ok &= isreadable(stream)
             close(stream)
         end
+        # read without stop_on_end should read the full data.
+        stream = TranscodingStream(Decoder(), IOBuffer(data))
+        ok &= read(stream) == reduce(vcat, chunks)
+        close(stream)
         Test.@test ok
     end
     finalize(encoder)

src/stream.jl

@@ -229,50 +229,26 @@ end
     end
 end
 
-function Base.ismarked(stream::TranscodingStream)
+function Base.ismarked(stream::TranscodingStream)::Bool
     checkmode(stream)
-    return ismarked(stream.buffer1)
+    isopen(stream) && ismarked(stream.buffer1)
 end
 
-function Base.mark(stream::TranscodingStream)
-    checkmode(stream)
-    return mark!(stream.buffer1)
+function Base.mark(stream::TranscodingStream)::Int64
+    ready_to_read!(stream)
+    mark!(stream.buffer1)
+    position(stream)
 end
 
-function Base.unmark(stream::TranscodingStream)
+function Base.unmark(stream::TranscodingStream)::Bool
     checkmode(stream)
-    return unmark!(stream.buffer1)
+    isopen(stream) && unmark!(stream.buffer1)
 end
 
-function Base.reset(stream::TranscodingStream)
-    checkmode(stream)
-    return reset!(stream.buffer1)
-end
-
-function Base.skip(stream::TranscodingStream, offset::Integer)
-    checkmode(stream)
-    if offset < 0
-        throw(ArgumentError("negative offset"))
-    end
-    mode = stream.state.mode
-    buffer1 = stream.buffer1
-    skipped = 0
-    if mode === :read || mode === :stop
-        while !eof(stream) && buffersize(buffer1) < offset - skipped
-            n = buffersize(buffer1)
-            emptybuffer!(buffer1)
-            skipped += n
-        end
-        if eof(stream)
-            emptybuffer!(buffer1)
-        else
-            skipbuffer!(buffer1, offset - skipped)
-        end
-    else
-        # TODO: support skip in write mode
-        throw(ArgumentError("not in read mode"))
-    end
-    return stream
+function Base.reset(stream::T) where T<:TranscodingStream
+    Base.ismarked(stream) || throw(ArgumentError("$T not marked"))
+    reset!(stream.buffer1)
+    position(stream)
 end
 
 """
@@ -389,6 +365,32 @@ function Base.readuntil(stream::TranscodingStream, delim::UInt8; keep::Bool=fals
     return ret
 end
 
+"""
+    skip(stream::TranscodingStream, offset)
+
+Read bytes from `stream` until `offset` bytes have been read or `eof(stream)` is reached.
+
+Return `stream`, discarding read bytes.
+
+This function will not throw an `EOFError` if `eof(stream)` is reached before
+`offset` bytes can be read.
+"""
+function Base.skip(stream::TranscodingStream, offset::Integer)
+    if offset < 0
+        # TODO support negative offset if stream is marked
+        throw(ArgumentError("negative offset"))
+    end
+    ready_to_read!(stream)
+    buffer1 = stream.buffer1
+    skipped = 0
+    while skipped < offset && !eof(stream)
+        n = min(buffersize(buffer1), offset - skipped)
+        skipbuffer!(buffer1, n)
+        skipped += n
+    end
+    return stream
+end
+
 function Base.unsafe_read(stream::TranscodingStream, output::Ptr{UInt8}, nbytes::UInt)
     ready_to_read!(stream)
     buffer = stream.buffer1

test/codecdoubleframe.jl

@@ -287,7 +287,7 @@ DoubleFrameDecoderStream(stream::IO; kwargs...) = TranscodingStream(DoubleFrameD
     @testset "reading zero bytes from invalid stream" begin
         # This behavior is required to avoid breaking JLD2.jl
         # `s` must go into read mode, but not actually call `eof`
-        for readnone in (io -> read!(io, UInt8[]), io -> read(io, 0))
+        for readnone in (io -> read!(io, UInt8[]), io -> read(io, 0), io -> skip(io, 0))
             for invalid_data in (b"", b"asdf")
                 s = DoubleFrameDecoderStream(IOBuffer(invalid_data;read=true,write=true))
                 @test iswritable(s)

test/codecnoop.jl

@@ -59,17 +59,21 @@
     data = collect(0x00:0x0f)
     stream = TranscodingStream(Noop(), IOBuffer(data))
     @test !ismarked(stream)
-    mark(stream)
+    @test mark(stream) == 0
     @test ismarked(stream)
     @test [read(stream, UInt8) for _ in 1:3] == data[1:3]
-    reset(stream)
+    @test reset(stream) == 0
+    @test_throws ArgumentError reset(stream)
     @test !ismarked(stream)
     @test [read(stream, UInt8) for _ in 1:3] == data[1:3]
-    mark(stream)
+    @test mark(stream) == 3
     @test ismarked(stream)
-    unmark(stream)
+    @test unmark(stream)
     @test !ismarked(stream)
+    @test !unmark(stream)
+    @test mark(stream) == 3
     close(stream)
+    @test !ismarked(stream)
 
     stream = TranscodingStream(Noop(), IOBuffer(b"foobarbaz"))
     @test stream == seek(stream, 2)

test/codecquadruple.jl

@@ -110,6 +110,47 @@ end
         end
     end
 
+    @testset "skip" begin
+        @testset "position" begin
+            # make sure position is updated correctly by skip
+            iob = IOBuffer(repeat("x", 400))
+            source = IOBuffer(repeat("x", 100))
+            stream = TranscodingStream(QuadrupleCodec(), source, bufsize=16)
+            @test position(stream) == position(iob)
+            for len in 0:10:100
+                skip(stream, len)
+                skip(iob, len)
+                @test position(stream) == position(iob)
+            end
+            close(stream)
+        end
+        @testset "mark" begin
+            iob = IOBuffer(repeat("x", 400))
+            source = IOBuffer(repeat("x", 100))
+            stream = TranscodingStream(QuadrupleCodec(), source, bufsize=16)
+            @test position(stream) == position(iob)
+            @test mark(stream) == mark(iob)
+            for len in 0:10:100
+                skip(stream, len)
+                skip(iob, len)
+                @test position(stream) == position(iob)
+            end
+            @test Base.reset(stream) == Base.reset(iob)
+            for len in 0:10:100
+                skip(stream, len)
+                skip(iob, len)
+                @test position(stream) == position(iob)
+            end
+            @test mark(stream) == mark(iob)
+            @test Base.reset(stream) == Base.reset(iob)
+            @test mark(stream) == mark(iob)
+            close(stream)
+            close(iob)
+            @test !ismarked(stream)
+            @test !ismarked(iob)
+        end
+    end
+
     @testset "seekstart" begin
         data = Vector(b"abracadabra")
         source = IOBuffer(data)