oh-my-bash icon indicating copy to clipboard operation
oh-my-bash copied to clipboard

feat: add support for ssh completion using files in .ssh/config.d too

Open typhoe opened this issue 1 year ago • 8 comments

From 7.3p1 and up, there is an Include keyword, which allows you to include additional configuration files.

Include
Include the specified configuration file(s). Multiple pathnames may be specified and each pathname may contain glob(3) wildcards and, for user configurations, shell-like “~” references to user home directories. Files without absolute paths are assumed to be in ~/.ssh if included in a user configuration file or /etc/ssh if included from the system configuration file. Include directive may appear inside a Match or Host block to perform conditional inclusion. Source: ssh_config(5).

So if you have Include config.d/* in your ~/.ssh/config file, this pull request allows files put in ~/.ssh/config.d/ to be also parsed for the Host keyword 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...)

typhoe avatar Feb 20 '24 13:02 typhoe

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. My bad !

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. The last commit seems to work correctly this time Tested with various Include:

Include config.d/* Include file_in_.ssh_dir Include /absolute_path/test_file

typhoe avatar Feb 22 '24 09:02 typhoe

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. I'll try my best !

typhoe avatar Feb 22 '24 15:02 typhoe

Thanks.

I just looked at these functions... but I4ll be honest and I'm not sure I understood what/how they do what they do...

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 awk and, after modifying the relative path, perform the pathname expansion. In this case, we don't want to perform the pathname expansion in the first splitting stage. Also, the pathname expansions can be suppressed by shell settings of set -f or GLOBIGNORE. Other non-trivial shell options can also affect the behavior of the pathname expansions.

akinomyoga avatar Feb 22 '24 15:02 akinomyoga

I just looked at these functions... but I4ll be honest and I'm not sure I understood what/how they do what they do...

Maybe I need to add some documentation.

b007c732d910f3db7fe1ba699ae9ab3e0e6cede6

Also, can you consider using utilities _omb_util_split and _omb_util_glob_expand?

f5d48c39fc978fb8492ee98dd3d5c40a54a9dd15

akinomyoga avatar Apr 28 '24 05:04 akinomyoga

I'm going to rebase.

akinomyoga avatar Apr 28 '24 05:04 akinomyoga

@typhoe I have updated the PR. Could you check it?

akinomyoga avatar Apr 28 '24 05:04 akinomyoga

I checked your modifications and there was a typo I think: In your "relative or absolute path, if relative transforms to absolute" rewrite, you forgot that the var was an array

With the change, the ssh completion sorks as intended. Thank you for finishing this PR, I'm sorry I didn't had the time to do it myself

typhoe avatar Apr 29 '24 10:04 typhoe

Ah, you are right. Thank you.

akinomyoga avatar Apr 29 '24 11:04 akinomyoga

rebased.

akinomyoga avatar May 09 '24 01:05 akinomyoga

Thank you for your contribution! I've merged it.

akinomyoga avatar May 09 '24 01:05 akinomyoga