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

Add aliases to std::path::Path via extension trait #26

Merged
merged 1 commit into from
Sep 19, 2020

Conversation

Emerentius
Copy link
Contributor

This PR is based on #25.

Path has some nice methods which are just shortcuts for free functions in std::fs. path.some_fn() => std::fs::some_fn(path). Having to translate such calls to free function calls to benefit from this crate is bothersome.

This PR adds aliases that work the same way except they call fs_err::some_fn. Because inherent methods take precedence over trait methods, they must have different names. I have simply appended _err to all method names.

I invite bike shedding on the method names and the entire approach.

Defining an extension trait is easier than creating a wrapper struct (unless you're abusing Deref), but also more error prone for the user as they have to make sure to call the right method manually.

@andrewhickman
Copy link
Owner

The extension trait approach sounds fine to me, although its a bit unfortunate that we have to rename the functions

The _err suffix doesn't read very well to me - e.g. canonicalize_err sounds like its canonicalizing an error. Maybe fs_err_* would be clearer?

@Emerentius
Copy link
Contributor Author

Yeah, I can rename it to fs_err_*".

Like with the structs in std::fs, creating a wrapper would give a nicer interface (same method names) but unlike the fs types, Path has a much larger API surface to replicate and I don't mean just the methods but also all the trait impls.

@andrewhickman andrewhickman merged commit 5f7c5c9 into andrewhickman:master Sep 19, 2020
@andrewhickman
Copy link
Owner

Thanks!

@Emerentius Emerentius deleted the path_extensions branch September 19, 2020 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants