PATCH: Improve Mosh completion

PATCH: Improve Mosh completion

[Andrei Shevchuk]

[-- Attachment #1: Type: text/plain, Size: 3984 bytes --]

This adds ~/.ssh/config based completion support (_ssh_hosts and 
_ssh_users functions and userhost state handling taken without 
modifications from ssh completion file), existing code was just wrapped 
into _mosh function without modifications (except userhost state 
handling mentioned above).

---
Andrei Shevchuk


diff --git a/Completion/Unix/Command/_mosh 
b/Completion/Unix/Command/_mosh
index 431fdbf..086aaf3 100644
--- a/Completion/Unix/Command/_mosh
+++ b/Completion/Unix/Command/_mosh
@@ -1,26 +1,84 @@
 #compdef mosh

-local curcontext="$curcontext" state line
-local -a _comp_priv_prefix
-
-_arguments -C \
-  '(-)--help[display help information]' \
-  '(-)--version[display version information]' \
-  "--no-init[don't set terminal init string]" \
-  '--ssh=[specify ssh command to setup session]:ssh command:_normal' \
-  '--port=[specify server-side port range]:port:_sequence -n 2 -s \: 
_ports' \
-  '(-a -n)--predict=[control speculative local echo]:mode:(adaptive 
always never)' \
-  '(--predict -n)-a[synonym for --predict=always]' \
-  '(--predict -a)-n[synonym for --predict=never]' \
-  '--server[specify command to run server helper]:remote file:_files' \
-  '--client[specify command to run client helper]:_command_names -e' \
-  '1:remote host name:->userhost' \
-  '*:::args:_normal' && return
-
-case $state in
-  userhost)
-    _hosts || _user_at_host && return
-  ;;
-esac
-
-return 1
+_mosh () {
+  local curcontext="$curcontext" state line expl tmp ret=1
+  local -a _comp_priv_prefix
+  typeset -A opt_args
+
+  _arguments -C \
+    '(-)--help[display help information]' \
+    '(-)--version[display version information]' \
+    "--no-init[don't set terminal init string]" \
+    '--ssh=[specify ssh command to setup session]:ssh command:_normal' 
\
+    '--port=[specify server-side port range]:port:_sequence -n 2 -s \: 
_ports' \
+    '(-a -n)--predict=[control speculative local echo]:mode:(adaptive 
always never)' \
+    '(--predict -n)-a[synonym for --predict=always]' \
+    '(--predict -a)-n[synonym for --predict=never]' \
+    '--server[specify command to run server helper]:remote 
file:_files' \
+    '--client[specify command to run client helper]:_command_names -e' 
\
+    '1:remote host name:->userhost' \
+    '*:::args:_normal' && return
+
+  case $state in
+    userhost)
+      if compset -P '*@'; then
+        _wanted hosts expl 'remote host name' _ssh_hosts && ret=0
+      elif compset -S '@*'; then
+        _wanted users expl 'login name' _ssh_users -S '' && ret=0
+      else
+        if (( $+opt_args[-l] )); then
+          tmp=()
+        else
+          tmp=( 'users:login name:_ssh_users -qS@' )
+        fi
+        _alternative \
+            'hosts:remote host name:_ssh_hosts' \
+            "$tmp[@]" && ret=0
+      fi
+      ;;
+  esac
+
+  return ret
+}
+
+_ssh_users () {
+  _combination -s '[:@]' my-accounts users-hosts users "$@"
+}
+
+_ssh_hosts () {
+  local -a config_hosts
+  local config
+  integer ind
+
+  # If users-hosts matches, we shouldn't complete anything else.
+  if [[ "$IPREFIX" == *@ ]]; then
+    _combination -s '[:@]' my-accounts users-hosts 
"users=${IPREFIX/@}" hosts "$@" && return
+  else
+    _combination -s '[:@]' my-accounts users-hosts \
+      ${opt_args[-l]:+"users=${opt_args[-l]:q}"} hosts "$@" && return
+  fi
+  if (( ind = ${words[(I)-F]} )); then
+    config=${~words[ind+1]} 2>/dev/null
+  else
+    config="$HOME/.ssh/config"
+  fi
+  if [[ -r $config ]]; then
+    local key hosts host
+    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"
+    if (( ${#config_hosts} )); then
+      _wanted hosts expl 'remote host name' \
+        compadd -M 'm:{a-zA-Z}={A-Za-z} r:|.=* r:|=*' "$@" 
$config_hosts
+    fi
+  fi
+}
+
+_mosh "$@"

Re: PATCH: Improve Mosh completion

[Daniel Shahaf]

Thanks for the ping on github.

Andrei Shevchuk wrote on Fri, 10 Nov 2017 03:35 +0300:
> This adds ~/.ssh/config based completion support (_ssh_hosts and 
> _ssh_users functions and userhost state handling taken without 
> modifications from ssh completion file),

Sorry, no.  https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

If you really need to call those two functions, break them out of _ssh into
separate files:

    % head -n1 Completion/*/Type/_ssh_files
    #autoload
    % 

Or — depending on mosh's syntax — it might be better to simply invoke _ssh
(directly or via _normal or one of its friends).

> existing code was just wrapped into _mosh function without modifications
> (except userhost state handling mentioned above).

Please don't mix whitespace and functional changes: that makes the diff harder
to review.

> +++ b/Completion/Unix/Command/_mosh
> @@ -1,26 +1,84 @@
> +    '--port=[specify server-side port range]:port:_sequence -n 2 -s \: 
> _ports' \

As you can see, your MUA mangled the patch (hard line break).  You can avoid
this by sending patches as attachments named *.txt.

Let's continue the discussion on list; there are more eyes here.

Cheers,

Daniel

Re: PATCH: Improve Mosh completion

[Bart Schaefer]

Daniel suggested continuing discussion of this on zsh-workers,
but no such discussion ensued. (workers/ 42000, 42167)

Re: PATCH: Improve Mosh completion

[Oliver Kiddle]

On 10 Nov, Andrei Shevchuk wrote:
> This adds ~/.ssh/config based completion support (_ssh_hosts and
> _ssh_users functions and userhost state handling taken without 
> modifications from ssh completion file), existing code was just wrapped 
> into _mosh function without modifications (except userhost state 
> handling mentioned above).

Daniel and I commented here and on github at the time that it'd be
nicer to factor _ssh_hosts out than to duplicate it. In fact _git also
makes use of _ssh_hosts. So this is a fresh patch based on the original.
It also made no sense to check $opt_args[-l] for mosh as mosh doesn't
have a -l option.

I'm not so sure about _ssh_users. I've expanded it inline in _mosh
here. I've never been entirely happy with the way _combination works
for one thing. For ssh, it is mostly sensible to configure usernames in
.ssh/config anyway so usernames are only relevant when using more than
one on a remote machine. The users-hosts style also disables completion of
hosts from .ssh/config.

Given an ssh configuration of:
  ControlPath /tmp/ssh-%r@%h:%p
It'd be useful to complete established connections conveniently,
especially for scp. So, I can try:
  zstyle -e ':completion:*:(scp|ssh):*:my-accounts' users-hosts \
      'reply=( /tmp/ssh-*@*:*(=N) ); reply=( ${${${reply:t}#*-}%:*} )'
That still breaks up username and hostname completion, requiring two
tabs but also blocks normal completion of hosts from .ssh/config

Oliver


diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 30ca80835..dccbcc67f 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -6897,10 +6897,6 @@ __git_tree_files () {
 
 (( $+functions[__git_remote_repositories] )) ||
 __git_remote_repositories () {
-  local service
-
-  service= _ssh
-
   if compset -P '*:'; then
     _remote_files -/ -- ssh
   else
diff --git a/Completion/Unix/Command/_mosh b/Completion/Unix/Command/_mosh
index 431fdbf9e..aa97587bb 100644
--- a/Completion/Unix/Command/_mosh
+++ b/Completion/Unix/Command/_mosh
@@ -1,6 +1,6 @@
 #compdef mosh
 
-local curcontext="$curcontext" state line
+local curcontext="$curcontext" state line expl ret=1
 local -a _comp_priv_prefix
 
 _arguments -C \
@@ -15,12 +15,20 @@ _arguments -C \
   '--server[specify command to run server helper]:remote file:_files' \
   '--client[specify command to run client helper]:_command_names -e' \
   '1:remote host name:->userhost' \
-  '*:::args:_normal' && return
+  '*:::args:_normal' && ret=0
 
 case $state in
   userhost)
-    _hosts || _user_at_host && return
+    if compset -P '*@'; then
+      _wanted hosts expl 'remote host name' _ssh_hosts && ret=0
+    elif compset -S '@*'; then
+      _wanted users expl 'login name' _combination -s '[:@]' my-accounts users-hosts users -S '' && ret=0
+    else
+      _alternative \
+	'hosts:remote host name:_ssh_hosts' \
+	'users:login name:_combination -s "[:@]" my-accounts users-hosts users -qS@' && ret=0
+    fi
   ;;
 esac
 
-return 1
+return ret
diff --git a/Completion/Unix/Command/_ssh b/Completion/Unix/Command/_ssh
index 8a122a5b3..20a5536ee 100644
--- a/Completion/Unix/Command/_ssh
+++ b/Completion/Unix/Command/_ssh
@@ -647,46 +647,4 @@ _ssh_users () {
   _combination -s '[:@]' my-accounts users-hosts users "$@"
 }
 
-_ssh_hosts () {
-  local -a config_hosts
-  local config
-  integer ind
-
-  # If users-hosts matches, we shouldn't complete anything else.
-  if [[ "$IPREFIX" == *@ ]]; then
-    _combination -s '[:@]' my-accounts users-hosts "users=${IPREFIX/@}" hosts "$@" && return
-  else
-    _combination -s '[:@]' my-accounts users-hosts \
-      ${opt_args[-l]:+"users=${opt_args[-l]:q}"} hosts "$@" && return
-  fi
-  if (( ind = ${words[(I)-F]} )); then
-    config=${~words[ind+1]} 2>/dev/null
-  else
-    config="$HOME/.ssh/config"
-  fi
-  if [[ -r $config ]]; then
-    local key line host
-    local -a lines=("${(@f)$(<"$config")}") 2>/dev/null
-    while (($#lines)); do
-      IFS=$'=\t ' read -r key line <<<"${lines[1]}"
-      case "$key" in
-      ((#i)include)
-        lines[1]=("${(@f)$(cd $HOME/.ssh; cat ${(z)~line})}") 2>/dev/null;;
-      ((#i)host(|name))
-        for host in ${(z)line}; do
-          case $host in
-          (*[*?]*) ;;
-          (*) config_hosts+=("$host") ;;
-          esac
-        done ;&
-      (*) shift lines;;
-      esac
-    done
-    if (( ${#config_hosts} )); then
-      _wanted hosts expl 'remote host name' \
-        compadd -M 'm:{a-zA-Z}={A-Za-z} r:|.=* r:|=*' "$@" $config_hosts
-    fi
-  fi
-}
-
 _ssh "$@"
diff --git a/Completion/Unix/Type/_ssh_hosts b/Completion/Unix/Type/_ssh_hosts
new file mode 100644
index 000000000..e20142cfd
--- /dev/null
+++ b/Completion/Unix/Type/_ssh_hosts
@@ -0,0 +1,41 @@
+#autoload
+
+local -a config_hosts
+local config
+integer ind
+
+# If users-hosts matches, we shouldn't complete anything else.
+if [[ "$IPREFIX" == *@ ]]; then
+  _combination -s '[:@]' my-accounts users-hosts "users=${IPREFIX/@}" hosts "$@" && return
+else
+  _combination -s '[:@]' my-accounts users-hosts \
+    ${opt_args[-l]:+"users=${opt_args[-l]:q}"} hosts "$@" && return
+fi
+if (( ind = ${words[(I)-F]} )); then
+  config=${~words[ind+1]} 2>/dev/null
+else
+  config="$HOME/.ssh/config"
+fi
+if [[ -r $config ]]; then
+  local key line host
+  local -a lines=("${(@f)$(<"$config")}") 2>/dev/null
+  while (($#lines)); do
+    IFS=$'=\t ' read -r key line <<<"${lines[1]}"
+    case "$key" in
+    ((#i)include)
+      lines[1]=("${(@f)$(cd $HOME/.ssh; cat ${(z)~line})}") 2>/dev/null;;
+    ((#i)host(|name))
+      for host in ${(z)line}; do
+	case $host in
+	(*[*?]*) ;;
+	(*) config_hosts+=("$host") ;;
+	esac
+      done ;&
+    (*) shift lines;;
+    esac
+  done
+  if (( ${#config_hosts} )); then
+    _wanted hosts expl 'remote host name' \
+      compadd -M 'm:{a-zA-Z}={A-Za-z} r:|.=* r:|=*' "$@" $config_hosts
+  fi
+fi