-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add terminal IO functions #148
Conversation
This new version will be 1.3.1. Can you add the |
Done |
classy-prelude/src/ClassyPrelude.hs
Outdated
-- Uses system locale settings | ||
-- | ||
-- @since 1.3.1 | ||
interact :: MonadIO m => (Text -> Text) -> m () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a strict Text
the correct data type to use here? I'm generally opposed to lazy I/O, but a strict I/O version of interact
is pretty confusing. At the very least, it should be documented. (Same basic idea applies to getContents
, though due to the type signature of interact
the downsides of lazy I/O are much less severe.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched it to lazy IO. It does have the downside here that LText
s have to be used instead of Text
s, while in the Prelude, it's String
s either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it's a downside.
|
||
-- | Write a character to stdout | ||
-- | ||
-- Uses system locale settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing a @since
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
||
-- | Read a line from stdin | ||
-- | ||
-- Uses system locale settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing @since
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Thanks! Just waiting for CI to pass then I'll merge. |
Thanks! |
I haven't added
@since
comments, since this would depend on what release this goes into.