Next release (5.3)

Next release (5.3)

[Peter Stephenson]

I should produce a new release of zsh soon.

It looks like there are a couple of things that need to settle down,
after which I'll make a test release --- don't think there are any
changes likely to be problematic.

pws

Re: Next release (5.3)

[Bart Schaefer]

On Jul 4, 11:40am, Peter Stephenson wrote:
}
} It looks like there are a couple of things that need to settle down,

I know of (in no particular order):

(1) Daniel's suggested change to :A [care to offer an opinion?]

(2) add-zle-hook-widget [I'd be fine with omitting that entirely]

(3) Doc for select-bracketed / select-quoted, and maybe C-code them

(4) Sebastian's mysterious slowdown on the first run of a nested
    pair of autoload functions

What else?

Re: Next release (5.3)

[Sebastian Gniazdowski]

On 4 July 2016 at 17:04, Bart Schaefer <schaefer@brasslantern.com> wrote:
> (4) Sebastian's mysterious slowdown on the first run of a nested
>     pair of autoload functions

Isn't this settled? Large data passed to positional parameters uses
heap, high heap usage causes slowness. Double call with the same large
positional parameters – twice as much heap usage, and more slow down.
That's why the trick "repeat 1; do ... done" helped less, but still
helped.

Best regards,
Sebastian Gniazdowski

Re: Next release (5.3)

[Bart Schaefer]

On Jul 5,  8:00am, Sebastian Gniazdowski wrote:
} Subject: Re: Next release (5.3)
}
} On 4 July 2016 at 17:04, Bart Schaefer <schaefer@brasslantern.com> wrote:
} > (4) Sebastian's mysterious slowdown on the first run of a nested
} >     pair of autoload functions
} 
} Isn't this settled? Large data passed to positional parameters uses
} heap, high heap usage causes slowness. Double call with the same large
} positional parameters - twice as much heap usage, and more slow down.

That doesn't explain why it happens for the inner autoload *every time
around the loop* in the outer one.  Once loaded, the second and later
calls should be faster.

(Unless it's simply the case that the heap from making the call to the
outer function is so large that it's affecting everything downstream.)

Re: Next release (5.3)

[Peter Stephenson]

On Mon, 04 Jul 2016 08:04:24 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> (1) Daniel's suggested change to :A [care to offer an opinion?]

I'd be vaguely inclined to make sure it does what the doc currently says
and leave it at that.  But that's only because I've not worked out a
case where I want anything different.  It's too difficult to come up
with a categorical answer because it depends whether the user is used to
CHASE_BLAH behaviour (I'm not suggesting option-specific behaviour,
either).

pws

Re: Next release (5.3)

[Peter Stephenson]

On Tue, 5 Jul 2016 09:33:21 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Mon, 04 Jul 2016 08:04:24 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > (1) Daniel's suggested change to :A [care to offer an opinion?]
> 
> I'd be vaguely inclined to make sure it does what the doc currently says
> and leave it at that.  But that's only because I've not worked out a
> case where I want anything different.  It's too difficult to come up
> with a categorical answer because it depends whether the user is used to
> CHASE_BLAH behaviour (I'm not suggesting option-specific behaviour,
> either).

Thinking out loud, don't take any notice...

It would be possible to add a different letter for this.  However, I
think you're still faced with asking what the user actually expects from
the paths they use, which comes back to the option settings.  So,
actually, option dependence, although apparently just giving you extra
ways of going wrong in a script or function, isn't necessarily *that*
stupid in practice.

pws

Re: Next release (5.3)

[Bart Schaefer]

On Jul 5, 12:48pm, Peter Stephenson wrote:
} Subject: Re: Next release (5.3)
}
} > > (1) Daniel's suggested change to :A [care to offer an opinion?]
} 
} Thinking out loud, don't take any notice...
} 
} It would be possible to add a different letter for this.  However, I
} think you're still faced with asking what the user actually expects
} from the paths they use, which comes back to the option settings.

That's actually sort of the point -- the option settings are to have
CHASE_DOTS unset by default, but :A behaves as if CHASE_DOTS is set,
so it's actually inverted from what the options indicate the user is
expecting.

Sure, we could add another modifier [maybe :L mneme "ls -L"?  Except
confusion with :l and with (L) which has always been an oddball] but
then you have the seemingly unnecessary case that :a:L is :A and that
:L:a may mean something entirely different.

Having written that, though, it would seem that if :L might still
leave ".." in the path (? is that what happens when the symlink is
itself ".."-relative?) then maybe :L is actually needed and even the
proposed change to the :A default is inadequate for all cases.

