Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modify Base.peek to return a Char and export it #16025

Closed
StefanKarpinski opened this issue Apr 23, 2016 · 7 comments
Closed

modify Base.peek to return a Char and export it #16025

StefanKarpinski opened this issue Apr 23, 2016 · 7 comments
Labels
good first issue Indicates a good issue for first-time contributors to Julia Hacktoberfest Good for Hacktoberfest participants help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Apr 23, 2016

The peek function is very useful and an efficient alternative to marking and resetting a stream when all you need to do is get a single character. The current peek returns a byte, however, which leads to errors when people assume that they can compare that byte to a character and check for a character (only works for UTF-8 streams and ASCII characters). This was a significant annoyance in implementing #16024. This is technically not breaking since Base.peek isn't exported. Most code that uses it will actually continue to work.

@StefanKarpinski StefanKarpinski added help wanted Indicates that a maintainer wants help on an issue or pull request good first issue Indicates a good issue for first-time contributors to Julia labels Apr 23, 2016
@quinnj
Copy link
Member

quinnj commented Apr 23, 2016

we should make this match the read API, so:

read(io, UInt8)
read(io, Char)

peek(io, UInt8)
peek(io, Char)

@vtjnash
Copy link
Member

vtjnash commented Apr 25, 2016

duplicate of #2638 [decision: no]

@StefanKarpinski
Copy link
Member Author

The mark/reset API is good but it's very high overhead for just looking at a single character, which requires only a fixed amount of lookahead. The fact that we still use peek in a lot of places in Base indicates that peek is necessary and should work correctly, rather than continue to exist and be broken and non-public.

@StefanKarpinski
Copy link
Member Author

This could probably be closed just by renaming peekchar to peek and renaming peek to peekbyte. Since both are unexported, we could potentially just do the rename.

@StefanKarpinski
Copy link
Member Author

Or we could just do what @quinnj recommends and make peek match read.

@StefanKarpinski
Copy link
Member Author

Actually, even better: we can support generic peek(io, T) implemented with mark, read and reset but with more efficient methods when T is UInt8 or Char.

bicycle1885 added a commit to bicycle1885/julia that referenced this issue May 3, 2016
bicycle1885 added a commit to bicycle1885/julia that referenced this issue May 3, 2016
bicycle1885 added a commit to bicycle1885/julia that referenced this issue May 3, 2016
@kshyatt kshyatt added the Hacktoberfest Good for Hacktoberfest participants label Oct 5, 2016
@KristofferC
Copy link
Member

#28811

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Indicates a good issue for first-time contributors to Julia Hacktoberfest Good for Hacktoberfest participants help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants