rofinn / FilePathsBase.jl

Filesystem path types in julia
Other
82 stars 17 forks source link

`eof(::FileBuffer)` incorrectly returns `true` before filling the buffer #134

Closed iamed2 closed 3 years ago

iamed2 commented 3 years ago

According to the eof docs:

If the stream is not yet exhausted, this function will block to wait for more data if necessary, and then return false.

The current behaviour of always returning true before the first read means that the write(A::IO, B::IO) method (for B<:FileBuffer) and similar patterns are currently broken.

This is currently causing a problem for CSV.jl with S3Path and is also causing https://github.com/rofinn/FilePathsBase.jl/issues/126.

nickrobinson251 commented 3 years ago

Do you know how you'd like to see this fixed?

Do we want to read the file into the buffer before that check, e.g.

-Base.eof(buffer::FileBuffer) = eof(buffer.io)
+function Base.eof(buffer::FileBuffer)
+    if position(buffer) == 0
+        _read(buffer)
+        seekstart(buffer)
+    end
+    return eof(buffer.io)
+end

or do we want to avoid reading the file into the buffer and check the file directly like

-Base.eof(buffer::FileBuffer) = eof(buffer.io)
+function Base.eof(buffer::FileBuffer)
+    if position(buffer) == 0
+        return size(buffer.path) == 0
+    end
+    return eof(buffer.io)
+end

or is there another approach you had in mind?

iamed2 commented 3 years ago

The former; the docs say following what I quoted above:

Therefore it is always safe to read one byte after seeing eof return false.