Skip to content

PathBuf set_file_name and with_file_name need docs for input abspaths #50116

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

Open
pnkfelix opened this issue Apr 20, 2018 · 3 comments
Open

PathBuf set_file_name and with_file_name need docs for input abspaths #50116

pnkfelix opened this issue Apr 20, 2018 · 3 comments
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Apr 20, 2018

Looking at:

https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.with_file_name

and

https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.set_file_name

I had thought, based on what was written there, that the code would extract the filename alone from the input, dropping the directory prefix.

But apparently what will actually happen is that it drops all the content from self and just turns it into a copy of the input.

(And if you give it a relative path with multiple components, then it effectively pushes all of the components onto self)

I don't actually mind the current behavior, and it is easy enough to workaround myself. (Basically, I think set_file_name and with_file_name are somewhat misleadingly misnamed...) But we should document it it, perhaps with concrete examples.

Here is an example of what I am getting at (playpen):

fn main() {
    use std::path::Path;

    println!("EXAMPLE 1");
    let path1 = Path::new("/dir1/file1.txt");
    let path2 = Path::new("/dir2/file2.txt");
    println!("path1: {}", path1.display());
    println!("path2: {}", path2.display());
    println!("path1.with_file_name(path2): {}", path1.with_file_name(path2).display());
    println!("Felix expected /dir1/file2.txt");
    
    println!("");
    println!("EXAMPLE 2");
    let path1 = Path::new("dir1/file1.txt");
    let path2 = Path::new("dir2/file2.txt");
    println!("path1: {}", path1.display());
    println!("path2: {}", path2.display());
    println!("path1.with_file_name(path2): {}", path1.with_file_name(path2).display());
    println!("Felix expected dir1/file2.txt");
}
@pnkfelix pnkfelix added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 20, 2018
@pnkfelix
Copy link
Member Author

pnkfelix commented May 4, 2018

Actually after thinking on the matter further, I'm having a hard time convincing myself of a scenario where the current behavior is anything but indicative of a bug in the client code calling this API.

In other words: Should we consider changing the API to instead panic if set_file_name is given an absolute path as input?

@retep998
Copy link
Member

retep998 commented May 4, 2018

How would you determine what an absolute path is? On windows for example, C:foo is a perfectly valid "filename" in that the name of the file itself is actually C and it refers to the foo alternate file stream, but still a filename. Would we consider / on Windows as making something a path and not just a single component suitable for a filename? \\?\ paths don't use / as a separator and treat them as just another character in the component for example.

@pnkfelix
Copy link
Member Author

Okay so I guess I’m back to thinking that this is a weakness in the docs

@XAMPPRocky XAMPPRocky added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Sep 25, 2018
@ChrisDenton ChrisDenton added the A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` label Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants