Tracking issue for File::with_options stabilisation

[Timmmm]

Implemented in #65429, behind feature flag with_options.

[Kinrany]

Just a thought about an alternative API:

let f = File::with_options("file.txt", |o| o.read(true).create(true));

[LPeter1997]

From the name I'd associate it with a function that takes an OpenOptions parameter, just like Vec::with_capacity takes a capacity. Basically not a builder type. For consistency's sake, why not either add a File::with_options(options: &OpenOptions) or rename it to something like File::builder()?

[Timmmm]

LPeter1997, that was my original suggestion but people overwhelmingly preferred this API.

[TheRadioGuy]

I agree with @LPeter1997. I think it'd be better to rename it to something like File::options()

[Timmmm]

I guess File::options() makes more sense when you just look at that API but File::with_options() makes more sense in typical usage.

let options = File::options(); // Ok
let options = File::with_options();  // Weird

let file = File::with_options().read(true).create(true).open("foo.txt"); // Ok
let file = File::options().read(true).create(true).open("foo.txt"); // Bit weirder. Maybe it is better actually?

Not that this is a democracy, but let's have a poll: upvote one or more of the following options...

[Timmmm]

pub fn with_options() -> OpenOptions {
  OpenOptions::new()
}

let options = File::with_options();
let file = File::with_options().read(true).create(true).open("foo.txt");

[Timmmm]

pub fn options() -> OpenOptions {
  OpenOptions::new()
}

let options = File::options();
let file = File::options().read(true).create(true).open("foo.txt");

[Timmmm]

pub fn builder() -> OpenOptions {
  OpenOptions::new()
}

let options = File::builder();
let file = File::builder().read(true).create(true).open("foo.txt");

[Kinrany]

From the API discovery point of view it's not great to have the only actual verb be the last in chain.

From the point of view of making code nicer to read, it's not great that open is not in any way syntactically distinguished from the rest of the methods. The reader has to inspect the types to find out that all but the last one return the same OpenOptions type. (It doesn't help that other methods are verbs too, even though they refer to fields and would be nouns in most other builders.)

[tesuji]

Update: The team doesn't have bandwidth to deal with bike-shredding about naming of this API,
at least for the current situation.
So the stabilization date of the API will be due until T-libs has more time.

[simias]

FWIW I don't think that having open not be syntactically distinguished is a big problem. Sure, it would be better if it was, but I can't really come up with a non-intrusive way to do that. All the alternatives I can come up with involve some sort of closure call, but that adds a lot of noise when the whole point of this API is to reduce it.

Besides, that's already how the stable OpenOptions works anyway.

Furthermore I think that the intent of a line such as:

let file = File::options().read(true).create(true).open("foo.txt");

is abundantly clear even if it's the first time you encounter it. And if you don't understand how it's implemented under the hood it's not a huge problem either, since any attempt to reorder the chain to put open in the middle of it will result in a compilation error.

Regarding the naming I don't have a strong preference, but I do agree that using a with_ prefix seems mildly misleading due to the (rather soft) convention of with_ methods taking the object as parameter instead of returning it (with_hasher, with_capacity, with_file_name, etc...).

I think it's probably worth taking the time to figure out what the conventions should be for these builder patterns in the general case, since whatever convention ends up being adopted by std will become the de-facto standard even for third party crates.

For this reason I very mildly favor the generic builder name. We know that, by convention, basic constructors in rust are named new, I think it would make sense for builder constructors to have a unique default name regardless of the class and builder fits the bill. options feels a bit less generic since it's possible for a builder to accept things that aren't strictly options.

[kangalio]

Why does such a method need to exist in the first place? std::fs::OpenOptions::new() already exists. Having two options for the same things is confusing and annoying.

[simias]

@kanglioo it's just sugar to avoid having to import OpenOptions on its own. You could also argue that semantically it makes sense to tie OpenOptions explicitly to File since it's effectively a fancy builder for it. IMO it makes the code more readable on top of adding a small convenience when writing it.

[joshtriplett]

We discussed this briefly in today's @rust-lang/libs meeting. We didn't come to a firm conclusion on naming. We did generally feel like with_ methods tend to be constructors of the type itself (such as Vec::with_capacity), not methods to return builders, so the name with_options doesn't seem right to us.

We don't have any specific convention for methods on type X that return a builder type that'll build an X; if we want to use that pattern, we should pick such a convention.

We've seen the convention ::builder() used in crates on crates.io, and it seems potentially reasonable, but we didn't come to a firm conclusion in the meeting.

[BurntSushi]

I am a fan of the builder name personally. And I do like this pattern. I have been meaning to introduce it in my various crates for a while now, but just haven't had the time. I think it's a nice ergonomic win.

With that said, usually the naming convention is Foo and FooBuilder. But here, we have File and OpenOptions. Of course, OpenOptions is a builder in all but name, so I think using the name builder is justifiable. But I could see just options working too.

I agree with the libs team meeting consensus that with_options isn't great.

[Kinrany]

Is it feasible to introduce FileBuilder, deprecate OpenOptions and redefine OpenOptions as an alias of FileBuilder?

[BurntSushi]

