-
-
Notifications
You must be signed in to change notification settings - Fork 674
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
completions/ssh: Include global ssh hosts #569
completions/ssh: Include global ssh hosts #569
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution. I merged another PR of ssh_config #529. The suggested addition of the support for /etc/ssh/ssh_{config,known_hosts}
is good. However, I'd like to reduce the code duplicates before merging.
Could you rebase on top of the latest master
and modify the code to store the found files in an array and run e.g. awk /^HOST/ ... "${config_files[@]}"
at once?
local -a config_files=()
if [[ -r ~/.ssh/config ]]; then
config_files+=(~/.ssh/config)
...
fi
if [[ -r /etc/ssh/ssh_config ]]; then
config_files+=(/etc/ssh/ssh_config)
fi
COMPREPLY+=($(compgen -W "$(awk '/^Host/ {for (i=2; i<=NF; i++) print $i}' "${config_files[@]}")" "${options[@]}"))
Similar for known_hosts.
Will do - |
Ah, right. I think the Include there can also be processed. |
1645fc3
to
3520883
Compare
* fix(completions/ssh): localize variables * fix(completions/ssh): quote parameter expansions * perf(completions/ssh): use parameter expansion to get directory name
649799f
to
e155f9a
Compare
Partial fix for #568