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

path type different than std::fs::File #34

Open
dbregman opened this issue Jan 31, 2021 · 5 comments
Open

path type different than std::fs::File #34

dbregman opened this issue Jan 31, 2021 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@dbregman
Copy link

std::fs::File uses AsRef<Path> as the type for passing paths while fs_err uses Into<PathBuf>

This makes fs_err not be a "drop in replacement": When I pass an AsRef<Path> to fs_err::File::open, it does not compile

@LPGhatguy LPGhatguy added the documentation Improvements or additions to documentation label May 24, 2021
@LPGhatguy
Copy link
Collaborator

LPGhatguy commented May 24, 2021

This is true. Maybe we should qualify this statement with an asterisk and explain how we differ?

Alternatively, maybe we should change our bounds to either P: AsRef<Path> or P: AsRef<Path> + Into<PathBuf>. The latter would let us avoid an allocation but doesn't really solve this issue.

@kellpossible
Copy link

I also hit this, and had to rework some code. I'm guessing the error types need to own the path and that's why this is done?

@TonalidadeHidrica
Copy link

Is it really necessary that the error should own the path? Can't it just store the reference to the path instead?

@hniksic
Copy link
Contributor

hniksic commented Jun 8, 2023

@TonalidadeHidrica The error must own the path, it cannot be a reference because that would make both the wrapper and the error non-'static. Requiring Into<PathBuf> rather than AsRef<Path> is basically an optimization, allowing fs_err to avoid an allocation when you pass an actual PathBuf.

This issue should probably be resolved by a PR that improves the docs to explain this, and show how to change the code that doesn't compile due to the difference. (The change is trivial, but can be perplexing to a newbie.)

@andrewhickman
Copy link
Owner

andrewhickman commented Jun 12, 2023

Its worth noting that all uses of Into<PathBuf> are for constructing an fs_err::File, which owns a PathBuf even in the happy path. All the other functions were changed to AsRef<Path> in #31 and will allocate only in the failure path.

I'd be happy to accept a PR to improve docs around this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

6 participants