ohmybash / oh-my-bash

A delightful community-driven framework for managing your bash configuration, and an auto-update tool so that makes it easy to keep up with the latest updates from the community.
https://ohmybash.github.io
MIT License
5.53k stars 624 forks source link

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

Closed typhoe closed 1 month ago

typhoe commented 4 months ago

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 commented 4 months ago

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 filein.ssh_dir Include /absolute_path/test_file

typhoe commented 4 months ago

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 !

akinomyoga commented 4 months ago

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 commented 2 months ago

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 commented 2 months ago

I'm going to rebase.

akinomyoga commented 2 months ago

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

typhoe commented 2 months ago

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

akinomyoga commented 2 months ago

Ah, you are right. Thank you.

akinomyoga commented 1 month ago

rebased.

akinomyoga commented 1 month ago

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