@Kinrany I'm personally not a fan of causing churn like that for such a small benefit.

[Timmmm]

So it sounds like the with_options() option (heh) is probably out (and it has the fewest votes anyway).

So the choice is between builder() and options(), depending on which consistency you care about more:

• Consistency with other crates' use of builder() to follow the builder pattern (because OpenOptions is sort of a builder).
• Consistency with the fact that it returns OpenOptions not FileBuilder or OptionsBuilder.

I think I still slightly prefer options() because it File::builder() sounds like you're building a file and its contents but you're actually just opening a file. OpenOptions doesn't exactly follow the builder pattern anyway which I think would be more like File::builder().create(true).filename("foo.txt").build().

I don't think we'll ever find a really strong reason to go with either though so maybe it would make sense for the libs team just to do a straw poll amongst themselves and go with one.

[sfackler]

I think I still slightly prefer options() because it File::builder() sounds like you're building a file and its contents but you're actually just opening a file.

This is a pretty compelling argument, IMO, though I would be fine with builder() as well.

[m-ou-se]

it's just sugar to avoid having to import OpenOptions on its own. You could also argue that semantically it makes sense to tie OpenOptions explicitly to File since it's effectively a fancy builder for it.

This goal could also be achieved using inherent associated types (#8995), such that we can add impl File { type OpenOptions = OpenOptions; } to make this work:

let f = File::OpenOptions::new().create(false).write(true).open("abc")?;

[m-ou-se]

Since there is already a lot of code with OpenOptions, I'd be against something like File::builder(), because it isn't immediately clear that's the same as OpenOptions::new(). Something with options or open_options in the name would make it easier to guess/remember that it's just an OpenOptions. (This is also an argument in favour of File::OpenOptions, as in the comment above.)

[BurntSushi]

I think I'd prefer options() over associated types here. Associated types feels a bit... much for this? (Unless we really want that to be a pattern for builders throughout the ecosystem.)

[joshtriplett]

👍 for File::options().

[workingjubilee]

Considering the appearance of what looks like a strong consensus, PR #85766 renames the function to File::options and proposes its stabilization. This is further motivated by the appearance of a strong linguistic convention of with_options functions taking args, whereas this is a nullary function.

[m-ou-se]

I'm not entirely convinced that adding File::options() is a good idea.

I agree with these comments made above:

Having two options for the same things is confusing and annoying.

From the API discovery point of view it's not great to have the only actual verb be the last in chain.

However, I also agree that having to import OpenOptions separately from File to open a File with some options isn't great either.

I think the concerns I mentioned can be addressed by naming it File::open_options(), since that puts the verb ('open') directly at the beginning, makes rustdoc sort it right next to File::open in the method overview (making it easier to discover), and makes it a bit clearer it's identical to OpenOptions::new().

The downside is that open is mentioned twice: File::open_options().abc(1).xyz(2).hello(3).open("file"). However, since this will often be used with several options formatted over multiple lines, that seems okay to me.

In addition, I think it'd be a good idea to check what combinations of options are commonly used, and add new File constructors for the most common ones. E.g. File::open_rw or File::open_append or File::create_new.

[BurntSushi]

@m-ou-se I like File::open_options. I think I do have a slight preference toward File::options, but I think File::open_options is a fine name too and I like your arguments in favor of it.

makes rustdoc sort it right next to File::open in the method overview (making it easier to discover)

My goodness! I have so thoroughly ignored that area of rustdoc that this is the first time I've realized that its names are sorted lexicographically in contrast to source-order. That's actually pretty useful.

In addition, I think it'd be a good idea to check what combinations of options are commonly used, and add new File constructors for the most common ones. E.g. File::open_rw or File::open_append or File::create_new.

I like File::open_append, but am skeptical of the others. However, I would bow to data showing that they're common.

[Timmmm]

open_options seems reasonable but I think all of the options (heh) are reasonable. Doesn't seem like it's enough better to bother changing the PR again.

Also I'm amused that you said you don't think there should be more than one way to do things, but you also want lots of open_append() style convenience methods!

[BurntSushi]

Also I'm amused that you said you don't think there should be more than one way to do things

Where did I say this? While I consider "fewer ways to do something" a general abstract good thing, it is not a rule I seek to impose everywhere. If I've ever said anything otherwise recently-ish, then I was likely being imprecise.

[joshtriplett]

FWIW, I think File::open_options feels potentially confusing. Based on other library crates, I would expect open_options to be an open method that takes options, not something that creates a builder; in other words, I'd expect it to have a signature like fn open_options(opts: OpenOptions) -> File.

[workingjubilee]

I think the argument about whether something sounds like a nullary function or not is valid, and is reasonable grounds to exclude File::with_options() from the running. But we already have OpenOptions as a very... interesting type name. So to me, I agree that nullary File::open_options() is quirky, but I also think it's "natural" to Rust, simultaneously, because of the existing legacy of OpenOptions. I believe it's reasonable to lean into the quirkiness. But of course, File::options() is also good to me.

So, I feel it is copacetic either way, and

open_options seems reasonable but I think all of the options (heh) are reasonable. Doesn't seem like it's enough better to bother changing the PR again.

I am happy to repaint the bikeshed if people can agree on a color!

[joshtriplett]

On Fri, Jun 04, 2021 at 10:09:34AM -0700, Jubilee wrote: I think the argument about whether something sounds like a nullary function or not is valid, and is reasonable grounds to exclude `File::with_options()` from the running. But we already have `OpenOptions` as a very... interesting type name. So to me, I agree that nullary `File::open_options()` is quirky, but I also think it's "natural" to Rust, simultaneously, because of the existing legacy of OpenOptions. I believe it's reasonable to lean into the quirkiness. But of course, `File::options()` is also good to me.

One difference is that `OpenOptions` is a type, not a function, so it's more obvious that `OpenOptions` itself doesn't perform an open operation. A function named `File::open_options`, living right next to `File::open`, seems more ambiguous. I'm not set on the function being called `File::options`; I just don't think the function name should start with `open`.

[m-ou-se]

agree on a color

Pink!

[Timmmm]

Where did I say this?

Sorry, "you" was referring to m-ou-se.

[simias]

I'd like to restate that IMO options() has the advantage of being a generic name that could be reused for other APIs which in turn could help discoverability. It could join new(), iter() and other well known method whose semantic you can guess without even having to look at the docs. You could have an option() method for an OpenGL context object, a database handle object, a cryptographic algorithm object etc...

open_options() is also fine, but that would make it semantically a little more "narrow" IMO. I suppose that for a DB handle the metaphor also works, but you don't typically "open" a SHA1 context for instance, you create it.

But I agree that this is a pure bikeshed at this point. If open_options is favoured, I'm absolutely fine with it.

[workingjubilee]

Recap

• m-ou-se suggests File::open_options() as an alternative, because it sorts directly next to File::open(Path) in the method overview, and proposes other shorthands like File::open_rw(), File::open_append(), or File::create_new().
  • Question: Is the function sorting algorithm for rustdoc stable or likely to change?
  • Question: Where does create_new() come in here? How exactly would it differ from create(Path) in usage?
• Josh suggests OpenOptions is meaningfully distinct from File::open_options() because it's a type (a "noun") not a function (a "verb"), and thus dissociated from File.
• Josh suggests fn open_options(arg) -> OpenOptions is what he would expect, on the basis of the "verb" being included and implying something similar to with_.
  • Question: If this is an expected pattern, maybe other examples of this already exist? What do programmers currently write, and thus, expect?
• Jubilee (hi!) suggests the name is quirky regardless.
  • Question: How did we arrive at the original name anyways?
• Josh suggests that fn open(Path) and fn open_options() are unnecessarily ambiguous next to each other.
  • Question: Isn't that the point, actually? That both are valid ways to start the same code string?
• simias believes that fn options() will carry more weight over time (alongside fn option()) and become a common library pattern.
  • Question: If it seems like a commonly reusable pattern, maybe it has already arisen? What do programmers currently write, and thus, expect?

Other Ideas

If the problem is imports, why not just use an std::fs::prelude::* a la std::io::prelude::*?

Reflections

Rereading this reveals what now feels like a misstep in the early drafts of the File API, and I would like to offer these remarks to both m-ou-se's and Josh's comments regarding the relationship (or not) of OpenOptions, fn open_options, and fn open:

I don't think it's a problem that fn open_options is nullary whereas fn open is unary, or that one is a constructor for a Result<File> and the other is a constructor for an OpenOptions that will eventually yield a Result<File>.

I think the problem is that they are actually logically unrelated, or perhaps I should say that OpenOptions is the distant ancestor of File::open(Path), not the immediate parent. OpenOptions is far more ambiguous than what you can get from File::open(Path) because it can imply creating a file, appending to an existing file, reviewing and editing the contents of an existing file, or opening a file in read-only mode. File::open(Path) only supports the last. Thus File::open_options() would only bear a passing resemblance in use to File::open(Path), which would actually be something more like File::read(Path) if I had my druthers.

This difference itself would not be a huge issue, honestly, but we have a relatively strong norm of highly "patterned" names in other std APIs, and people would expect two names which use the same initial word to have similar meaning, when one is far different. But I don't believe that is a definitive argument against using fn open_options() per se, merely that it is an issue. I would probably want to research answers to some of the other questions more.

[workingjubilee]

A quick skim of grep.app for open_options reveals that the dominant use of it is "a variable or struct field that is OpenOptions". In one or two cases it is in fact used as "open with this OpenOptions".

Searching for the regexp /fn open_.*\(/ yields quite a lot of examples of "open_something that takes an argument", it's true.

/fn options?(<.*>)?\(/ shows that fn option and fn options are both about equally often a "getter" and a nullary constructor, and sometimes it also takes an argument other than self.

So there's some data: clear as mud! 😃

[jplatte]

Another possibility: First introduce additional File constructors such as File::(open_)?append, then re-evaluate how much the additional OpenOptions constructor is.

[yaahc]

This has been stalled for a while and so I'm nominating it for discussion in our next meeting to help it get unblocked.

[yaahc]

The PR stabilizing this feature, #85766, has entered its final comment period with disposition merge.