-
Notifications
You must be signed in to change notification settings - Fork 1.7k
add back bufferless read_to_string/read_to_end methods #970
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
Closed
Closed
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| - Feature Name: read_to_string_without_buffer | ||
| - Start Date: 2015-03-13 | ||
| - RFC PR: https://github.com/rust-lang/rfcs/pull/970 | ||
| - Rust Issue: | ||
|
|
||
| # Summary | ||
|
|
||
| Add back `read_to_string` and `read_to_end` methods to the `Read` trait that | ||
| don't take a buffer. | ||
|
|
||
| # Motivation | ||
|
|
||
| While the `fn read_to_end(&mut self, buf: &mut Vec<u8>) -> Result<()>` and | ||
| `fn read_to_string(&mut self, buf: &mut String) -> Result<()>` APIs are more | ||
| efficient removing the APIs that don't require passing a buffer entirely comes | ||
| at with convenience loss in some situations. In particular if one want's to | ||
| implement a chaining API and doesn't care about efficiency. | ||
|
|
||
| Today we either have to write this | ||
|
|
||
| ```rust | ||
| fn get_last_commit () -> String { | ||
|
|
||
| let output = Command::new("git") | ||
| .arg("rev-parse") | ||
| .arg("HEAD") | ||
| .output() | ||
| .ok().expect("error invoking git rev-parse"); | ||
|
|
||
| let encoded = String::from_utf8(output.stdout).ok().expect("error parsing output of git rev-parse"); | ||
|
|
||
| encoded | ||
| } | ||
| ``` | ||
|
|
||
| Or this: | ||
|
|
||
|
|
||
| ```rust | ||
| fn get_last_commit () -> String { | ||
|
|
||
| Command::new("git") | ||
| .arg("rev-parse") | ||
| .arg("HEAD") | ||
| .output() | ||
| .map(|output| { | ||
| String::from_utf8(output.stdout).ok().expect("error reading into string") | ||
| }) | ||
| .ok().expect("error invoking git rev-parse") | ||
| } | ||
| ``` | ||
|
|
||
| But we'd like to be able to just write | ||
|
|
||
| ```rust | ||
| fn get_last_commit () -> String { | ||
|
|
||
| Command::new("git") | ||
| .arg("rev-parse") | ||
| .arg("HEAD") | ||
| .spawn() | ||
| .ok().expect("error spawning process") | ||
| .stdout.read_to_string() | ||
| .ok().expect("error reading output") | ||
| } | ||
| ``` | ||
|
|
||
| This was possible before but since there is not such `read_to_string` API | ||
| anymore, it's currently impossible. | ||
|
|
||
|
|
||
| # Detailed design | ||
|
|
||
| Add back methods with following signature | ||
|
|
||
| `fn read_to_end(&mut self) -> Result<Vec<u8>>` | ||
|
|
||
| `fn read_to_string(&mut self) -> Result<String>` | ||
|
|
||
| # Drawbacks | ||
|
|
||
| Two more methods to maintain | ||
|
|
||
| # Alternatives | ||
|
|
||
| Don't do it and force users to use things like `map` for chaining | ||
|
|
||
| # Unresolved questions | ||
|
|
||
| None. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Don't these conflict with the existing
read_to_endandread_to_stringmethods?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.
Good point. What's the state of method overloading in Rust? I've been basically away from Rust for the last couple of months which means I have to start over. If there's no such thing as method overloading I'd need to give them other names.
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.
You can provide multiple methods of the same name from differing traits, and use UFCS (ie
ReadExt::read_to_end(&mut rdr, Vec::new())) to differentiate. But that would not improve the convenience you mentioned in this RFC. You'd need a new name.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.
read_into_vecandread_into_string, possiblyThere 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.
@reem I like those, updated the PR