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

allow home by env as $HOME in paths #469

Closed

Conversation

madogiwa0124
Copy link

Added support for $HOME in relation to #464, #465

@madogiwa0124 madogiwa0124 changed the title allow tilde as $HOME in paths allow home by env as $HOME in paths Jul 20, 2019
@leehambley
Copy link
Member

Is there a very good reason why you cannot use ~?

Expanding ~ is a Bash (and other shells) feature https://www.gnu.org/software/bash/manual/html_node/Tilde-Expansion.html which is distinct from the expansion of environment variables.

Whilst you may find it convenient to permit $HOME in paths, many people may be distressed and confused when finding out that it does not in fact do environment variable expansion and is simply a string replacement alias.

I prefer not to support confusing APIs, and would prefer not to merge this contribution, but I am open to discussion about the relative merits.

Thanks, and sorry it took me some time to get back to you, I've been travelling for work.

@madogiwa0124
Copy link
Author

@leehambley

I see 😄

Whilst you may find it convenient to permit $HOME in paths, many people may be distressed and confused when finding out that it does not in fact do environment variable expansion and is simply a string replacement alias.

Since I used $ HOME for my service, I created a PR, I agree with your opinion.
I fix my service to use a tilde. 👍

Thanks! ✨

@leehambley
Copy link
Member

Thank you @madogiwa0124 - I look forward to future contributions from you :) Thank you for using and trusting SSHKit 🙇‍♂️

@madogiwa0124 madogiwa0124 deleted the allow-tilde-home-env branch July 26, 2019 08:28
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