Document and export Base.peek

[ararslan]

It's generally useful but is currently both undocumented and unexported.

Closes #28797.

[StefanKarpinski]

Does it work on all kinds of I/O streams now?

[ararslan]

Maybe? I didn't change any methods, but there is a method already there for peek(::IO).

[StefanKarpinski]

@Keno, you recently added some methods to peek, any sense of how complete it is at this point? Another issue is making it more generic—i.e. combine peek and peekchar?

[bicycle1885]

There is a generic method:

julia/base/iostream.jl

Lines 513 to 519 in e4d933d

	function peek(s::IO)
	mark(s)
	try read(s, UInt8)
	finally
	reset(s)
	end
	end

.

But it depends on mark and reset, and the default implementations for these functions are not enough generic to support all kinds of IO streams.

[ViralBShah]

I am guessing we do not want to export a new API here. Please suggest to merge or close.

Since this changes the export, I'll remove the doc label (since it's not strictly a doc PR).

[ararslan]

I think peek is useful enough to be exported but this PR is quite stale and I don't know when I'd have time to revisit it.

[StefanKarpinski]

I think the API should be that peek(io) returns a single byte and peek(io, Char) returns a character. Since Base.peek already implements peek(io) that can just be exported. The methods for Base.peekchar can be turned into Base.peek(io, Char) methods too.

[ararslan]

Found a spare couple of minutes for a rebase. peekchar seems not to be a thing anymore though, but I added peek(io, Char) for consistency with read.

[StefanKarpinski]

I'm having trouble seeing where peek(io, Char) is defined...

[ararslan]

I changed it to peek(io, T) where T defaults to UInt8. The method in base/io.jl for peek(io, T) calls read(io, T) internally instead of unconditionally calling read(io, UInt8) as it does on master. That allows peek(io, Char) to work while also permitting peeking other types supported by read.

[ararslan]

I noticed that peek(::IOStream) returns an Int32. That is inconsistent with everything else, but changing it would be potentially breaking...

[StefanKarpinski]

Ah, that seems bad. Does it actually read/return 32-bits of data or just a byte cast to Int32?

[ararslan]

It's doing a ccall to return a Cint but the underlying C function seems to only be reading a byte; it actually does an explicit cast to unsigned char then implicit cast to int. Indeed:

julia> open(io -> print(io, "こんにちは"), "thing", "w")

julia> open(Base.peek, "thing", "r")
227

[StefanKarpinski]

So in C it's traditional to return an int value that's not equal to any possible byte value when the stream is EOF but in Julia we consider that to be an exception instead and expect the programmer to check for EOF using eof before doing a read and it seems like peek should work the same. So it seems like this should check if the returned value is the EOF value and raise an error if so and otherwise return the value converted to a byte. @JeffBezanson, does that seem right to you?

[ararslan]

Something like the definition of read(::IOStream) but with ios_peekc instead of ios_getc, then?

[StefanKarpinski]

Exactly.

[StefanKarpinski]

This is a really nice bit of API rationalization and making peek coherent enough to export. Thank you! Might be good for @JeffBezanson to take a look since he's got a good eye for details I may have missed.

[JeffBezanson]

Seems to be identical to the fallback method. Not a problem though.

[ararslan]

Yeah, I noticed that. Figured I'd punt on dealing with that though, just in case there was something I was missing.

[JeffBezanson]

I'm a little concerned that the default peek(io, Char) will be slow but I guess we can fix that later.

[JeffBezanson]

In io.jl around line 354 there's a for loop that defines another peek method that needs to accept a type argument.

[StefanKarpinski]

I'm a little concerned that the default peek(io, Char) will be slow but I guess we can fix that later.

I had the same thought: the efficient way to do this is to add support for peeking four bytes at a time and then decoding a UTF-8 character from that.

[ararslan]

Thanks everyone! Always fun to revisit a nearly 2 year old PR and get it merged. :)