Re: Next release (5.3)

[Daniel Shahaf]

Bart Schaefer wrote on Mon, Jul 04, 2016 at 08:04:24 -0700:
> On Jul 4, 11:40am, Peter Stephenson wrote:
> }
> } It looks like there are a couple of things that need to settle down,
> 
> I know of (in no particular order):
> 
> (1) Daniel's suggested change to :A [care to offer an opinion?]

This needn't block the release, I'm perfectly happy to let it slide
to 5.4.

> (2) add-zle-hook-widget [I'd be fine with omitting that entirely]
> 

Well, I'd like to use it in z-sy-h.  (I'd like to use zle-line-pre-redraw
in z-sy-h, because that would solve multiple bugs; and I'd like to do so
through add-zle-hook-widget for interoperability with other plugins.)

How about including add-zle-hook-widget in 5.3, but without any indices
or before:/after: support, just the basic 'add-zle-hook-widget
$hook_name $widget_name' syntax?  (And have it just invoke the widgets
in the order they were registered)  That would be an improvement over
the 5.2 status quo, and we can add indices or --option arguments in 5.4.

Cheers,

Daniel

add-zle-hook-widget Re: Next release (5.3)

[Daniel Shahaf]

Daniel Shahaf wrote on Thu, Jul 07, 2016 at 02:00:21 +0000:
> Bart Schaefer wrote on Mon, Jul 04, 2016 at 08:04:24 -0700:
> > (2) add-zle-hook-widget [I'd be fine with omitting that entirely]
> > 
> 
> Well, I'd like to use it in z-sy-h.  (I'd like to use zle-line-pre-redraw
> in z-sy-h, because that would solve multiple bugs; and I'd like to do so
> through add-zle-hook-widget for interoperability with other plugins.)
> 
> How about including add-zle-hook-widget in 5.3, but without any indices
> or before:/after: support, just the basic 'add-zle-hook-widget
> $hook_name $widget_name' syntax?  (And have it just invoke the widgets
> in the order they were registered)  That would be an improvement over
> the 5.2 status quo, and we can add indices or --option arguments in 5.4.

I've heard neither ayes nor nays, so here's a patch we can use if we
choose this approach.

The ifzman() thing at the beginning is to fix a preëxisting issue
where the "rather than..." line is rendered in bold.  (I tried making
the example() macro generate \fB/\fP around the .RS/.RE but that didn't
help.)

Shall we go for this patch then, or something else (what?)?

Daniel


diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo
index d4a4538..2e4ba9a 100644
--- a/Doc/Zsh/contrib.yo
+++ b/Doc/Zsh/contrib.yo
@@ -345,9 +345,15 @@ tt(zle-isearch-exit), etc.
 var(widgetname) is the name of a ZLE widget.  If no options are given this
 is added to the array of widgets to be invoked in the given hook context.
 Note that the hooks are called as widgets, that is, with
-example(tt(zle )var(widgetname)tt( -Nw "$@"))
+example(tt(zle )var(widgetname)tt( -Nw "$@"))ifzman(NOTRANS(\fR))
 rather than as a function call.
 
+COMMENT(
+The implementation supports specifying a var(widgetname) of the form
+var(index)tt(:)var(name), however, that is an implementation detail that third
+party scripts may not rely on.  See various threads in June/July 2016.  The
+original docs of that feature are retained herein for reference:
+
 In typical usage, var(widgetname) has the form var(index)tt(:)var(name).
 In this case var(index) is an integer which determines the order in which
 the widget var(name) will be called relative to other widgets in the
@@ -355,6 +361,7 @@ array.  Widgets having the same var(index) are called in unspecified
 order.  However, var(widgetname) may omit the index, in which case an
 index is computed for it to arrange for it to be called in the order
 in which it was added to the array.
+)
 
 If the option tt(-d) is given, the var(widgename) is removed from
 the array of widgets to be executed.

Cheers,

Daniel

Re: add-zle-hook-widget Re: Next release (5.3)

[Bart Schaefer]

On Jul 13,  5:00am, Daniel Shahaf wrote:
} Subject: add-zle-hook-widget Re: Next release (5.3)
}
} Daniel Shahaf wrote on Thu, Jul 07, 2016 at 02:00:21 +0000:
} > How about including add-zle-hook-widget in 5.3, but without any indices
} > or before:/after: support
} 
} I've heard neither ayes nor nays, so here's a patch we can use if we
} choose this approach.
} 
} Shall we go for this patch then, or something else (what?)?

