Rust: Tracking issue for std::fs::read, read_string, and write

Created on 8 Dec 2017  Â·  32Comments  Â·  Source: rust-lang/rust

Implemented in https://github.com/rust-lang/rust/pull/45837

New APIs in std::fs:

pub fn read<P: AsRef<Path>>(path: P) -> io::Result<Vec<u8>> { … }
pub fn read_string<P: AsRef<Path>>(path: P) -> io::Result<String> { … }
pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> io::Result<()> { ... }

(read_string is based on read_to_string and so returns an error on non-UTF-8 content.)

Before:

use std::fs::File;
use std::io::Read;

let mut bytes = Vec::new();
File::open(filename)?.read_to_end(&mut bytes)?;
do_something_with(bytes)

After:

use std::fs;

do_something_with(fs::read(filename)?)
B-unstable C-tracking-issue T-libs final-comment-period

Most helpful comment

How about read_as_string? It definitely doesn't imply a String is given to read into. According to the naming conventions, as_ is meant to be a costless conversion of borrowed -> borrowed, but I think in the context of reading from the file system the cost of verifying UTF8 can be considered to be quite minimal, and so as_ is a reasonable choice?

All 32 comments

Implemented, so shouldn't this be closed?

@JordiPolo this issue is tracking their stabilization, not their implementation.

By the way, is there a delay before we should start FCP for stabilization? This is niche enough that I don’t expect to get more feedback just by waiting. I’m not planning to use these functions myself until they’re stable.

Is also adding a lazy iterator on lines a good idea?

@leonardo-m https://doc.rust-lang.org/std/io/trait.BufRead.html#method.lines already exists. Or do you mean adding a convenience function that calls it from a filename? I personally feel less need for that function, but feel free to open a new PR for it.

I meant a convenience function. It's often useful for small script-like programs, tests, benchmarks, online code competitions and games, etc.

@aturon Let’s stabilize?

@rfcbot fcp merge

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged teams:

  • [x] @BurntSushi
  • [x] @Kimundi
  • [x] @KodrAus
  • [x] @alexcrichton
  • [x] @aturon
  • [x] @dtolnay
  • [x] @sfackler
  • [ ] @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

This landed less than a month ago, right? Is that a bit rapid for stabilization?

Why read_string instead of read_to_string? Seems incorrect to me.

I would expect something named read_to_string to take a &mut String argument, like in io::Read.

@alexcrichton assuming we don't land a PR for this cycle, it's another 3 months away from hitting stable.

@aturon that's true yeah, if we land on the next nightly (which branches Feb 15) I think that's an ok length of time.

@dtolnay That's interesting. I don't think that is connected to to because read also takes an argument to read into.

To me, read_string seems either ungrammatical or wrong (you're not reading a String, you're reading something and constructing a String of it). I'd prefer some preposition connecting read and string, but I'm not picky - read_to_string, read_as_string, read_into_string, etc.

Previous discussion on the naming of read_string (which was originally read_utf8): https://github.com/rust-lang/rust/pull/45837#issuecomment-346162867

:bell: This is now entering its final comment period, as per the review above. :bell:

It seems like there was a lot of discussion about read_string vs read_utf8, but not much about the suitability of read_string vs. read_into_string. I'd agree with @withoutboats that read_string isn't right: it sounds like I'm reading something _from_ a string, not creating a new one.

read_to_string isn't exactly right either though, it is really reading into a new string not to an existing one.

Is there any chance of changing the function to read_into_string before the FCP is over?

@rfcbot concern read_string

Registering that I meant to be blocking on the name read_string.

I'd really like this to be stabilized. I think read_to_string is the best name, but I'd settle for read_into_string. I don't think read_to_string implies that its reading into an existing string - if it did, wouldn't read be called read_to_slice and read_to_end be called read_to_vec?

I don’t have a strong opinion for the name of this function, but the Read trait already has a read_to_string method that takes &mut String.

if it did, wouldn't read be called read_to_slice and read_to_end be called read_to_vec?

I think if they were differentiating themselves from some other "default" read mode, then those would be reasonable names. Since we default to bytes, I'd say they don't need to differentiate?

I'm not sure I think read_into_string is better anymore though. I mean a distinction from Read::read_to_string would be good given that one takes a string to write to, but into and to have very similar connotations, and neither is really associated with "reading to an existing variable" more than the other.

:thumbsup: for either read_into_string or read_to_string

The final comment period is now complete.

I would be happy with read_to_string, or maybe read_into_string. It doesn't seem like a perfect name to address all concerns is likely to come up, so it would be great to settle on a maybe-imperfect one.

The one issue with read_to_string is that our other read_to_string appends to an existing String, while this one returns a String. @dtolnay (or others), do you feel this difference is important enough that it needs a different name?

How about read_as_string? It definitely doesn't imply a String is given to read into. According to the naming conventions, as_ is meant to be a costless conversion of borrowed -> borrowed, but I think in the context of reading from the file system the cost of verifying UTF8 can be considered to be quite minimal, and so as_ is a reasonable choice?

In fs::read_as_string, the as_string part is definitely a "minimal-cost conversion" if compared to fs::read.

It seems like the naming for the string-returning function isn't super resolved, so how about stabilizing just read and write in the meantime?

@sfackler sounds good to me :)

read_as_string seems to me like it satisfies all the concerns voiced, could we just take a quick tally on that? When's the next deadline for getting into stabilisation?

@angusholder There is no deadline for stabilizations. The only thing time-based things are release trains, https://forge.rust-lang.org/#release_info shows some dates. When a version reaches Stable, the next one graduates from Nightly to Beta at the same time and then reaches Stable 6 weeks later.

At this point I have no opinion about naming for this function. I’ll let @withoutboats pick one and resolve their concern with rfcbot.

looking forward to this being stabilized :) read_as_string seems good

The libs team discussed this today and the consensus is to stabilize with the read_string method renamed to read_to_string. A stabilization PR is welcome.

49422 stabilized read and write.

49522 submitted to rename and stabilize read_to_string.

Was this page helpful?
0 / 5 - 0 ratings