-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Add std::io::inputln() #3196
base: master
Are you sure you want to change the base?
Conversation
d1b96ae
to
211cae9
Compare
211cae9
to
1309352
Compare
I think that this might be a better idea. In my RFC I proposed a more advanced approach which ultimately evolved to have formatting, however that is a lot more complex to implement and might be better to leave to crates. I think the standard library would benefit more from a simpler implementation like the one suggested here. |
Since it's a function it can be added to std::prelude.
I'm personally all for this method, but I think that the method should be called |
Since there is already a |
|
I think a specific example might help: https://doc.rust-lang.org/std/fs/fn.read_to_string.html One |
Note that print!() and println!() are "specialised" variant of write!() and writeln!(), where all of them are based on io::Write but the first two specifically writes to stdout. So analogously, we should have something that provides a more convenient interface to io::Read first, then to provide a more specific one such as inputln(). Introducing inputln() without something like read() or readln() just feels weird logically. |
Thanks Clar, that is a really good observation! I however see a different consistency axis: for any function named
So having a |
I am not sure this is a big deal, but I tried using your function in a test project and noticed that it doesn't work with parsing to other values. For example: let age: i32 = io::inputln()?.parse().expect("Invalid age!"); This will always fail here, because when it reads it uses the pub fn read_line() -> Result<String, std::io::Error> {
let mut line = String::new();
std::io::stdin().read_line(&mut line)?;
Ok(line.trim_end_matches('\n').to_string())
} I might be wrong, but I see no reason to include the newline character at the end. EDIT: Why does |
Good observation @undersquire! I however don't think that this is necessary. You can just do: let age: i32 = io::inputln()?.trim().parse().expect("Invalid age!"); which also has the benefit that it does not fail when the user accidentally enters other whitespace before or after their input.
Please stop talking about breaking backwards compatability 😅 |
Given that the entire purpose of This is the same reason that |
That's a very good point! I updated the RFC to include the same newline trimming as |
What does this function do when it hits EOF or reads a non-newline-terminated line? Python's fn main() -> std::io::Result<()> {
loop {
dbg!(inputln()?);
}
}
|
I would expect
|
The proposed |
@not-my-profile Is the function being renamed to |
Thanks @m-ou-se, you're of course right, when the function performs newline trimming it also has to convert zero reads to UnexpectedEof errors, or otherwise users have no chance of detecting EOF. I added EOF handling to the RFC (along with rationales for both newline trimming and EOF handling).
If the function would just allocate a string, call But now that the function is additionally trimming newlines and handling EOF, I am thinking that a distinct function name might be a better choice, so that API users aren't mislead into thinking that the function returns the string/error unchanged, as is the case with the Do we want two functions in the same standard library module with the same name but subtly different behavior? Is there precedent for that? Is that desirable? |
Rather than returning an error on EOF, I personally would expect this to be an expected possibility to handle: |
The RFC notes that contrary to println! and friends, that are macros, inputln is a function and that printing a prompt with print doesn't flush. Couldn't both be addressed by instead of making inputln a function it would be a macro.
This would add to the namespace but the RFC seems to propose that the function is available in prelude in any case. |
I've started pointing my students at |
I agree with you, in my case, my community (Rust Lang Es) do divulgation about the language, and it's one of the most frequent questions in new developers. |
Great implementation using macros @BartMassey Pieces I like of this implementation is leveraging generics, implementing everything based on FromStr trait to handle nearly any common cast, generic error handling (yours returns a string panic), and abstracting the allocation of the String itself (unless the cast is to a String of course). I think macros are probably a less opinionated approach and easy to understand. Still requires a match statement to cast, but you have to do that in every dynamic language prompt ( x = int(input("input:")) ) An outsider with a lot of experience building standard library functions reviewing different methods presented here would be 👩🍳 💋 I think a main argument point should be around the guessing game example. "Rust is powerful, yet high-level" is a branding the community tries to run with... but the guessing game does not do a very good job of getting the point across. Failing this negatively affects adoption; and thus, reduces awareness. Rust has done great despite this, but I know many JavaScript and python devs who look at my rust code and are like, "why would you do this to yourself." They don't necessarily understand the full potential or picture behind the reasons to use Rust. Other points on why this should be accepted:
|
Problems with
use std::str::FromStr;
fn parsery<T: FromStr>(s: &str) -> Result<T, <T as FromStr>::Err> {
T::from_str(s)
}
fn main() {
let s = parsery("4.5").unwrap();
let t = s + 7.0f32;
println!("{}", t);
}
These are the kinds of issues that made me have my All of that said, I think these kinds of differences are a part of what make people reluctant to stick anything in |
BTW, any friendly line reader should maybe use GNU readline or similar for line editing. Or maybe not. |
Excellent first point, but a major goal it seems in rust is make advanced things simple. An advanced programmer could find reasons to use the Error result and do something with it (like maybe set a new variable or provide a match. I think impl Display and Debug were my ways of providing user feedback. It is true beginners would not know how to use errors returned, but that is not gonna convince managers of rust to implement alone. Moving on, wow that's the most wtf thing I've seen in rust, truly undefined behavior (I started seriously learning post-stable). I wonder what could cause this? Do they use the same address of bytes? Would setting s to f32 at instantiating assist the compiler in allocating smarter? I'll for sure play around. Finally, like most open source things if we find a consensus ourselves and can get a couple more people on board, people will eventually fall inline. The people above talking about scanner is a stretch too far in use-cases and further complication. If anything it's a later iteration. This is not just my opinion, but the consensus here. @BartMassey I would be willing to split work and collaborate with you, so far as committing to maintenance (should be minimal). A couple people willing to lead it also is gonna make this more appetizing to people who would give the green light! 😃 |
Linereader should be the responsibility of the IO libraries we (as well as other depending libs/internals) use. I haven't seen any compatability issues ever for IO. |
In a demo/tutorial/exploration program, sure. In a real program, you may want to "handle" it by printing a more user friendly error message before aborting. |
So let's start with a finished library for integration to base off of, what I'm reading is:
I'm gonna set a reminder for myself to come back in 2 weeks where I'd like to get some more consensus points before sending this rfc more fleshed out as a library. I will go ahead and put in any work required to get things working like we agree on this board. Please pr to bazylhorsey/io-cast for direct changes or recommend them here. If we diverge in thoughts I'll make a couple different branches with different ways to solve the problem which the rust leads can take from there. I have a good feeling organizing things this way with a few people who keep this conversation going will get this to the finish line. Cheers! 😃 |
Thanks much for summarizing some of the issues!
Issues to be added:
|
Why should we return a String if we already have allocated what we need. I'd argue str is more powerful and better performing. Moving the data from the heap to the stack seems sane. If the user wants to mutate again they can use the as_string function right? Most use cases I image they're only ever going to be reallocating if they want the string which they then either scrub or change values. My understanding is str is an array, and String is a vector. In our case once you grab the line you know they exact amount of bytes you need so the str should be returned and owned by the variable that called this function. For the panic stuff, maybe someone else can weigh in here. In my implementation, I'm still printing the error the same way you are, but at the same time presenting the option of using the generic error so it can be caught in a match. IMO there's no downside to this as you still get a message in debug which is for beginner purposes fulfills your points while being a more full solution. |
Allocated where? I don't understand the proposed implementation: short of passing a buffer into the macro I see no place to put the read data except on the heap. What lifetime do you propose that the returned
I strongly prefer not having to Genuinely sorry to be so argumentative. I'll bow out now and let you and others discuss. |
No no, this is great stuff. Not having to unwrap is something I didn't think of, and while valid, look at other things in std where unwrap is pretty commonplace. In general I think panic should be kept for things outside rusts implementation when you know the behavior is isolated. When I first was learning rust I was like, why do I have to unwrap everything, now it makes sense and I see the power of handling errors in a multitude of different ways. If someone wanted to then make a library that abstracts it with a panic that sounds fine. As for std we should not make assumptions around beginner-based use. It's still multitudes simpler than doing io yourself with its current implementation. I'm still not a rust god, but in my head I have it compartmentalized as: if I'm mutating a string i use String, otherwise I use str. In our case obviously at some point we need to construct an unknown length of bytes into a vector so String is obviously a piece, but for the return we should pass it to str because it's more low-level, implements FromStr is straightforward, and the user can always recast it to a String of the value changes. Strings are heavier more complex objects the programmer should know when they need to allocate a mutable space in the heap. My 2 cents. |
not if you have any kind of redirection or piping going on. For example if stdin is a pipe, and the process on the other end terminates, you will get an I/O error (if your process isn't killed by SIGPIPE). And if you want to avoid |
actually, afaict you just get EOF when reading from a pipe where the write end was closed (after reading any cached data). you get SIGPIPE and/or EPIPE when writing where the read end was closed. |
@programmerjake considering this do you think a result should be returned from the function? |
yes, because EOF can be considered an error and is quite common. after all, you want to be able to distinguish between reading an infinite sequence of empty lines and reaching the end of a file. |
@programmerjake any thoughts on how what type we should return or parse, and where we should draw the line? |
pub fn inputln() -> io::Result<String> {...} Python's usage: let foo: i32 = inputln()?.parse(); alternatively, have it return usage: loop {
let Some(line) = inputln()? else {
break;
};
let v: i32 = line.parse()?;
println!("{v} * 2 == {}", v * 2);
} |
@programmerjake Also you think this should also be a macro? Do you have a preference or intuition on if Option String or expected String in your example of if it's more likely to be considered acceptable to rusts team? Just collecting your thoughts while I have your attention :) |
Also everyone don't forget to cast your vote from this old RFC https://strawpoll.com/zxds5jye6 This will be a good community study for a considered proposal |
What would own the underlying data? Or perhaps written another way, what would be the lifetime of the returned The possibilities I see for that are:
|
I've linked this before but I personally think my ACP suggestion is a better option for easy input. It can entirely avoid any ownership problems because you're just using the buffer from stdin and parsing directly from there. You probably want to parse quite often into a different type than |
Wow now that's some real code hombre. |
It's an ACP, not an RFC. I was advised to use the ACP process instead of the RFC process because it was just a single function to It is currently still waiting for the libs team to look at it and give a respond and it's unfortunately taking a long time. Or at least that's what I think but I can't seem to find any documentation on the ACP process any more and now I'm curious what happened to it. |
After a lot of deliberation, I aimed at simplification and producing minimal code changes while maximizing velocity for newcomers to I/O related problems. This is made somewhat with the general rust dev in mind, but not to the point it becomes an advance level feature, or fails in solving the original problem: making the Rust intro smoother. What this board learned:
Things this does:
Things this does NOT DO:
use std::io::{self, Write};
use std::str::FromStr;
/// Custom error type for input parsing.
pub enum InputError {
IoError(io::Error),
ParseError(String),
}
/// Function to get input and parse it into the desired type.
pub fn parse_input<T: FromStr>(prompt: &str) -> Result<T, InputError>
where
<T as FromStr>::Err: ToString,
{
print!("{}", prompt);
io::stdout().flush().map_err(InputError::IoError)?;
let mut input = String::new();
io::stdin().read_line(&mut input).map_err(InputError::IoError)?;
input.trim().parse::<T>().map_err(|e| InputError::ParseError(e.to_string()))
}
/// Macro to simplify calling the parse_input function.
#[macro_export]
macro_rules! parse_input {
($prompt:expr) => {
$crate::parse_input::<_>($prompt)
};
}
#[cfg(test)]
mod tests {
// Example test for successful parsing
#[test]
fn test_parse_input_success() {
let result = "42".parse::<f64>();
assert_eq!(result, Ok(42.));
}
// Example test for failed parsing
#[test]
fn test_parse_input_failure() {
let result = "abc".parse::<i32>();
assert!(result.is_err());
}
} |
The enum InputError<E> {
Io(io::Error),
Parse(E),
}
pub fn parse_input<T: FromStr>(prompt: &str) -> Result<T, InputError<T::Err>> { ... } |
To add on to @RustyYato 's comment, when I first saw the definition of InputError, I thought that the content of the Parse variant was the value of the string that was read but failed to parse, not the stringification of the parse error. And actually, it might be worth including the string read in that variant. |
Maybe it's better to separate the topics. I said that because in my opinion, we are talking about different cases. However, this doesn't solve the first problem that Mara said.
Besides we must start with just one easy case (Maybe) to replace the current let mut input = String::new();
stdin().read_line(&mut input) |
I think the best way to drive the addition of this function forward is to have a dedicated RFC for it.
Rendered
Previous discussions:
std::io::input
simple input function. rust#75435