Other things have required my attention.

Here's a patch to the add-zle-hook-widget function which deletes the
reference to INDEX:NAME syntax and its implementation in the argument
list.  It's still visible in the stored record if you pass the -L
option just so I didn't have to rewrite too much of the rest of the
code and because a future patch might revive it.

This also handles some edge cases, like, what happens if you do

    autoload add-zle-hook-widget
    add-zle-hook-widget zle-line-init some-new-widget
    zle -N zle-line-finish my-line-finish
    add-zle-hook-widget zle-line-finish some-other-widget

Previously the second a-z-h-w would have been ignored because the check
for existing user widgets was only done at the first call.  Now it
checks on each call to see if the user has stepped on it and tries to
do the right thing.

I've also tried to fix the kshautoload handling.  Note that if you're
in the habit of doing goofy stuff like

	unsetopt kshautoload
	autoload -k +X somefunc
	somefunc

then you get what you deserve, i.e., somefunc will run twice the first
time you use it.  In the case of add-zsh-hook-widget, that's harmless.

The contrib.yo file needs to be updated, I wanted first to see whether
anyone spots any obvious problems with this rendition of the function.


diff --git a/Functions/Zle/add-zle-hook-widget b/Functions/Zle/add-zle-hook-widget
index 608a776..760e26d 100644
--- a/Functions/Zle/add-zle-hook-widget
+++ b/Functions/Zle/add-zle-hook-widget
@@ -1,18 +1,15 @@
 # Add to HOOK the given WIDGET
 # 
 # HOOK is one of isearch-exit, isearch-update, line-pre-redraw, line-init,
-# line-finish, history-line-set, keymap-select (the zle- prefix is not
-# required).
+# line-finish, history-line-set, keymap-select (the zle- prefix is allowed
+# but not required).  If a widget corresponding to HOOK already exists, it
+# is preserved and called first in the new set of HOOK widgets.
 #
-# WIDGET may be of the form INDEX:NAME in which case the INDEX determines
-# the order in which the widget executes relative to other hook widgets.
-# Othewise the widget is assigned an index that appends it to the array.
-# 
-# With -d, remove the widget from the hook instead; delete the hook
-# variable if it is empty.
+# With -d, remove the WIDGET from the hook instead; deletes the hook
+# linkage if it is empty.
 #
-# -D behaves like -d, but pattern characters are active in the
-# widget name, so any matching widget will be deleted from the hook.
+# -D behaves like -d, but pattern characters are active in WIDGET, so
+# any matching widget will be deleted from the hook.
 #
 # Without -d, if the WIDGET is not already defined, a function having the
 # same name is marked for autoload; -U is passed down to autoload if that
@@ -47,16 +44,15 @@ do
       # and we run them in number order
       zstyle -a $WIDGET widgets hook_widgets
       for hook in "${(@)${(@on)hook_widgets[@]}#<->:}"; do
-	  zle "$hook" -Nw "$@" || return
+	  if [[ "$hook" = user:* ]]; then
+	      # Preserve $WIDGET within the renamed widget
+	      zle "$hook" -N "$@"
+	  else
+	      zle "$hook" -Nw "$@"
+	  fi || return
       done
       return 0
   }
-  # Check for an existing widget, add it as the first hook
-  if [[ ${widgets[$hook]} = user:* ]]; then
-      zle -A "$hook" "${widgets[$hook]}"
-      zstyle -- "$hook" widgets 0:"${widgets[$hook]}"
-      zle -N "$hook" azhw:"$hook"
-  fi
 done
 
 # Redefine ourself with the setup left out
