Re: patch for ssh completion

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Jeremy Cantrell wrote on Tue, Feb 28, 2017 at 11:17:29 -0800:
> Hello!
> 
> I've found a tiny problem with the ssh completion script where any host
> entries defined in included configuration files do not appear as choices
> when completing. I've attached a patch that fixes it for me. Below is an
> example that illustrates the problem:
> 
> ~/.ssh/config:
> Include hosts
> 
> ~/.ssh/hosts:
> Host example
> HostName example.local

From ssh_config(5):

     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.

> Thanks,
> Jeremy

> --- /usr/share/zsh/functions/Completion/Unix/_ssh	2016-12-22 11:27:00.000000000 -0800
> +++ packages/base/.local/share/zsh/functions/_ssh	2017-02-26 22:37:47.513422235 -0800
> @@ -681,16 +681,21 @@
>    fi
>    if [[ -r $config ]]; then
>      local key hosts host
> +    local filename configs=($config)
> +    grep '^Include\b' "$config" | sed 's/\s\+/ /g' | cut -d' ' -f2 |
> +    while read -r filename; do
> +        config=$HOME/.ssh/$filename

This handles your example, but not the other possibilities documented in
the man page, e.g., absolute paths.  I expect using those would cause
errors on stderr.  (I can't test this since my ssh isn't new enough.)

The «\b» in grep is not in POSIX.  It could be changed to match
whitespace explicitly.  I assume we'll also want to use builtin
functionality such as ${(M)…:#Include*} instead of external executables.

Can filenames be quoted?  I.e., does «Include "foo"» include ~/.ssh/foo
or ~/.ssh/\"foo\"?

Can Include directives be nested?  If they can, it'd be nice to handle
that.  (But I don't think that's a blocker to accepting the patch)

>          while IFS=$'=\t ' read -r key hosts; do
>          if [[ "$key" == (#i)host ]]; then
>              for host in ${(z)hosts}; do
>                  case $host in
>                  (*[*?]*) ;;
>                  (*) config_hosts+=("$host") ;;
>                  esac
>              done
>          fi
>          done < "$config"
> +    done

Thanks for the patch.

Cheers,

Daniel