Subject: [PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.
From: Daniel Shahaf <[email-masked]>
To: Oliver Kiddle <[email-masked]>
Cc: [email-masked]
Date: Wed, 20 Jul 2016 06:54:21 +0000
List-Id: Zsh Workers List <zsh-workers.zsh.org>

Bart Schaefer wrote on Sun, Jul 17, 2016 at 13:40:41 -0700:
> _add-zle-hook-widget_classes() {
>   local -a hooktypes=( ${=${(f)"$( add-zle-hook-widget -h )"}[3]} )
>   compadd "$@" -M 'L:|zle-=' -a hooktypes
> }

I'd rather not parse help output; the updated patch invokes -h but then
uses 'zstyle -g'.

Bart Schaefer wrote on Sun, Jul 17, 2016 at 14:57:18 -0700:
> On Jul 17,  3:00pm, Daniel Shahaf wrote:
> }
> } +    _wanted widgets expl widget compadd -M 'r:|-=* r:|=*' -k widgets && ret=0 && return 0
> 
> Better might be
> 
>   local -a user_widgets=( ${(k)widgets[(R)user:*]} )
>   _wanted widgets expl widget compadd -M 'r:|-=* r:|=*' -a user_widgets && ret=0 && return 0

Agreed, done.

Oliver Kiddle wrote on Mon, Jul 18, 2016 at 11:47:57 +0200:
> Daniel Shahaf wrote:
> > +local context state state_descr line
> > +typeset -A opt_args
> 
> Those are only needed if you're using _values or _arguments with states.
> This function doesn't so that isn't needed.

I do use $opt_args.  Not sure whether I can remove the others (obviously
keeping them isn't wrong).

[I've moved the 'local' call, can drop it in the next iteration if needed.]

> > +  else
> > +    _wanted widgets expl widget compadd -M 'r:|-=* r:|=*' -k widgets && ret=0 && return 0

Should both _wanted calls specify the same tag?  (This code is based on
_add-zsh-hook so any change may want to be ported thereto.)

> > +  fi
> > +  return 1
[...]
> Actually, all this messing with return codes is superfluous. Given an
> if..then..else block, the last command in either branch determines the
> final return status. If you have '&& return 0', the 0 is always
> redundant. But in this case, no return statement at all should be needed.

This is just a coding style question; there are arguments for both
sides.  What's the house style for completion functions, to have
explicit 'return' statements or not to have them?

I'll make the v3 iteration use the house style, whatever it is.

> Also, Bart suggested:
> >  compadd "$@" -M 'L:|zle-=' -a hooktypes
> 
> That L: form makes the zle- a sort of optional prefix. Is that what
> you intended? Unfortunately it doesn't work together with -M 'r:|-=*
> r:|=*' and zl<tab> won't complete. 'B:zle-=' is a shorter version
> of the same thing. It might be better to do:
>   compadd "$@" -a hooktypes || compadd "$@" -pzle- -a hooktypes

The updated patch uses:
.
  compadd "$@" -M 'L:|=zle-' -M 'r:|-=* r:|=*' -a -- zle-${^hooktypes}
.
which completes both k-s<TAB> and z-k-s<TAB> to (zle-|)keymap-select.
I'm open to suggestions here, matchspecs aren't my forté.

Updated patch below.

Cheers,

Daniel

diff --git a/Completion/Zsh/Function/_add-zle-hook-widget b/Completion/Zsh/Function/_add-zle-hook-widget
new file mode 100644
index 0000000..da65dd6
--- /dev/null
+++ b/Completion/Zsh/Function/_add-zle-hook-widget
@@ -0,0 +1,36 @@
+#compdef add-zle-hook-widget
+
+_add-zle-hook-widget_types() {
+  local -a tmp
+
+  autoload -U add-zle-hook-widget
+  add-zle-hook-widget -h > /dev/null # sets the zstyle
+  zstyle -g tmp zle-hook types
+
+  compadd "$@" -M 'L:|=zle-' -M 'r:|-=* r:|=*' -- zle-${^tmp}
+}
+
+_add-zle-hook-widget_widgets() {
+  if (( $+opt_args[-d] )); then
+    local -a tmp
+    zstyle -g tmp $line[1] widgets
+    _wanted widgets expl "installed hook" compadd -- ${tmp#<->:} && return 0
+  else
+    local -a user_widgets=( ${(k)widgets[(R)user:*]} )
+    _wanted widgets expl widget compadd -M 'r:|-=* r:|=*' -a user_widgets && return 0
+  fi
+  return 1
+}
+
+_add-zle-hook-widget() {
+  local context state state_descr line
+  typeset -A opt_args
+  _arguments -s -w -S : \
+    '(-D)-d[remove HOOK from the array]' \
+    '(-d)-D[interpret HOOK as pattern to remove from the array]' \
+    {-U,-z,-k}"[passed to 'autoload']" \
+    ':hook type:_add-zle-hook-widget_types' \
+    ':widget:_add-zle-hook-widget_widgets'
+}
+
+_add-zle-hook-widget "$@"
diff --git a/Completion/Zsh/Function/_add-zsh-hook b/Completion/Zsh/Function/_add-zsh-hook
index c70a497..fb5403a 100644
--- a/Completion/Zsh/Function/_add-zsh-hook
+++ b/Completion/Zsh/Function/_add-zsh-hook
@@ -1,8 +1,5 @@
 #compdef add-zsh-hook
 
-local context state state_descr line
-typeset -A opt_args
-
 _add-zsh-hook_hooks() {
   if (( $+opt_args[-d] )); then
     _wanted functions expl "installed hooks" compadd -a - "$line[1]_functions" && return 0
@@ -13,6 +10,8 @@ _add-zsh-hook_hooks() {
 }
 
 _add-zsh-hook() {
+  local context state state_descr line
+  typeset -A opt_args
   _arguments -s -w -S : \
     '(-D)-d[remove HOOK from the array]' \
     '(-d)-D[interpret HOOK as pattern to remove from the array]' \