@@ -133,17 +129,19 @@ function add-zle-hook-widget {
     else
 	integer i=${#options[ksharrays]}-2
 	zstyle -g extant_hooks "$hook" widgets
-	if [[ "$fn" != <->:* ]]; then
-	    if [[ -z ${(M)extant_hooks[@]:#(<->:|)$fn} ]]; then
-	        # no index and not already hooked
-		# assign largest existing index plus 10
-		i=${${(On@)${(@M)extant_hooks[@]#<->:}%:}[i]}+10
-	    else
-		return 0
-	    fi
+        # Check for an existing widget, add it as the first hook
+	if [[ ${widgets[$hook]} != "user:azhw:$hook" ]]; then
+	    zle -A "$hook" "${widgets[$hook]}"
+	    extant_hooks=(0:"${widgets[$hook]}" "${extant_hooks[@]}")
+	    zle -N "$hook" azhw:"$hook"
+	fi
+	# Add new widget only if not already in the hook list
+	if [[ -z ${(M)extant_hooks[@]:#(<->:|)$fn} ]]; then
+       	    # no index and not already hooked
+            # assign largest existing index plus 1
+	    i=${${(On@)${(@M)extant_hooks[@]#<->:}%:}[i]}+1
 	else
-	    i=${${(M)fn#<->:}%:}
-	    fn=${fn#<->:}
+	    return 0
 	fi
 	extant_hooks+=("${i}:${fn}")
 	zstyle -- "$hook" widgets "${extant_hooks[@]}"
@@ -157,7 +155,17 @@ function add-zle-hook-widget {
     fi
 }
 
-# Handle zsh autoloading conventions
-if [[ "$zsh_eval_context" = *loadautofunc && ! -o kshautoload ]]; then
-    add-zle-hook-widget "$@"
-fi
+# Handle zsh autoloading conventions:
+# - "file" appears last in zsh_eval_context when "source"-ing
+# - "evalautofunc" appears with kshautoload set or autoload -k
+# - "loadautofunc" appears with kshautoload unset or autoload -z
+# - use of autoload +X cannot reliably be detected, use best guess
+case "$zsh_eval_context" in
+*file) ;;
+*evalautofunc) ;;
+*loadautofunc) add-zle-hook-widget "$@";;
+*) [[ -o kshautoload ]] || add-zle-hook-widget "$@";;
+esac
+# Note fallback here is equivalent to the usual best-guess used by
+# functions written for zsh before $zsh_eval_context was available
+# so this case-statement is backward-compatible.

Re: add-zle-hook-widget Re: Next release (5.3)

[Bart Schaefer]

On Jul 13,  5:11pm, Bart Schaefer wrote:
}
} The contrib.yo file needs to be updated, I wanted first to see whether
} anyone spots any obvious problems with this rendition of the function.

Nobody has commented, so ...

The patch below fixes a couple of edge cases, notably:
- the case where "bindkey" has created an undefined slot in $widgets
- attempting to add a special widget to its own hook list
- a bug when checking whether to alias an existing hookd widget

It also switches from defining the hooks functions in a loop to defining
them with multifuncdef syntax, and updates contrib.yo.


diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo
index d4a4538..1d2b7ca 100644
--- a/Doc/Zsh/contrib.yo
+++ b/Doc/Zsh/contrib.yo
@@ -340,23 +340,20 @@ those additional widgets.
 var(hook) is one of tt(isearch-exit), tt(isearch-update),
 tt(line-pre-redraw), tt(line-init), tt(line-finish), tt(history-line-set),
 or tt(keymap-select), corresponding to each of the special widgets
-tt(zle-isearch-exit), etc.
+tt(zle-isearch-exit), etc.  The special widget names are also accepted
+as the var(hook) argument.
 
 var(widgetname) is the name of a ZLE widget.  If no options are given this
 is added to the array of widgets to be invoked in the given hook context.
 Note that the hooks are called as widgets, that is, with
 example(tt(zle )var(widgetname)tt( -Nw "$@"))
-rather than as a function call.
 
-In typical usage, var(widgetname) has the form var(index)tt(:)var(name).
-In this case var(index) is an integer which determines the order in which
-the widget var(name) will be called relative to other widgets in the
-array.  Widgets having the same var(index) are called in unspecified
-order.  However, var(widgetname) may omit the index, in which case an
-index is computed for it to arrange for it to be called in the order
-in which it was added to the array.
+vindex(WIDGET, in hooks)
+Note that this means that the `tt(WIDGET)' special parameter tracks the
+var(widgetname) when the widget function is called, rather than tracking
+the name of the corresponding special hook widget.
 
-If the option tt(-d) is given, the var(widgename) is removed from
+If the option tt(-d) is given, the var(widgetname) is removed from
 the array of widgets to be executed.
 
 If the option tt(-D) is given, the var(widgetname) is treated as a pattern
@@ -370,7 +367,6 @@ passed as arguments to tt(autoload) as with tt(add-zsh-hook).  The
 widget is also created with `tt(zle -N )var(widgetname)' to cause the
 corresponding function to be loaded the first time the hook is called.
 
-
 The arrays of var(widgetname) are currently maintained in tt(zstyle)
 contexts, one for each var(hook) context, with a style of `tt(widgets)'.
 If the tt(-L) option is given, this set of styles is listed with
diff --git a/Functions/Zle/add-zle-hook-widget b/Functions/Zle/add-zle-hook-widget
index 760e26d..cc2d54a 100644
--- a/Functions/Zle/add-zle-hook-widget
+++ b/Functions/Zle/add-zle-hook-widget
@@ -35,25 +35,23 @@ local -a hooktypes=( zle-isearch-exit zle-isearch-update
 # Stash in zstyle to make it global
 zstyle zle-hook types ${hooktypes#zle-}
 
-for hook in $hooktypes
-do
-  function azhw:$hook {
-      local -a hook_widgets
-      local hook
-      # Values of these styles look like number:name
-      # and we run them in number order
-      zstyle -a $WIDGET widgets hook_widgets
-      for hook in "${(@)${(@on)hook_widgets[@]}#<->:}"; do
-	  if [[ "$hook" = user:* ]]; then
-	      # Preserve $WIDGET within the renamed widget
-	      zle "$hook" -N "$@"
-	  else
-	      zle "$hook" -Nw "$@"
-	  fi || return
-      done
-      return 0
-  }
-done
+# Relying on multifuncdef option here
+function azhw:${^hooktypes} {
+    local -a hook_widgets
+    local hook
+    # Values of these styles look like number:name
+    # and we run them in number order
+    zstyle -a $WIDGET widgets hook_widgets
+    for hook in "${(@)${(@on)hook_widgets[@]}#<->:}"; do
+	if [[ "$hook" = user:* ]]; then
+	    # Preserve $WIDGET within the renamed widget
+	    zle "$hook" -N "$@"
+	else
+	    zle "$hook" -Nw "$@"
+	fi || return
+    done
+    return 0
+}
 
 # Redefine ourself with the setup left out
 
@@ -127,12 +125,25 @@ function add-zle-hook-widget {
 	    fi
 	fi
     else
+	# Check whether attempting to add a widget named for the hook
+	if [[ "$fn" = "$hook" ]]; then
+	    if [[ -n "${widgets[$fn]}" ]]; then
+		print -u2 "Cannot hook $fn to itself"
+		return 1
+	    fi
+	    # No point in building the array until another is added
+	    autoload "${autoopts[@]}" -- "$fn"
+	    zle -N "$fn"
+	    return 0
+	fi
 	integer i=${#options[ksharrays]}-2
 	zstyle -g extant_hooks "$hook" widgets
         # Check for an existing widget, add it as the first hook
 	if [[ ${widgets[$hook]} != "user:azhw:$hook" ]]; then
-	    zle -A "$hook" "${widgets[$hook]}"
-	    extant_hooks=(0:"${widgets[$hook]}" "${extant_hooks[@]}")
+	    if [[ -n ${widgets[$hook]} ]]; then
+		zle -A "$hook" "${widgets[$hook]}"
+		extant_hooks=(0:"${widgets[$hook]}" "${extant_hooks[@]}")
+	    fi
 	    zle -N "$hook" azhw:"$hook"
 	fi
 	# Add new widget only if not already in the hook list

[PATCH] _add-zle-hook-widget: New completion.

[Daniel Shahaf]

---
I wanted to use 'autoload +X add-zle-hook-widget; zstyle -g reply zle-hook
types' to avoid duplicating $hooktypes, but couldn't get it to work.  (Neither
with -k nor with -z was the style defined)

Daniel

 Completion/Zsh/Function/_add-zle-hook-widget | 34 ++++++++++++++++++++++++++++
 Functions/Zle/add-zle-hook-widget            |  1 +
 2 files changed, 35 insertions(+)
 create mode 100644 Completion/Zsh/Function/_add-zle-hook-widget

diff --git a/Completion/Zsh/Function/_add-zle-hook-widget b/Completion/Zsh/Function/_add-zle-hook-widget
new file mode 100644
index 0000000..40afb7e
--- /dev/null
+++ b/Completion/Zsh/Function/_add-zle-hook-widget
@@ -0,0 +1,34 @@
+#compdef add-zle-hook-widget
+
+local context state state_descr line
+typeset -A opt_args
+
+_add-zle-hook-widget_classes() {
+  local -a hooktypes=( zle-isearch-exit zle-isearch-update
+                     zle-line-pre-redraw zle-line-init zle-line-finish
+                     zle-history-line-set zle-keymap-select )
+                     # $hooktypes is duplicated in add-zle-hook-function itself, if you add entries update both places
+  compadd "$@" -a hooktypes
+}
+
+_add-zle-hook-widget_widgets() {
+  if (( $+opt_args[-d] )); then
+    local -a tmp
+    zstyle -g tmp $line[1] widgets
+    _wanted widgets expl "installed hooks" compadd -- ${tmp#<->:} && return 0
+  else
+    _wanted widgets expl widget compadd -M 'r:|-=* r:|=*' -k widgets && ret=0 && return 0
+  fi
+  return 1
+}
+
+_add-zle-hook-widget() {
+  _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 class:_add-zle-hook-widget_classes' \
+    ':widget:_add-zle-hook-widget_widgets'
+}
+
+_add-zle-hook-widget "$@"
diff --git a/Functions/Zle/add-zle-hook-widget b/Functions/Zle/add-zle-hook-widget
index cc2d54a..7f749b4 100644
--- a/Functions/Zle/add-zle-hook-widget
+++ b/Functions/Zle/add-zle-hook-widget
@@ -32,6 +32,7 @@ zmodload -e zsh/zle || return 1
 local -a hooktypes=( zle-isearch-exit zle-isearch-update
                      zle-line-pre-redraw zle-line-init zle-line-finish
                      zle-history-line-set zle-keymap-select )
+                     # $hooktypes is duplicated in _add-zle-hook-function, if you add entries update both places
 # Stash in zstyle to make it global
 zstyle zle-hook types ${hooktypes#zle-}
 

Re: [PATCH] _add-zle-hook-widget: New completion.

[Bart Schaefer]

On Jul 17,  3:00pm, Daniel Shahaf wrote:
}
} I wanted to use 'autoload +X add-zle-hook-widget; zstyle -g reply
} zle-hook types' to avoid duplicating $hooktypes, but couldn't get it
} to work. (Neither with -k nor with -z was the style defined)

Er, yeah:

     The flag +X attempts to load each NAME as an autoloaded function,
     but does _not_ execute it.

This is in part why I went to some lengths to be able to "source" the
file as well as autoload it.

You could do this:

_add-zle-hook-widget_classes() {
  local -a hooktypes=( ${^${=${(f)"$( add-zle-hook-widget -h )"}[3]}} )
  compadd "$@" -M 'L:|zle-=' -a hooktypes
}

Re: [PATCH] _add-zle-hook-widget: New completion.

[Bart Schaefer]

On Jul 17, 12:21pm, Bart Schaefer wrote:
}
} _add-zle-hook-widget_classes() {
}   local -a hooktypes=( ${^${=${(f)"$( add-zle-hook-widget -h )"}[3]}} )
}   compadd "$@" -M 'L:|zle-=' -a hooktypes
} }

Er, never mind the ${^...} , that was before I thought of -M 'L:...'

So just

_add-zle-hook-widget_classes() {
  local -a hooktypes=( ${=${(f)"$( add-zle-hook-widget -h )"}[3]} )
  compadd "$@" -M 'L:|zle-=' -a hooktypes
}

Re: [PATCH] _add-zle-hook-widget: New completion.

[Bart Schaefer]

One other remark:

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

Re: [PATCH] _add-zle-hook-widget: New completion.

[Oliver Kiddle]

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.

> +    _wanted widgets expl "installed hooks" compadd -- ${tmp#<->:} && return 0

The description should be singular - "installed hook". Even if you can
specify a list, they are only completed one at a time.

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

ret was not declared local so ret=0 should not be there.
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.

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

Oliver

Re: [PATCH] _add-zle-hook-widget: New completion.

[Bart Schaefer]

On Jul 18, 11:47am, Oliver Kiddle wrote:
}
} 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?

Yes.

} 'B:zle-=' is a shorter version

Might be worth mentioning that in the yodl page, I was going by the
example of removing "no" from the front of setopt names.

Re: [PATCH] _add-zle-hook-widget: New completion.

[Oliver Kiddle]

Bart wrote:
> } >  compadd "$@" -M 'L:|zle-=' -a hooktypes

> } 'B:zle-=' is a shorter version
>
> Might be worth mentioning that in the yodl page, I was going by the
> example of removing "no" from the front of setopt names.

After reading the documentation, it is apparent that there is a
difference. The B: form allows zle- to appear multiple times because the
zle- needs to be at that start of the match rather than at the start
of what's on the line. I've used this for numbers for initial zeros -
B:0= which is sometimes useful as a way to force a particular completion
alternative without contraining the matches. fc 0<tab> for example.

For the hook types, I wonder if the best choice might be -M 'L:|=zle-'
along with including zle- in the matches. That allows partial completion
to work.

Oliver

Re: [PATCH] _add-zle-hook-widget: New completion.

[Bart Schaefer]

On Jul 19, 12:30pm, Oliver Kiddle wrote:
}
} For the hook types, I wonder if the best choice might be -M 'L:|=zle-'
} along with including zle- in the matches. That allows partial completion
} to work.

I tried that but it adds the trailing space after "zle- " -- I suppose
it could be added with a separate call (compadd -S '' zle-) instead of
just added to the array of hook types.

[PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.

[Daniel Shahaf]

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]' \

Re: [PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.

[Oliver Kiddle]

Daniel Shahaf wrote:
> 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.

I don't claim to be the arbiter on whatever the house style is.
Most early completion code was written by Sven and I've tried to
be consistent with that. Looking over some examples now, he didn't
appear to use superfluous return statements.

In the end, it is more important to get the return status right,
however.

In the process of looking, I noticed that _normal appears to use a
ret variable without declaring it local. _main_complete does set
ret to 1 so this never actually matters. Furthermore, while checking
that this wasn't intentional I noticed a couple of functions that
use _normal where _default was meant. If you haven't got a compset
-q, -n or similar changes to words/CURRENT, _normal ends up being
a never ending recursive loop.

Oliver

diff --git a/Completion/Base/Core/_normal b/Completion/Base/Core/_normal
index 539b378..dd607d2 100644
--- a/Completion/Base/Core/_normal
+++ b/Completion/Base/Core/_normal
@@ -30,9 +30,9 @@ if [[ CURRENT -eq 1 ]]; then
   curcontext="${curcontext%:*:*}:-command-:"
 
   comp="$_comps[-command-]"
-  [[ -n "$comp" ]] && eval "$comp" && ret=0
+  [[ -n "$comp" ]] && eval "$comp" && return
 
-  return ret
+  return 1
 fi
 
 _set_command
diff --git a/Completion/Unix/Command/_iostat b/Completion/Unix/Command/_iostat
index 7dc33a1..6653a5d 100644
--- a/Completion/Unix/Command/_iostat
+++ b/Completion/Unix/Command/_iostat
@@ -129,4 +129,4 @@ if (( $#args )); then
   return
 fi
 
-_normal
+_default
diff --git a/Completion/Unix/Command/_top b/Completion/Unix/Command/_top
index 10c0e34..0259c23 100644
--- a/Completion/Unix/Command/_top
+++ b/Completion/Unix/Command/_top
@@ -98,4 +98,4 @@ if (( $#specs )); then
   return
 fi
 
-_normal
+_default

Re: [PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.

[Daniel Shahaf]

Oliver Kiddle wrote on Thu, Jul 21, 2016 at 17:28:26 +0200:
> Daniel Shahaf wrote:
> > 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.
> 
> I don't claim to be the arbiter on whatever the house style is.
> Most early completion code was written by Sven and I've tried to
> be consistent with that. Looking over some examples now, he didn't
> appear to use superfluous return statements.
> 
> In the end, it is more important to get the return status right,
> however.

I've decided to let PEP 20 be the arbiter and kept the return
statements.

> Furthermore, while checking that this wasn't intentional I noticed
> a couple of functions that use _normal where _default was meant.

_default appears to be undocumented.

Cheers,

Daniel

Re: [PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.

[Bart Schaefer]

On Jul 22,  6:22am, Daniel Shahaf wrote:
}
} _default appears to be undocumented.

There was a decision made at some point (*very* long ago, while Sven
was still actively inventing the compltion system) that the yodl doc
should only describe commands and functions etc. that an ordinary user
might employ in startup files, and all other details that developers
would use to write new parts of the completion system would only be
documented where developers could see it, e.g., in code comments.

Hence for example the complete lack of explanation of comptags and
comptry, even though they are at least *mentioned* in the yodl.

Sadly this didsn't really take new developers into account, because
you'd have to know what you were looking for before you could find
any doc for it, and not everything was adequately commented either.
Leaving us with a bunch of stuff that ought to be documented and
nobody with time to backfill it.

I learned the term "technical debt" recently ...

Re: [PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.

[Oliver Kiddle]

Daniel Shahaf wrote:
> > Furthermore, while checking that this wasn't intentional I noticed
> > a couple of functions that use _normal where _default was meant.
>
> _default appears to be undocumented.

Actually it sort of is documented: the section on special contexts which
mentions -default- and says that default implementations exist named
_context. Should there perhaps be findex entries there for each context?

Thinking about it, perhaps the original intention was that _dispatch
should be used but direct calls to _default are widespread and date
back to early times. How about the following.

Oliver

diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 8792324..460cbb6 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -4217,6 +4217,12 @@ 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.
 )
+findex(_default)
+item(tt(_default))(
+This function corresponds to the tt(-default-) special context. It is
+also used directly as a fallback when a function is unable to provide
+more specific completions.
+)
 findex(_describe)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ @ @ ))ifnztexi(          )))
 xitem(tt(_describe )[tt(-12JVx)] [ tt(-oO) | tt(-t) var(tag) ] var(descr) var(name1) [ var(name2) ] [ var(opt) ... ])

Re: [PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.

[Daniel Shahaf]

Oliver Kiddle wrote on Fri, Jul 22, 2016 at 20:45:40 +0200:
> > > Furthermore, while checking that this wasn't intentional I noticed
> > > a couple of functions that use _normal where _default was meant.
> >
> > _default appears to be undocumented.
> 
> Actually it sort of is documented: the section on special contexts which
> mentions -default- and says that default implementations exist named
> _context.

Ah, I missed those since I just grepped zshall for /_default/.

> Should there perhaps be findex entries there for each context?

This would make a search for /_default/ succeed in the pdf/html
rendering, but won't affect the man page rendering.

I'm not sure what we can do for the man page version, short of
enumerating all the _foo, _bar, _baz names in the running text right
after the enumeration of -foo-, -bar-, -baz- names.

> Thinking about it, perhaps the original intention was that _dispatch
> should be used but direct calls to _default are widespread and date
> back to early times. How about the following.

I'm not sufficiently familiar with this corner of compsys to be able to
review the diff for completeness, but thanks for writing it.

Cheers,

Daniel

Re: add-zle-hook-widget Re: Next release (5.3)

[Daniel Shahaf]

Bart Schaefer wrote on Wed, Jul 13, 2016 at 17:11:52 -0700:
> -# Handle zsh autoloading conventions
> -if [[ "$zsh_eval_context" = *loadautofunc && ! -o kshautoload ]]; then
> -    add-zle-hook-widget "$@"
> -fi
> +# Handle zsh autoloading conventions:
> +# - "file" appears last in zsh_eval_context when "source"-ing
> +# - "evalautofunc" appears with kshautoload set or autoload -k
> +# - "loadautofunc" appears with kshautoload unset or autoload -z
> +# - use of autoload +X cannot reliably be detected, use best guess
> +case "$zsh_eval_context" in
> +*file) ;;
> +*evalautofunc) ;;
> +*loadautofunc) add-zle-hook-widget "$@";;
> +*) [[ -o kshautoload ]] || add-zle-hook-widget "$@";;

The [[ -o ]] test will be always false because of the 'emulate -L zsh'
at the top, won't it?

This hunk should also be applied to bracketed-paste-magic; there there
is no toplevel 'emulate'.

> +esac
> +# Note fallback here is equivalent to the usual best-guess used by
> +# functions written for zsh before $zsh_eval_context was available
> +# so this case-statement is backward-compatible.


Bart Schaefer wrote on Sat, Jul 16, 2016 at 18:51:03 -0700:
> It also switches from defining the hooks functions in a loop to defining
> them with multifuncdef syntax, and updates contrib.yo.

The new docs look great, thank you very much! ☺

Re: add-zle-hook-widget Re: Next release (5.3)

[Bart Schaefer]

On Jul 17,  2:59pm, Daniel Shahaf wrote:
} Subject: Re: add-zle-hook-widget Re: Next release (5.3)
}
} > +*evalautofunc) ;;
} > +*loadautofunc) add-zle-hook-widget "$@";;
} > +*) [[ -o kshautoload ]] || add-zle-hook-widget "$@";;
} 
} The [[ -o ]] test will be always false because of the 'emulate -L zsh'
} at the top, won't it?

True, though only applicable if the evalautofunc test fails, which it
will on zsh versions from before mid-2010.  Could fix this by wrapping
the entire body up to just before that "case" in an anonymous sub,
but that fails on zsh versions from before mid-2008.  OTOH there are
other incompatibilities lurking, so that's probably the easiest thing.

On a third hand (?), though, a large fraction of files in Functions/
assume "autoload -Uz", use emulate, and don't bother testing ksh-ism.

} This hunk should also be applied to bracketed-paste-magic

Yep.  Was waiting to see if there was feedback.

I think I should also move add-zle-hook-widget to Functions/Misc/.  All
the other files in Functions/Zle/ represent widgets.