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

handle home directory expansion #205

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

skewballfox
Copy link

currently if you supply a path with a tilde character, you get a runtime panic. I added a function to simply replace ~ with the home directory. It adds one dependency, though one maintained by the cargo team. This could be done without adding another dependency, though using a deprecated function std::env::home_dir()(which honestly the dependency is just calling and handling the edge cases not handled).

If you want me to change anything, let me know.

@zoni
Copy link
Owner

zoni commented Dec 31, 2023

Truthfully, I'm quite on the fence about this. Path expansion should be the responsibility of the user's shell in my opinion, and allowing homedir/tilde expansion would open up the door to other similar requests in the future.

@skewballfox
Copy link
Author

Truthfully, I'm quite on the fence about this. Path expansion should be the responsibility of the user's shell in my opinion, and allowing homedir/tilde expansion would open up the door to other similar requests in the future.

fair, though the reason I opened the PR was because I ran into an edge case where the ~ character wasn't expanded: the path was in an env variable being used in a bash script, and in this situation, the path wasn't expanded. I suppose it might be better to handle within that shell script, given there are situations this doesn't cover (like ~foo to expand to /home/foo).

If you would rather not merge that's totally fine.

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.

None yet

2 participants