feat: add support for ssh completion using files in .ssh/config.d too #529
Conversation
typhoe
changed the title
There was a problem hiding this comment.
Thank you!
You seem to directly source the extracted path "$additional_include_defined_file", but the description you quoted says
Files without absolute paths are assumed to be in ~/.ssh if included in a user configuration file
Could you test whether each additoinal_include_defined_path is a relative path or an absolute path, and prepend "$HOME/.ssh/" if it's a relative path?
edit: Ah, OK. It's WIP. No need to rush, so please take your time.
typhoe
changed the title
|
Sorry, it's my first pull request using github and I didn't realized it would push my commit directly to the pull request, I change the subjet to WIP as soon as I received the github email. I indeed found the 2 issues you immediately saw (missing file for awk cmd and not checking against absolute or relative path), there was also a type Î instead of I for the "Include" regex in the awk cmd and the need for a second 'for' loop to interpret possible globing in the Include option.
|
There was a problem hiding this comment.
Also, can you consider using utilities _omb_util_split and _omb_util_glob_expand?
I just looked at these functions... but I4ll be honest and I'm not sure I understood what/how they do what they do... I'll try to test them and see if I manage to understand. |
|
Thanks.
Maybe I need to add some documentation. The word splitting and the pathname expansions by glob patterns can be affected by various shell settings. Those functions perform word splitting and pathname expansions, respectively, in safe ways. In the present case, you first split the result of |
|
|
I'm going to rebase. |
akinomyoga
force-pushed
the
feat-extend-ssh_completion-to-config.d-directory
branch
from
0056806 to
1ecc6a3
Compare
|
@typhoe I have updated the PR. Could you check it? |
|
I checked your modifications and there was a typo I think: With the change, the ssh completion sorks as intended. |
|
Ah, you are right. Thank you. |
….ssh/config.d too
…e content files for completion
akinomyoga
force-pushed
the
feat-extend-ssh_completion-to-config.d-directory
branch
from
09f75cb to
8e38234
Compare
|
rebased. |
Co-authored-by: Koichi Murase <myoga.murase@gmail.com>
…actoring and performance improvements
* refactor(completions/ssh): move declaration of local variables
* perf(completions/ssh): include the for-loop inside the if-statement of ~/.ssh/config
* refactor(completions/ssh): use glob matching
* perf(completions/ssh): read config files in a single grep&awk
* perf(completions/ssh): match ^Host by awk to reduce use of grep
* fix(completions/ssh): use "_omb_util_{split,expand_glob}" for safer expansions
* refactor(completions/ssh): rename local variables
* refactor(completions/ssh): perform pathname expansion at once
* fix(completions/ssh): correct to array var in relative or absolute transform loop
Co-authored-by: Stéphane Juventy <sjuventy.ext@orange.com>
Co-authored-by: me <stephane.juventy@gmail.com>
* style(completions/ssh): normalize quoting
akinomyoga
force-pushed
the
feat-extend-ssh_completion-to-config.d-directory
branch
from
8e38234 to
c2ca9be
Compare
|
Thank you for your contribution! I've merged it. |
From 7.3p1 and up, there is an Include keyword, which allows you to include additional configuration files.
So if you have
Include config.d/*in your~/.ssh/configfile, this pull request allows files put in~/.ssh/config.d/to be also parsed for theHostkeyword to use completion when calling ssh command (in addition to the default~/.ssh/config)Why would you want to do that: It's easier to maintain lists of hosts in dedicated files (e.g. separate private-home-lan, company1-project1, company2-project3, etc...)