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

feat: add new "with_path" module, which accepts and returns paths instead #18

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hasezoey
Copy link

add a module named "with_path"
add function "with_path::tilde_with_context" that uses Paths instead
"with_path::tilde" that uses Paths instead

fixes #15


i tried my luck with fixing #15, but because this is a massive single-file library, i opted to using a sub-module to keep it clean, also the functions in the sub-module currently have the same name, because they are literally the same only that they accept & return a Path instead

also, with_path::tilde_with_context was changed so that it works without problems on Path

the tests have also been copied and extended for the new functions


in the future i would recommend to maybe switch to the with_path implementation completely so that there is less confusion and because str / String can be used in a AsRef<Path>

@ijackson
Copy link

ijackson commented Aug 3, 2022

Hi. Thanks for the contribution.

As discussed in #17 I am taking over maintenance of this crate. The new repository is here: https://gitlab.com/ijackson/rust-shellexpand

I agree that this would be a good feature to have. I have opened a ticket https://gitlab.com/ijackson/rust-shellexpand/-/issues/2 in my new gitlab repo for this feature. However, I have some qualms about the implementation - I've discussed them in the ticket.

So I have not merged this. AIUI this github repo is going to be archived soon.

I would still welcome a contribution of this feature, along the lines I discuss in that ticket. If you would still like to work on this, please come to the gitlab repo and we can talk about it there. Given that this seems not entirely trivial, it might be a good idea to have a chat with me in the ticket before diving in and writing code.

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.

[Question] what is the reason that "AsRef<str>" is used as input instead of "AsRef<Path>"?
2 participants