rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

ACP: `PathBuf::make_dir` #335

Open clarfonthey opened 5 months ago

clarfonthey commented 5 months ago

Proposal

Problem statement

Rust's Path types require this slash to treat paths as a directory. While methods like push will always treat the current path as a directory, other methods like set_file_name and set_extension won't.

Motivating examples or use cases

It's common for users to write paths while omitting the trailing slash, like so:

windows_path = "C:\\Users\\Me"
unix_path = "/home/me"

When converting these strings to paths via methods like Path::new, it could be desirable to normalise these into the trailing-slash form to ensure that methods work correctly, like set_file_name. For example, a method to check for a configuration file in a directory might look like this:

let mut buf = PathBuf::from(some_path_string);
for filename in &[/* ... */] {
    buf.set_file_name(filename);
    match File::open(&buf) {
        Err(err) if err.kind() == io::ErrorKind::NotFound => continue,
        res => return res,
    }
}

In this case, the set_file_name call will only work as expected if the initial path is a directory. If it's not, the last component of the directory will be overwritten.

Solution sketch

The proposal is to add a single method, PathBuf::make_dir, which is equivalent to calling push("").

Alternatives

As alluded above, push("") will be equivalent to make_dir, since adding an "empty" component will simply add a trailing slash. However, it's not clear that this will always work as expected, and that's why this method is proposed.

In particular, make_dir has some obvious properties that aren't clear from push(""):

Links and related work

N/A

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution:

ChrisDenton commented 5 months ago

I agree that this handling of trailing / isn't ideal. There is also no good way to tell if a path ends with a path separator.

The result will always have is_dir() == true

Path::is_dir calls out to the OS (and therefore the filesystem) so we can't make that claim.

clarfonthey commented 5 months ago

I agree that this handling of trailing / isn't ideal. There is also no good way to tell if a path ends with a path separator.

The result will always have is_dir() == true

Path::is_dir calls out to the OS (and therefore the filesystem) so we can't make that claim.

Oh, I completely missed that this actually calls out to the filesystem, and thought it was referring to the path itself. Will delete that part.

cuviper commented 5 months ago

Similarly, make_dir sounds to me like it's going to actually call mkdir.

clarfonthey commented 5 months ago

RIP my ability to name things, the shed is still under construction. x_x

cuviper commented 5 months ago
  • The result will always have file_name() == None

I don't think that works like you expect either -- the examples on that method include "/usr/bin/" returning Some("bin").

How about Path::has_dir_suffix, Path::with_dir_suffix, and PathBuf::set_dir_suffix?