adding '-p precmd' to _normal

adding '-p precmd' to _normal

[Jun T]

In workers/52037⁩, I replaced '_normal' by '_normal -p env' 
in _env (and similarly in _watch), so that only external
command (and its args) are completed.

But I've noticed there are many (about 15?) completion
functions that should complete only external commands
and may require the similar change (adding -p precmd).
Examples are _chroot, _chrt, _screen, etc. 

In some other completion functions (_dchroot, _truss, etc.)
'_command_name -e' is used for completing external
command and '_normal' is used for args.

If anyone knows any reason that this two-step method is
better than '_normal -p precmd' then please let me know.

Otherwise I will use '_normal -p precmd' in _chroot etc.

Re: adding '-p precmd' to _normal

[dana]

On Mon 28 Aug 2023, at 22:55, Jun T wrote:
> If anyone knows any reason that this two-step method is
> better than '_normal -p precmd' then please let me know.

I don't think there's a reason to break them out like this any more,
usually, thanks to workers/44200 and workers/44201. Matthew updated
several existing users of _normal so that they wouldn't have to, but
must have missed the ones you mentioned, or wasn't confident about
changing them for some reason

Related: It's not really explained *why* using _normal with -p has this
benefit. Attached patch addresses that

PS: Currently it doesn't matter that much because we usually don't care
about exactly which commands are in the pre-commands list, just whether
there are any in there, but technically it would be more robust to use
`-p $service` than to hard-code it like `-p env`

dana


diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 33baeab49..5035097bb 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -4389,8 +4389,10 @@ the functions for the fields if they are called.
 findex(_command_names)
 item(tt(_command_names) [ tt(-e) | tt(-) ])(
 This function completes words that are valid at command position: names of
-aliases, builtins, hashed commands, functions, and so on.  With the tt(-e)
-flag, only hashed commands are completed.  The tt(-) flag is ignored.
+aliases, builtins, hashed commands, functions, and so on.  If the tt(-e)
+flag is given, or if the list of precommands contains a non-builtin command
+(e.g. because tt(_normal -p) has been used previously), only hashed commands
+are completed.  The tt(-) flag is ignored.
 )
 findex(_comp_locale)
 item(tt(_comp_locale))(
@@ -4776,7 +4778,11 @@ functions) regardless of prior precommands (e.g. `tt(zsh -c)').
 )
 item(tt(-p) var(precommand))(
 Append var(precommand) to the list of precommands. This option should be
-used in nearly all cases in which tt(-P) is not applicable.
+used in nearly all cases in which tt(-P) is not applicable. An advantage
+of using this option is that it can automatically signal to
+tt(_command_names) that subsequent completion should be limited to hashed
+commands, which is usually the desired behaviour following commands like
+tt(chroot) and tt(nohup).
 )
 enditem()
 

Re: adding '-p precmd' to _normal

[Jun. T]

> 2023/08/30 9:25, dana <dana@dana.is> dana:
> 
> On Mon 28 Aug 2023, at 22:55, Jun T wrote:
>> If anyone knows any reason that this two-step method is
>> better than '_normal -p precmd' then please let me know.
> 
> I don't think there's a reason to break them out like this any more,
> usually, thanks to workers/44200 and workers/44201.

Thanks. It seems I didn't read the thread carefully.

> Matthew updated (...) but must have missed the ones you mentioned, 

I think he has updated only those that had 'precommands+=( precmd )'.

Below are the files that I think '-p $service' needs be added.

# _cmdstring still offers alias etc. Should we fix this also?

_nice also uses _normal but it has some other problems; I will
send a patch for it later.


 Completion/BSD/Command/_jexec      | 2 +-
 Completion/Linux/Command/_chrt     | 2 +-
 Completion/Linux/Command/_cpupower | 2 +-
 Completion/Linux/Command/_ionice   | 2 +-
 Completion/Linux/Command/_setpriv  | 2 +-
 Completion/Linux/Command/_sysstat  | 2 +-
 Completion/Unix/Command/_chroot    | 2 +-
 Completion/Unix/Command/_mosh      | 4 ++--
 Completion/Unix/Command/_route     | 2 +-
 Completion/Unix/Command/_screen    | 2 +-
 Completion/Unix/Command/_script    | 2 +-
 Completion/Unix/Command/_ssh       | 4 ++--
 Completion/Unix/Command/_stdbuf    | 2 +-
 Completion/Unix/Command/_timeout   | 2 +-
 14 files changed, 16 insertions(+), 16 deletions(-)


diff --git a/Completion/BSD/Command/_jexec b/Completion/BSD/Command/_jexec
index 6a2d05a81..cd99ebe91 100644
--- a/Completion/BSD/Command/_jexec
+++ b/Completion/BSD/Command/_jexec
@@ -6,7 +6,7 @@ _jexec_normal() {
   # relative paths are relative to the jail's root
   path=( "$(_call_program paths jls -j $words[1] path)"/$^path )
   shift 1 words; (( CURRENT-- ))
-  _normal
+  _normal -p $service
 }
 
 _jexec() {
diff --git a/Completion/Linux/Command/_chrt b/Completion/Linux/Command/_chrt
index 6789b66cf..5431b0799 100644
--- a/Completion/Linux/Command/_chrt
+++ b/Completion/Linux/Command/_chrt
@@ -62,7 +62,7 @@ elif (( CURRENT == 1 )); then
 else
   shift words
   (( CURRENT-- ))
-  _normal && ret=0
+  _normal -p $service && ret=0
 fi
 
 return ret
diff --git a/Completion/Linux/Command/_cpupower b/Completion/Linux/Command/_cpupower
index 6763bdd12..d342b69d9 100644
--- a/Completion/Linux/Command/_cpupower
+++ b/Completion/Linux/Command/_cpupower
@@ -95,7 +95,7 @@ case $state in
           '-i+[measurement interval]:interval (seconds)'
           '-c[schedule on every core]'
           '-v[increase verbosity]'
-          '*:::command: _normal'
+          '*:::command: _normal -p $service'
         )
       ;;
     esac
diff --git a/Completion/Linux/Command/_ionice b/Completion/Linux/Command/_ionice
index ba403ca56..9989cd6a9 100644
--- a/Completion/Linux/Command/_ionice
+++ b/Completion/Linux/Command/_ionice
@@ -28,7 +28,7 @@ if [[ -n $state ]]; then
   elif (( $+opt_args[args--u] || $+opt_args[args---uid] )); then
     _message -e uids 'user id'
   else
-    _normal && ret=0
+    _normal -p $service && ret=0
   fi
 fi
 
diff --git a/Completion/Linux/Command/_setpriv b/Completion/Linux/Command/_setpriv
index 196f2f627..9e38152b9 100644
--- a/Completion/Linux/Command/_setpriv
+++ b/Completion/Linux/Command/_setpriv
@@ -96,7 +96,7 @@ _arguments -C -S -s \
   '--selinux-label[request a selinux label]:SELinux labels: ' \
   '--apparmor-profile[request an apparmor profile]:AppArmor profiles: ' \
   '--reset-env[set environment as for a classic login shell]' \
-  '*:::command:_normal' \
+  '*:::command: _normal -p $service' \
   && return 0
 
 case $state in
diff --git a/Completion/Linux/Command/_sysstat b/Completion/Linux/Command/_sysstat
index 5620da73d..0baae0764 100644
--- a/Completion/Linux/Command/_sysstat
+++ b/Completion/Linux/Command/_sysstat
@@ -130,7 +130,7 @@ _pidstat() {
   _arguments -s : \
     '-C[filter tasks by string]:task filter' \
     '-d[report I/O statistics]' \
-    '-e[execute specified program and monitor it with pidstat]:*::command: _normal' \
+    '-e[execute specified program and monitor it with pidstat]:*::command: _normal -p $service' \
     '-H[display timestamp in seconds since the epoch]' \
     '-h[display horizontally]' \
     '-I[divide CPU usage by number of processors]' \
diff --git a/Completion/Unix/Command/_chroot b/Completion/Unix/Command/_chroot
index 516992694..a9c577bd7 100644
--- a/Completion/Unix/Command/_chroot
+++ b/Completion/Unix/Command/_chroot
@@ -33,7 +33,7 @@ case $variant in
     ;;
 esac
 
-args+=( '1:new root directory:_directories' '*:::command:_normal' )
+args+=( '1:new root directory:_directories' '*::: : _normal -p $service' )
 
 _arguments -s -S : $args && ret=0
 
diff --git a/Completion/Unix/Command/_mosh b/Completion/Unix/Command/_mosh
index 7d1250320..6d0f746f8 100644
--- a/Completion/Unix/Command/_mosh
+++ b/Completion/Unix/Command/_mosh
@@ -7,7 +7,7 @@ _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' \
+  '--ssh=[specify ssh command to setup session]:ssh command: _cmdstring' \
   '--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]' \
@@ -22,7 +22,7 @@ _arguments -C \
   '--local[run mosh-server locally without using ssh]' \
   '--experimental-remote-ip=[select method for discovering remote IP address to use for mosh]:method:(local remote proxy)' \
   '1:remote host name:->userhost' \
-  '*:::args:_normal' && ret=0
+  '*::: : _normal -p $service' && ret=0
 
 case $state in
   userhost)
diff --git a/Completion/Unix/Command/_route b/Completion/Unix/Command/_route
index 95df6d936..f0775a5d2 100644
--- a/Completion/Unix/Command/_route
+++ b/Completion/Unix/Command/_route
@@ -196,7 +196,7 @@ if [[ -n $state ]]; then
   if [[ $line[1] = exec ]]; then
     shift words
     (( CURRENT-- ))
-    _normal
+    _normal -p $service && return
   elif [[ $line[1] = (flush|monitor) ]]; then
     sequential=()
   fi
diff --git a/Completion/Unix/Command/_screen b/Completion/Unix/Command/_screen
index 6d47d2638..9336ae82d 100644
--- a/Completion/Unix/Command/_screen
+++ b/Completion/Unix/Command/_screen
@@ -107,7 +107,7 @@ if [[ -n $state ]]; then
       elif (( CURRENT > 2 )) && [[ ${words[1]} == /dev/* ]]; then
 	  _message "no more parameters"
       else
-	  _normal
+	  _normal -p $service
       fi
     ;;
     attached-sessions)
diff --git a/Completion/Unix/Command/_script b/Completion/Unix/Command/_script
index 7a3960be0..f39cfe535 100644
--- a/Completion/Unix/Command/_script
+++ b/Completion/Unix/Command/_script
@@ -68,7 +68,7 @@ case $OSTYPE in
       '-F[send output to specified named pipe]:fifo:_files -g "*(p)"'
       '-t+[specify interval of data flushing]:interval (seconds)'
       '-k[log keys sent to the program as well as output]'
-      '*:::arguments: _normal'
+      '*:::arguments: _normal $service'
     )
   ;|
   darwin*|freebsd*)
diff --git a/Completion/Unix/Command/_ssh b/Completion/Unix/Command/_ssh
index fd2a90b59..0ba1f3775 100644
--- a/Completion/Unix/Command/_ssh
+++ b/Completion/Unix/Command/_ssh
@@ -136,7 +136,7 @@ _ssh () {
       '(-k -c)-s[force sh-style shell]' \
       '-t+[set default maximum lifetime for identities]: :_numbers -u seconds "maximum lifetime" \:s\:seconds m\:minutes h\:hours d\:days w\:weeks' \
       '-v[verbose mode]' \
-      '*::command: _normal'
+      '*::command: _normal -p $service'
     return
     ;;
   ssh-keygen)
@@ -738,7 +738,7 @@ _ssh () {
       local -a _comp_priv_prefix
       shift 1 words
       (( CURRENT-- ))
-      _normal
+      _normal -p $service
       return
       ;;
     destinations)
diff --git a/Completion/Unix/Command/_stdbuf b/Completion/Unix/Command/_stdbuf
index 4b7d98ba0..32b3cae2f 100644
--- a/Completion/Unix/Command/_stdbuf
+++ b/Completion/Unix/Command/_stdbuf
@@ -27,6 +27,6 @@ for ((i=1;i<=3;i++)); do
     "(${short[i]})${long[i]}=${(e)opt}"
   )
 done
-(( CURRENT > 2 )) && args+=( '*::command:_normal' )
+(( CURRENT > 2 )) && args+=( '*::command: _normal -p $service' )
 
 _arguments -s -S $args
diff --git a/Completion/Unix/Command/_timeout b/Completion/Unix/Command/_timeout
index c041283ac..9c7f1a004 100644
--- a/Completion/Unix/Command/_timeout
+++ b/Completion/Unix/Command/_timeout
@@ -17,4 +17,4 @@ _arguments -S -A "-" $args \
   '(-s --signal)'{-s,--signal}'[specify the signal to send on timeout]:signal:_signals' \
   '(-k --kill-after)'{-k,--kill-after}'[followup first signal with SIGKILL if command persists after specified time]:time' \
   '1: :_numbers -f -u seconds duration :s:seconds m:minutes h:hours d:days' \
-  '*:::command:_normal'
+  '*:::command: _normal -p $service'

Re: adding '-p precmd' to _normal

[Jun. T]

> 2023/08/30 9:25, dana <dana@dana.is> wrote:
> 
> diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo

Could you please push this by yourself?