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

Idea: Add camino::absolute() #94

Closed
noamraph opened this issue Aug 15, 2024 · 6 comments · Fixed by #95
Closed

Idea: Add camino::absolute() #94

noamraph opened this issue Aug 15, 2024 · 6 comments · Fixed by #95

Comments

@noamraph
Copy link

Currently in order to get the absolute path of a Utf8Path, it seems that I need to do:

Utf8PathBuf::from_path_buf(absolute(path)?).unwrap()

How about adding an absolute() function to camino?

I'll be happy to try to write a PR if you approve of the idea.

Thanks!
Noam

@sunshowers
Copy link
Collaborator

Thanks for the suggestion! "absolute" can mean several things depending on the context. The closest thing currently supported is canonicalize and the related canonicalize_utf8.

In general camino follows Rust's Path API, so I'd be hesitant to add methods that aren't already in Rust's Path. But you're welcome to have your own function, of course.

@noamraph
Copy link
Author

I now see I didn't mention the standard Rust API. I meant to follow's Rust's std::path::absolute, without inventing any new API. Does that seem like a good idea?

@sunshowers
Copy link
Collaborator

Oh, gotcha! Yeah that makes sense then.

sunshowers added a commit to sunshowers/camino that referenced this issue Aug 18, 2024
Note that we take `AsRef<Path>`, not `AsRef<Utf8Path>`, for the reason
mentioned in the comment.

Closes camino-rs#94.
sunshowers added a commit to sunshowers/camino that referenced this issue Aug 18, 2024
Note that we take `AsRef<Path>`, not `AsRef<Utf8Path>`, for the reason
mentioned in the comment.

Closes camino-rs#94.
@sunshowers
Copy link
Collaborator

This is now out in camino 1.1.9. Thanks for the issue!

@sunshowers
Copy link
Collaborator

I'll be happy to try to write a PR if you approve of the idea.

Ohhh I just noticed this, sorry! I'd love a future contribution from you if you ever have an idea. Thanks!

@noamraph
Copy link
Author

Ohhh I just noticed this, sorry! I'd love a future contribution from you if you ever have an idea. Thanks!

Thanks! Really no problem, it would surely have taken longer if I had tried to implement it. For example, I wouldn't have thought of adding the Rust version check. So thanks for adding this so quickly!

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 a pull request may close this issue.

2 participants