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

Rename File::with_options to builder #76744

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,32 +358,26 @@ impl File {
OpenOptions::new().write(true).create(true).truncate(true).open(path.as_ref())
}

/// Returns a new OpenOptions object.
/// Returns a builder struct to configure how a file is opened.
Copy link
Contributor

@pickfire pickfire Sep 16, 2020

Choose a reason for hiding this comment

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

It is not a builder struct but OpenOptions struct.

Suggested change
/// Returns a builder struct to configure how a file is opened.
/// Configure how to open a file with `OpenOptions`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OpenOptions struct is a builder struct. I'm not sure we want to repeatedly mention
OpenOptions. I'll leave this for reviewer to decide.

///
/// This function returns a new OpenOptions object that you can use to
/// open or create a file with specific options if `open()` or `create()`
/// are not appropriate.
/// This function returns an empty [`OpenOptions`] object
/// to avoid the need of importing [`OpenOptions`].
///
/// It is equivalent to `OpenOptions::new()` but allows you to write more
/// readable code. Instead of `OpenOptions::new().read(true).open("foo.txt")`
/// you can write `File::with_options().read(true).open("foo.txt")`. This
/// also avoids the need to import `OpenOptions`.
///
/// See the [`OpenOptions::new`] function for more details.
/// See [`OpenOptions::new`] function for more details.
///
/// # Examples
///
/// ```no_run
/// #![feature(with_options)]
/// #![feature(file_builder)]
/// use std::fs::File;
///
/// fn main() -> std::io::Result<()> {
/// let mut f = File::with_options().read(true).open("foo.txt")?;
/// let _ = File::builder().read(true).open("foo.txt")?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "with_options", issue = "65439")]
pub fn with_options() -> OpenOptions {
#[unstable(feature = "file_builder", issue = "65439")]
pub fn builder() -> OpenOptions {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why not options?

Copy link
Contributor Author

@tesuji tesuji Sep 16, 2020

Choose a reason for hiding this comment

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

I guess I've never seen APIs like that. But yeah, it could be if reviewer prefers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the informal poll in #65439, builder was the least popular name and options was the most popular (just).

Copy link
Contributor Author

@tesuji tesuji Sep 16, 2020

Choose a reason for hiding this comment

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

I know that. However Rust design has never been a community vote, but
it depends much on official respective teams.
(To be clear: I don't belong to any Rust teams).

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, just pointing it out in case whoever reviews this wasn't aware.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, I didn't even know there is a vote. I just came out with it from my mind. Good to know that we are on the same page and same taste. :D

OpenOptions::new()
}

Expand Down