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

Add std::io::input simple input function. #74178

Closed
wants to merge 69 commits into from
Closed

Add std::io::input simple input function. #74178

wants to merge 69 commits into from

Conversation

sHaDoW-54
Copy link

Condenses the normal input process into one function. This is good for new rust users who don't understand the normal process and for users who don't require special functionality.

Especially with new rust users looking to make small programs, input can be a tricky thing for them to grasp, especially when other languages do this much simpler.

EX:
Python : user_input = input("Enter: ")
Ruby: user_input = gets
C#: user_input = Console.ReadLine();

So this...

use std::io::{stdin,stdout,Write};
let mut s=String::new();
print!("Please enter some text: ");
let _=stdout().flush();
stdin().read_line(&mut s).expect("Did not enter a correct string");
if let Some('\n')=s.chars().next_back() {
    s.pop();
}
if let Some('\r')=s.chars().next_back() {
    s.pop();
}
println!("You typed: {}",s);

Would turn into this...

use std::io::input;
let user_input = input("Please enter some text: ");
println!("You typed: {}", user_input);

Condenses the normal input process into one function. This is good for new rust users who don't understand the normal process and for users who don't require special functionality.
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 9, 2020
@pickfire
Copy link
Contributor

pickfire commented Jul 9, 2020

I would suggest having two set of API, input() and inputln().

pub fn inputln(prompt: &str) -> String {}

pub fn input() -> String {}

I could think of a use like

let name = inputln("name: ");
let age = input();

I also don't know if it will be good to have -> T or generics here using Into<T>.

I wonder if these kind of pull request requires a FCP first.

Sidenote: @sHaDoW-54 can you please format the code with rustfmt and use ```rust?

@frewsxcv frewsxcv added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 9, 2020
- Properly formated.
- Added ```rust for docs.
- Split function into two : ```input()``` and ```inputln(&str)```.
- Deleted unnecessary comment for non-windows users.
- Removed ```print!()``` macro.
- Added warning not recommending this method for normal use.
- Accounted for all return options, ("\n"), ("\r"), ("\r\n"), ("\n\r")
Streamlined the ```inputln(&str)``` function.
sHaDoW-54 added 2 commits July 9, 2020 16:13
Changed ```inputln(&str)``` function name to better represent the function.
Fixed docs
@retep998
Copy link
Member

retep998 commented Jul 9, 2020

I think this would be a very useful addition to std as it really is a major hurdle for simple console prompts to be so complicated.

Attempting to fix, Error unused function.
sHaDoW-54 added 3 commits July 9, 2020 20:03
Set functions as unstable
Use input functions.
Flagged by tidy for trailing whitespace?
sHaDoW-54 and others added 4 commits July 17, 2020 14:08
Co-authored-by: Ivan Tham <pickfire@riseup.net>
Co-authored-by: Ivan Tham <pickfire@riseup.net>
Co-authored-by: Ivan Tham <pickfire@riseup.net>
@retep998
Copy link
Member

Given these functions would start out as unstable, and it really is just one or two functions, at most we'd want a libs team FCP. A full RFC is overkill. If someone wants to come up with a more comprehensive solution in the meantime, by all means, but that shouldn't block the unstable addition of these simple functions. I regularly write simple Rust scripts where I'd love to quickly read some input but it's always such a hassle to pull out the stdio machinery to do it so these functions are perfect for me.

@the8472
Copy link
Member

the8472 commented Jul 17, 2020

I agree we could just have one function, though I don't think they need to know some/none. The function could have the signature of prompt and users who want input without writing anything can just pass an empty string.

Seconded. There's essentially no performance benefit for having a no-output version. An empty string is a constant and the performance is limited by the human anyway.

* the name feels off (I like `read_input` better)

How about read_line or readline (like gnu readline)?

Merged both functions
remove excess code
removed unused feature
sHaDoW-54 and others added 4 commits July 20, 2020 20:11
remove double locking

Co-authored-by: Joshua Nelson <joshua@yottadb.com>
fix double lock, again
@pickfire
Copy link
Contributor

pickfire commented Jul 21, 2020

Seconded. There's essentially no performance benefit for having a no-output version. An empty string is a constant and the performance is limited by the human anyway.

Oh, why merge prompt and input? I think it should be separated. Why print an empty string when you can not print at all? It looks like an extra useless step is being called when user have input(""). If this is the case, I rather it be a macro instead like input!() or input!("prompt") or even input!("prompt {}", name) (from print!).

Co-authored-by: Ivan Tham <pickfire@riseup.net>
@sHaDoW-54
Copy link
Author

@pickfire Do you mean something like the difference between unwrap() and expect("message") where one gives and output and the other doesn't? If so, i think this is a good idea since it is already established with these two functions, so why not input()?

@pickfire
Copy link
Contributor

pickfire commented Jul 22, 2020

@sHaDoW-54 I meant something like println! where you could use it to print just an empty line println!() or print some stuff println!("hello {}", 123). Note that for our case, we currently does not have input() after your last change, it requires people to do a syscall to print nothing which is extra while needing to use it with input(""). In my personal opinion, I prefer the previous behavior where we have input() and prompt("text"), or we could follow how the macros for printing like input!() and input!("text") such as the current print!, println!, eprint!, eprintln!, write!, writeln!, assert!, assert_eq!, assert_ne! and format! (this have no output) macros.

Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some suggestions to move the code examples in line with the standard style, that is, import types, but don't import functions directly.

One thing to consider about adding functionality like this is its impact on docs. Consider https://doc.rust-lang.org/stable/book/ch02-00-guessing-game-tutorial.html, which is where it would be used in the book; this function would decently simplify the very beginning.

@bors
Copy link
Collaborator

bors commented Jul 28, 2020

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

@sHaDoW-54
Ping from triage: can you please address the merge conflicts?

@Dylan-DPC-zz
Copy link

reworked in #75435

@pickfire
Copy link
Contributor

I wrote the design in https://hackmd.io/@G8ZK5BSuQPOxvEQWVZSxng/By2UUacFD/edit, hopefully it covers the design space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.