Does add-zle-hook-widget violate the contract of ZLE hook widgets?

Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Marlon Richert]

% foo() { [[ -n $foo ]] && print foo }
% zle -N zle-line-init foo
% autoload -Uz add-zle-hook-widget
% bar() { print bar }
% add-zle-hook-widget line-init bar
%

foo implements a perfectly fine zle-line-init widget. It obeys the
contract laid out at
https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
which says nothing about return values. There are plenty of such
zle-xxx widgets out there in the wild, written before the introduction
of add-zle-hook-widget.

bar is also a perfectly fine hook function, implementing the contract
laid out by add-zle-hook-widget.

However, through no fault of its own, bar will never get called,
because once foo has return non-zero, azsh:zle-line-init will not call
any further hook widgets:

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" -f "nolast" -N -- "$@"
else
    zle "$hook" -f "nolast" -Nw -- "$@"
fi || return  # <-- This is the offending line.
    done
    return 0
}

(Comment mine.)

Isn't add-zle-hook-widget here violating the contract between foo and
zle -N zle-line-init? Should that `|| return` be removed?

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Daniel Shahaf]

Marlon Richert wrote on Wed, 23 Jun 2021 20:30 +00:00:
> foo implements a perfectly fine zle-line-init widget. It obeys the
> contract laid out at
> https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
> which says nothing about return values.

That has two possible interpretations:

- Hook functions may set $? however they want

- Hook functions should set $? to zero on success and to non-zero
  otherwise

In favour of the first interpretation is that the manual's language
doesn't explicitly rule it out and compatibility with hook functions
that predate add-zle-hook-widget and don't set $? as
add-zle-hook-widget expects.

In favour of the second interpretation is that those semantics are the
default, ubiquitous, baked into the language's syntax, and extensible
(same thing as "This bit must be zero" in wire protocol specifications).

I'm voting for the second interpretation.

I suppose we could've added something to NEWS/README when add-zle-hook-widget
was introduced, if only because this is an interoperability issue (foo
and bar may be have separate maintainers).

> Isn't add-zle-hook-widget here violating the contract between foo and
> zle -N zle-line-init?

No.  The contract with foo is that it'll be called, and it does get called.

> Should that `|| return` be removed?

No, because that would break the case of _deliberately_ returning non-zero
from one a-z-h-w hook to prevent further a-z-h-w hooks from running.
Granted, that's not a documented promise, but there's no reason to break
it, either.

Hope that answers your question.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Lawrence Velázquez]

On Wed, Jun 23, 2021, at 5:57 PM, Daniel Shahaf wrote:
> Marlon Richert wrote on Wed, 23 Jun 2021 20:30 +00:00:
> > foo implements a perfectly fine zle-line-init widget. It obeys the
> > contract laid out at
> > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
> > which says nothing about return values.
> 
> That has two possible interpretations:
> 
> - Hook functions may set $? however they want
> 
> - Hook functions should set $? to zero on success and to non-zero
>   otherwise
> 
> In favour of the first interpretation is that the manual's language
> doesn't explicitly rule it out

It's worth noting that the "Hook Functions" section, while not
saying anything about ZLE hook functions in particular, does say:

	A function found by this mechanism is referred to elsewhere
	as a 'hook function'.  An error in any function causes
	subsequent functions not to be run.

https://zsh.sourceforge.io/Doc/Release/Functions.html#Hook-Functions

-- 
vq

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Daniel Shahaf]

Lawrence Velázquez wrote on Wed, 23 Jun 2021 23:33 +00:00:
> On Wed, Jun 23, 2021, at 5:57 PM, Daniel Shahaf wrote:
> > Marlon Richert wrote on Wed, 23 Jun 2021 20:30 +00:00:
> > > foo implements a perfectly fine zle-line-init widget. It obeys the
> > > contract laid out at
> > > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
> > > which says nothing about return values.
> > 
> > That has two possible interpretations:
> > 
> > - Hook functions may set $? however they want
> > 
> > - Hook functions should set $? to zero on success and to non-zero
> >   otherwise
> > 
> > In favour of the first interpretation is that the manual's language
> > doesn't explicitly rule it out
> 
> It's worth noting that the "Hook Functions" section, while not
> saying anything about ZLE hook functions in particular, does say:
> 
> 	A function found by this mechanism is referred to elsewhere
> 	as a 'hook function'.  An error in any function causes
> 	subsequent functions not to be run.
> 

By code inspection, the last sentence applies to add-zle-hook-widget and
«add-zsh-hook zsh_directory_name», but not to other «add-zsh-hook foo».

> https://zsh.sourceforge.io/Doc/Release/Functions.html#Hook-Functions

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Marlon Richert]

On Thu, Jun 24, 2021 at 1:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Marlon Richert wrote on Wed, 23 Jun 2021 20:30 +00:00:
> > foo implements a perfectly fine zle-line-init widget. It obeys the
> > contract laid out at
> > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
> > which says nothing about return values.
>
> That has two possible interpretations:
>
> - Hook functions may set $? however they want
>
> - Hook functions should set $? to zero on success and to non-zero
>   otherwise
>
> In favour of the first interpretation is that the manual's language
> doesn't explicitly rule it out and compatibility with hook functions
> that predate add-zle-hook-widget and don't set $? as
> add-zle-hook-widget expects.
>
> In favour of the second interpretation is that those semantics are the
> default, ubiquitous, baked into the language's syntax, and extensible
> (same thing as "This bit must be zero" in wire protocol specifications).
>
> I'm voting for the second interpretation.
>
> I suppose we could've added something to NEWS/README when add-zle-hook-widget
> was introduced, if only because this is an interoperability issue (foo
> and bar may be have separate maintainers).

I think this should be mentioned permanently in the manual, exactly
because we see foo and bar from separate maintainers in the wild.

> > Isn't add-zle-hook-widget here violating the contract between foo and
> > zle -N zle-line-init?
>
> No.  The contract with foo is that it'll be called, and it does get called.
>
> > Should that `|| return` be removed?
> No, because that would break the case of _deliberately_ returning non-zero
> from one a-z-h-w hook to prevent further a-z-h-w hooks from running.
> Granted, that's not a documented promise, but there's no reason to break
> it, either.

For hooks added through azhw, yes. But hooks added through zle -N
zle-<hook> have no reason to expect that their return value has any
effect.

How about if azhw would ignore the return value _only_ of any
pre-existing zle -N zle-X widget? For example, this part of the azhw
code could be modified to wrap the original widget function in a
function that always returns zero:

        # Check for an existing widget, add it as the first hook
    if [[ ${widgets[$hook]:-} != "user:azhw:$hook" ]]; then
        if [[ -n ${widgets[$hook]:-} ]]; then
        zle -A "$hook" "${widgets[$hook]}"  # <-- This line could be
changed. --M
        extant_hooks=(0:"${widgets[$hook]}" "${extant_hooks[@]}")
        fi
        zle -N "$hook" azhw:"$hook"
    fi

Also, regardless of the above, the documentation in
https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets
could be updated to guide the user towards azhw. And the documentation
of azhw could be updated to mention the canceling behavior.

> Hope that answers your question.

I didn't mean this as a Q&A, but more as a point of discussion. :)

If we are happy with the behavior as-is, then this should be
documented. In particular, 3rd-party developers should be strongly
encouraged to use azhw and not zle -N zle-X.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Roman Perepelitsa]

On Thu, Jun 24, 2021 at 12:34 PM Marlon Richert
<marlon.richert@gmail.com> wrote:
>
> [...] the documentation in
> https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets
> could be updated to guide the user towards azhw.
>
> [...] 3rd-party developers should be strongly
> encouraged to use azhw and not zle -N zle-X.

Given the current state of add-zle-hook-widget [*] I would be wary of
giving this advice. I personally never use it and cannot recommend
anyone to use it. If your goal is to write a robust plugin that is
unlikely to break in the presence of other plugins,
add-zle-hook-widget is a poor choice.

Roman.

[*] This thread, workers/49084 and workers/49085.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Daniel Shahaf]

Roman Perepelitsa wrote on Thu, Jun 24, 2021 at 12:45:53 +0200:
> On Thu, Jun 24, 2021 at 12:34 PM Marlon Richert
> <marlon.richert@gmail.com> wrote:
> >
> > [...] the documentation in
> > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets
> > could be updated to guide the user towards azhw.
> >
> > [...] 3rd-party developers should be strongly
> > encouraged to use azhw and not zle -N zle-X.
> 
> [*] This thread, workers/49084 and workers/49085.

Let's break this down.

This thread is about the fact that a-z-h-w has an «if (( $? )); then
break; fi» in its "loop over all registered hooks and call each in turn"
code.  It has had that check since its first release, and whatever
semantics we desire are easy to implement.  It's not a big deal.

workers/49084 is an edge case involving pre-azhw code trying to wrap
azhw.  It's a teething problem, and shouldn't be too hard to fix.

I haven't fully digested workers/49085, but its author does say there
that a fix is "not too difficult".

> Given the current state of add-zle-hook-widget [*] I would be wary of
> giving this advice. I personally never use it and cannot recommend
> anyone to use it. If your goal is to write a robust plugin that is
> unlikely to break in the presence of other plugins,
> add-zle-hook-widget is a poor choice.
> 

It's not clear to me why you're raising the alarm bells against uses of
a-z-h-w, for two reasons:

One, the specific bugs you cite seem to be easy to fix and uncommon to
run into (they haven't been run into in the 4+ years since a-z-h-w's
first release).

Two, you don't discuss alternatives.  The conclusion "Don't use a-z-h-w"
does not follow from the premise "a-z-h-w has bugs".  It might follow
from a proposition of the form "a-z-h-w is worse than the alternative",
but that was not argued — and, _prima facie_, it's easier to write bugs
without a-z-h-w than with it.

> Roman.
> 

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Roman Perepelitsa]

On Thu, Jun 24, 2021 at 8:54 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Roman Perepelitsa wrote on Thu, Jun 24, 2021 at 12:45:53 +0200:
> > Given the current state of add-zle-hook-widget [*] I would be wary of
> > giving this advice
>
> It's not clear to me why you're raising the alarm bells against uses of
> a-z-h-w

That's not a fair rephrasing of what I wrote. I wouldn't recommend
using the current version of add-zle-hook-widget.

> you don't discuss alternatives

That's fair.

Roman.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Daniel Shahaf]

Marlon Richert wrote on Thu, Jun 24, 2021 at 13:34:00 +0300:
> On Thu, Jun 24, 2021 at 1:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Marlon Richert wrote on Wed, 23 Jun 2021 20:30 +00:00:
> > > foo implements a perfectly fine zle-line-init widget. It obeys the
> > > contract laid out at
> > > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
> > > which says nothing about return values.
> >
> > That has two possible interpretations:
> >
> > - Hook functions may set $? however they want
> >
> > - Hook functions should set $? to zero on success and to non-zero
> >   otherwise
> >
> > In favour of the first interpretation is that the manual's language
> > doesn't explicitly rule it out and compatibility with hook functions
> > that predate add-zle-hook-widget and don't set $? as
> > add-zle-hook-widget expects.
> >
> > In favour of the second interpretation is that those semantics are the
> > default, ubiquitous, baked into the language's syntax, and extensible
> > (same thing as "This bit must be zero" in wire protocol specifications).
> >
> > I'm voting for the second interpretation.
> >
> > I suppose we could've added something to NEWS/README when add-zle-hook-widget
> > was introduced, if only because this is an interoperability issue (foo
> > and bar may be have separate maintainers).
> 
> I think this should be mentioned permanently in the manual, exactly
> because we see foo and bar from separate maintainers in the wild.

I'm ambivalent about this.

On the one hand, as mentioned, "set $? correctly" is a ground rule that
shouldn't need to be made explicit everywhere it matters.  I don't want
to create an expectation that where the manual _doesn't_ explicitly
specify that $? should be set correctly, $?'s value doesn't matter.

On the one hand, someone who uses «zle -N zle-line-init foo» may not be
aware of a-z-h-w and of the fact that the return value does have an
effect.

In balance, I think I would rather see it documented globally that $?
should always be set to zero/non-zero unless explicitly specified
otherwise.  Makes sense?

> > > Isn't add-zle-hook-widget here violating the contract between foo and
> > > zle -N zle-line-init?
> >
> > No.  The contract with foo is that it'll be called, and it does get called.
> >
> > > Should that `|| return` be removed?
> > No, because that would break the case of _deliberately_ returning non-zero
> > from one a-z-h-w hook to prevent further a-z-h-w hooks from running.
> > Granted, that's not a documented promise, but there's no reason to break
> > it, either.
> 
> For hooks added through azhw, yes. But hooks added through zle -N
> zle-<hook> have no reason to expect that their return value has any
> effect.
> 

I did give some reasons in my previous reply; and compare how drivers
should use their turn indicators even when they don't see anyone on the
road.

> How about if azhw would ignore the return value _only_ of any
> pre-existing zle -N zle-X widget? For example, this part of the azhw
> code could be modified to wrap the original widget function in a
> function that always returns zero:
> 
>         # Check for an existing widget, add it as the first hook
>     if [[ ${widgets[$hook]:-} != "user:azhw:$hook" ]]; then
>         if [[ -n ${widgets[$hook]:-} ]]; then
>         zle -A "$hook" "${widgets[$hook]}"  # <-- This line could be changed. --M
>         extant_hooks=(0:"${widgets[$hook]}" "${extant_hooks[@]}")
>         fi
>         zle -N "$hook" azhw:"$hook"
>     fi
> 

At this point, given that the current behaviour has appeared in stable
releases for 4.5 years now and is _a priori_ preferable, I think it's
better to document the current behaviour and move on.  So, basically, an
addition to the incompatible changes section in README (for 5.9) that
describes the change in 5.3.  WDYT?

> Also, regardless of the above, the documentation in
> https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets
> could be updated to guide the user towards azhw.

+1.  Anyone interested in writing a patch?

> And the documentation of azhw could be updated to mention the
> canceling behavior.

Sure.

Also, the documentation of a-z-h-w should spell out "zle-line-init",
"zle-line-finish", etc., in full for greppability.

> > Hope that answers your question.
> 
> I didn't mean this as a Q&A, but more as a point of discussion. :)
> 

Ack.  I did take it this way; sorry for my unclarity.

> If we are happy with the behavior as-is, then this should be
> documented. In particular, 3rd-party developers should be strongly
> encouraged to use azhw and not zle -N zle-X.

Some sort of "Best practices for third-party plugin maintainers"
document might be a good idea.  Could make it a bit more general and
also answer "Where do I install my completion function to?" for
third-party packages (e.g., _curl) and so on.  It could start as
additions to the FAQ.

Cheers,

Daniel

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Marlon Richert]

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

On Thu, Jun 24, 2021 at 9:30 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Marlon Richert wrote on Thu, Jun 24, 2021 at 13:34:00 +0300:
> > On Thu, Jun 24, 2021 at 1:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > >
> > > Marlon Richert wrote on Wed, 23 Jun 2021 20:30 +00:00:
> > > > foo implements a perfectly fine zle-line-init widget. It obeys the
> > > > contract laid out at
> > > > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
> > > > which says nothing about return values.
> > >
> > > That has two possible interpretations:
> > >
> > > - Hook functions may set $? however they want
> > >
> > > - Hook functions should set $? to zero on success and to non-zero
> > >   otherwise
> > >
> > > In favour of the first interpretation is that the manual's language
> > > doesn't explicitly rule it out and compatibility with hook functions
> > > that predate add-zle-hook-widget and don't set $? as
> > > add-zle-hook-widget expects.
> > >
> > > In favour of the second interpretation is that those semantics are the
> > > default, ubiquitous, baked into the language's syntax, and extensible
> > > (same thing as "This bit must be zero" in wire protocol specifications).
> > >
> > > I'm voting for the second interpretation.
> > >
> > > I suppose we could've added something to NEWS/README when add-zle-hook-widget
> > > was introduced, if only because this is an interoperability issue (foo
> > > and bar may be have separate maintainers).
> >
> > I think this should be mentioned permanently in the manual, exactly
> > because we see foo and bar from separate maintainers in the wild.
>
> I'm ambivalent about this.
>
> On the one hand, as mentioned, "set $? correctly" is a ground rule that
> shouldn't need to be made explicit everywhere it matters.  I don't want
> to create an expectation that where the manual _doesn't_ explicitly
> specify that $? should be set correctly, $?'s value doesn't matter.
>
> On the one hand, someone who uses «zle -N zle-line-init foo» may not be
> aware of a-z-h-w and of the fact that the return value does have an
> effect.
>
> In balance, I think I would rather see it documented globally that $?
> should always be set to zero/non-zero unless explicitly specified
> otherwise.  Makes sense?

That makes sense, but then, it should also be documented what that
actually means. If your function returns zero/non-zero, what is
expected to happen? Does this always happen? Or are there different
classes of functions with different results for zero/non-zero?

I can already think of two examples that contradict each other:
* In hook functions, callee returning NON-zero causes the caller to
refrain from calling more functions.
* In completion functions, callee returning ZERO causes the caller to
refrain from calling more functions.

Those two behave exactly opposite of each other.

My point being, I think this is best spelled out everywhere.
Zero/non-zero don't always appear to mean the same thing between
caller and callee. Clearly it matters, but it doesn't appear to always
matter in the same way.


> > > > Should that `|| return` be removed?
> > > No, because that would break the case of _deliberately_ returning non-zero
> > > from one a-z-h-w hook to prevent further a-z-h-w hooks from running.
> > > Granted, that's not a documented promise, but there's no reason to break
> > > it, either.
> >
> > For hooks added through azhw, yes. But hooks added through zle -N
> > zle-<hook> have no reason to expect that their return value has any
> > effect.
> >
>
> I did give some reasons in my previous reply; and compare how drivers
> should use their turn indicators even when they don't see anyone on the
> road.

That's because it's convention everywhere that you signal in the
direction to which you turn. However, what convention we're supposed
to follow for return values in Zsh is not quite as clear. See my
contradictory examples above.


> > How about if azhw would ignore the return value _only_ of any
> > pre-existing zle -N zle-X widget? For example, this part of the azhw
> > code could be modified to wrap the original widget function in a
> > function that always returns zero:
>
> At this point, given that the current behaviour has appeared in stable
> releases for 4.5 years now and is _a priori_ preferable, I think it's
> better to document the current behaviour and move on.  So, basically, an
> addition to the incompatible changes section in README (for 5.9) that
> describes the change in 5.3.  WDYT?

I'm fine with that. That documentation needs to be findable, though,
from the docs for both zle -N zle-X and azhw.


> > Also, regardless of the above, the documentation in
> > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets
> > could be updated to guide the user towards azhw.
>
> +1.  Anyone interested in writing a patch?

Done. See attachment.


> > If we are happy with the behavior as-is, then this should be
> > documented. In particular, 3rd-party developers should be strongly
> > encouraged to use azhw and not zle -N zle-X.
>
> Some sort of "Best practices for third-party plugin maintainers"
> document might be a good idea.  Could make it a bit more general and
> also answer "Where do I install my completion function to?" for
> third-party packages (e.g., _curl) and so on.  It could start as
> additions to the FAQ.

I don't know if the FAQ is the right place. At least, I have never
found that document to be useful. ¯\_(ツ)_/¯  I would prefer to see
these things in the manual. How about putting the actual info in the
manual, but link to it from the FAQ? I consider the manual to be Zsh's
Single Source of Truth. Better to maintain everything there and not
repeat ourselves.

[-- Attachment #2: 0001-Document-important-points-for-ZLE-hook-widgets.txt --]
[-- Type: text/plain, Size: 9153 bytes --]

From 9cbc49f44a8e1ffe0ecd37378ed81fc0090ab411 Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlon.richert@gmail.com>
Date: Fri, 25 Jun 2021 00:39:11 +0300
Subject: [PATCH] Document important points for ZLE hook widgets

Under ZLE:
* Warn against overriding existing hook widgets.
* Refer to `add-zle-hook-widget`.

Under `add-zle-hook-widget`:
* Note about return values.
---
 Doc/Zsh/contrib.yo | 59 ++++++++++++++++++++++++++++------------------
 Doc/Zsh/manual.yo  |  6 ++++-
 Doc/Zsh/zle.yo     | 28 ++++++++++++++++++----
 3 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo
index e74528341..46a9ac08d 100644
--- a/Doc/Zsh/contrib.yo
+++ b/Doc/Zsh/contrib.yo
@@ -10,7 +10,11 @@ these are documented here.  For documentation on other contributed items
 such as shell functions, look for comments in the function source files.
 
 startmenu()
-menu(Utilities)
+menu(Accessing On-Line Help)
+menu(Recompiling Functions)
+menu(Keyboard Definition)
+menu(Dumping Shell State)
+menu(Manipulating Hook Functions)
 menu(Recent Directories)
 menu(Other Directory Functions)
 menu(Version Control Information)
@@ -23,10 +27,8 @@ menu(User Configuration Functions)
 menu(Other Functions)
 endmenu()
 
-texinode(Utilities)(Recent Directories)()(User Contributions)
-sect(Utilities)
-
-subsect(Accessing On-Line Help)
+texinode(Accessing On-Line Help)(Recompiling Functions)()(User Contributions)
+sect(Accessing On-Line Help)
 cindex(helpfiles utility)
 
 The key sequence tt(ESC h) is normally bound by ZLE to execute the
@@ -80,7 +82,8 @@ ifnzman(noderef(Parameters Used By The Shell))\
 installation; if it is not, copy tt(Functions/Misc/run-help) to an
 appropriate directory.
 
-subsect(Recompiling Functions)
+texinode(Recompiling Functions)(Keyboard Definition)(Accessing On-Line Help)(User Contributions)
+sect(Recompiling Functions)
 cindex(functions, recompiling)
 cindex(zrecompile utility)
 
@@ -168,7 +171,8 @@ Once the digests have been created and your tt(fpath) modified to refer to
 them, you can keep them up to date by running tt(zrecompile) with no
 arguments.
 
-subsect(Keyboard Definition)
+texinode(Keyboard Definition)(Dumping Shell State)(Recompiling Functions)(User Contributions)
+sect(Keyboard Definition)
 cindex(keyboard definition)
 
 findex(zkbd)
@@ -211,7 +215,8 @@ ifnzman(noderef(Parameters Used By The Shell))\
 installation; if it is not, copy tt(Functions/Misc/zkbd) to an
 appropriate directory.
 
-subsect(Dumping Shell State)
+texinode(Dumping Shell State)(Manipulating Hook Functions)(Keyboard Definition)(User Contributions)
+sect(Dumping Shell State)
 cindex(reporter utility)
 
 Occasionally you may encounter what appears to be a bug in the shell,
@@ -287,7 +292,8 @@ any prefix, even a single letter; thus tt(a) is the same as tt(aliases),
 tt(z) is the same as tt(zstyles), etc.
 enditem()
 
-subsect(Manipulating Hook Functions)
+texinode(Manipulating Hook Functions)(Recent Directories)(Dumping Shell State)(User Contributions)
+sect(Manipulating Hook Functions)
 cindex(hook function utility)
 
 startitem()
@@ -330,8 +336,8 @@ options tt(-Uz) are appropriate.
 findex(add-zle-hook-widget)
 item(tt(add-zle-hook-widget) [ tt(-L) | tt(-dD) ] [ tt(-Uzk) ] var(hook) var(widgetname))(
 Several widget names are special to the line editor, as described in the section
-ifnzman(Special Widgets, noderef(Zle Widgets))\
-ifzman(Special Widgets, see zmanref(zshzle)),
+ifnzman(Hook Widgets, noderef(Zle Widgets))\
+ifzman(Hook Widgets, see zmanref(zshzle)),
 in that they are automatically called at specific points during editing.
 Unlike function hooks, these do not use a predefined array of other names
 to call at the same point; the shell function tt(add-zle-hook-widget)
@@ -340,8 +346,8 @@ 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.  The special widget names are also accepted
+or tt(keymap-select), corresponding to each of the special hook widget names
+tt(zle-isearch-exit), etc.  The actual hook 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
@@ -354,6 +360,10 @@ 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.
 
+Also note that, for each hook, when any registered hook wigdet returns 
+non-zero, tt(add-zle-hook-widget) will exit and not call any subsequent 
+widgets.
+
 If the option tt(-d) is given, the var(widgetname) is removed from
 the array of widgets to be executed.
 
@@ -378,7 +388,7 @@ hooks, then all hooks should be managed only via this function.
 )
 enditem()
 
-texinode(Recent Directories)(Other Directory Functions)(Utilities)(User Contributions)
+texinode(Recent Directories)(Other Directory Functions)(Manipulating Hook Functions)(User Contributions)
 cindex(recent directories, maintaining list of)
 cindex(directories, maintaining list of recent)
 findex(cdr)
@@ -4357,9 +4367,10 @@ The return status is 0 if at least one match was performed, else 1.
 findex(run-help)
 item(tt(run-help) var(cmd))(
 This function is designed to be invoked by the tt(run-help) ZLE widget,
-in place of the default alias.  See `Accessing On-Line Help'
-ifzman(above)\
-ifnzman((noderef(Utilities))) for setup instructions.
+in place of the default alias.  See
+ifzman(see the bf(Accessing On-Line Help) section above)\
+ifnzman(noderef(Accessing On-Line Help))
+for setup instructions.
 
 In the discussion which follows, if var(cmd) is a file system path, it is
 first reduced to its rightmost component (the file name).
@@ -4579,9 +4590,10 @@ appear in the zsh distribution, but can be created by linking tt(zmv) to
 the names tt(zcp) and tt(zln) in some directory in your tt(fpath).
 )
 item(tt(zkbd))(
-See `Keyboard Definition'
-ifzman(above)\
-ifnzman((noderef(Utilities))).
+See
+ifzman(see the bf(Keyboard Definition) section above)\
+ifnzman(noderef(Keyboard Definition))\
+.
 )
 findex(zmv)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ ))ifnztexi(    )))
@@ -4664,9 +4676,10 @@ tt(zmv) source file, usually located in one of the directories named in
 your tt(fpath), or in tt(Functions/Misc/zmv) in the zsh distribution.
 )
 item(tt(zrecompile))(
-See `Recompiling Functions'
-ifzman(above)\
-ifnzman((noderef(Utilities))).
+See
+ifzman(see the bf(Recompiling Functions) section above)\
+ifnzman(noderef(Recompiling Functions))\
+.
 )
 findex(zstyle+)
 item(tt(zstyle+) var(context) var(style) var(value) [ tt(+) var(subcontext) var(style) var(value) ... ])(
diff --git a/Doc/Zsh/manual.yo b/Doc/Zsh/manual.yo
index 51601adbd..cb7cd7c2b 100644
--- a/Doc/Zsh/manual.yo
+++ b/Doc/Zsh/manual.yo
@@ -162,7 +162,11 @@ menu(Miscellaneous Features)
 
 User Contributions
 
-menu(Utilities)
+menu(Accessing On-Line Help)
+menu(Recompiling Functions)
+menu(Keyboard Definition)
+menu(Dumping Shell State)
+menu(Manipulating Hook Functions)
 menu(Recent Directories)
 menu(Other Directory Functions)
 menu(Version Control Information)
diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo
index 9694a1f74..d5763b491 100644
--- a/Doc/Zsh/zle.yo
+++ b/Doc/Zsh/zle.yo
@@ -1136,11 +1136,31 @@ example(if [[ $ZLE_STATE == *globalhistory*insert* ]]; then ...; fi)
 )
 enditem()
 
-subsect(Special Widgets)
+subsect(Hook Widgets)
 
-There are a few user-defined widgets which are special to the shell.
-If they do not exist, no special action is taken.  The environment
-provided is identical to that for any other editing widget.
+There are a few widget names that are special to the shell, with each name 
+corresponding to a ZLE event hook. For each hook name, if a user-defined widget 
+exists with the name, this widget is called automatically when an event 
+associated with the hook occurs, as described for hook name, below. If no 
+widget with a particular hook name exits, then no action is taken for the 
+associated events. Otherwise, the environment in which a hook widget runs is 
+identical to that for any other widget.
+
+Please note:
+startitemize()
+itemiz(When a widget with particular name is created, it replaces any existing 
+widget with the same name, regardless of whether that name is special to shell.
+Be aware of this when using tt(zle -N) to create a hook widget, as you might
+be overwriting an existing hook widget with the same name that's already in
+place.)
+itemiz(If your intention is not to overwrite an existing hook widget, but 
+rather to add another widget for the same hook, then you should strongly 
+consider using the tt(add-zle-hook-widget) function, which enables multiple 
+widget functions to be defined for the same ZLE hook. See
+ifzman(the subsection Manipulating Hook Functions in zmanref(zshmisc))\
+ifnzman(noderef(Manipulating Hook Functions))
+for more info.)
+enditemize()
 
 startitem()
 tindex(zle-isearch-exit)
-- 
2.30.1 (Apple Git-130)

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Lawrence Velázquez]

On Thu, Jun 24, 2021, at 5:48 PM, Marlon Richert wrote:
> On Thu, Jun 24, 2021 at 9:30 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > Marlon Richert wrote on Thu, Jun 24, 2021 at 13:34:00 +0300:
> > > Also, regardless of the above, the documentation in
> > > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets
> > > could be updated to guide the user towards azhw.
> >
> > +1.  Anyone interested in writing a patch?
> 
> Done. See attachment.

This patch appears to contain extraneous changes from workers/48978
that are not relevant to this discussion.  It would be best if you
submitted a new one based on a clean tree.

-- 
vq

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Marlon Richert]

On Fri, Jun 25, 2021 at 1:30 AM Lawrence Velázquez <larryv@zsh.org> wrote:
>
> On Thu, Jun 24, 2021, at 5:48 PM, Marlon Richert wrote:
> > On Thu, Jun 24, 2021 at 9:30 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > Marlon Richert wrote on Thu, Jun 24, 2021 at 13:34:00 +0300:
> > > > Also, regardless of the above, the documentation in
> > > > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets
> > > > could be updated to guide the user towards azhw.
> > >
> > > +1.  Anyone interested in writing a patch?
> >
> > Done. See attachment.
>
> This patch appears to contain extraneous changes from workers/48978
> that are not relevant to this discussion.  It would be best if you
> submitted a new one based on a clean tree.

I included those changes intentionally:
1. In the section ZLE > Hook Widgets of the HTML version of the
manual, I want to include a link to User Contrib > Manipulating Hook
Functions.
2. To create a link, I need to use a `noderef`.
3. To be able to use `noderef`, I need a `texinode` to link to.
4. Manipulating Hook Functions doesn't have a `texinode`, so I need to add one.
5. When I add a `texinode`, `make install.info` fails to compile if
there isn't a `menu` in the same page that links to it.
6. Hence, I need to update the menu structure of User Contrib.

However, my understanding of Yodl is limited. Is there a simpler way to do this?

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Bart Schaefer]

On Sat, Jun 26, 2021 at 6:14 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> I included those changes intentionally:
> 1. In the section ZLE > Hook Widgets of the HTML version of the
> manual, I want to include a link to User Contrib > Manipulating Hook
> Functions.

Typically the way to do this would be to include a link to User
Contrib within text that says something like 'see Manipulating Hook
Functions in noderef(...)'.

> 2. To create a link, I need to use a `noderef`.
> 3. To be able to use `noderef`, I need a `texinode` to link to.
> 4. Manipulating Hook Functions doesn't have a `texinode`, so I need to add one.

You don't NEED to, you just happen to want to.  Which is not a bad
thing, but no consensus was reached about the best way to do so.

> 5. When I add a `texinode`, `make install.info` fails to compile if
> there isn't a `menu` in the same page that links to it.

That's not really what's going on.  There doesn't have to be a menu,
but a texinode can't stand by itself, it has to be linked into the
hierarchy of nodes in the document, which means that not only does it
need to reference other nodes, but other nodes need to be updated to
reference it.

> 6. Hence, I need to update the menu structure of User Contrib.

Per our previous discussion ... you chose to restructure things to
have a menu.  I had only pointed out that the new section you at first
created didn't have introductory text after the manner of similar
sections elsewhere.  Nobody weighed in on preference for either one.
There are less radical changes that could be made than restructuring,
and if restructuring is the right thing, it could be done in a
separate patch.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Daniel Shahaf]

Marlon Richert wrote on Fri, Jun 25, 2021 at 00:48:16 +0300:
> On Thu, Jun 24, 2021 at 9:30 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Marlon Richert wrote on Thu, Jun 24, 2021 at 13:34:00 +0300:
> > > On Thu, Jun 24, 2021 at 1:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > >
> > > > Marlon Richert wrote on Wed, 23 Jun 2021 20:30 +00:00:
> > > > > foo implements a perfectly fine zle-line-init widget. It obeys the
> > > > > contract laid out at
> > > > > https://zsh.sourceforge.io/Doc/Release/Zsh-Line-Editor.html#Special-Widgets,
> > > > > which says nothing about return values.
> > > >
> > > > That has two possible interpretations:
> > > >
> > > > - Hook functions may set $? however they want
> > > >
> > > > - Hook functions should set $? to zero on success and to non-zero
> > > >   otherwise
> > > >
> > > > In favour of the first interpretation is that the manual's language
> > > > doesn't explicitly rule it out and compatibility with hook functions
> > > > that predate add-zle-hook-widget and don't set $? as
> > > > add-zle-hook-widget expects.
> > > >
> > > > In favour of the second interpretation is that those semantics are the
> > > > default, ubiquitous, baked into the language's syntax, and extensible
> > > > (same thing as "This bit must be zero" in wire protocol specifications).
> > > >
> > > > I'm voting for the second interpretation.
> > > >
> > > > I suppose we could've added something to NEWS/README when add-zle-hook-widget
> > > > was introduced, if only because this is an interoperability issue (foo
> > > > and bar may be have separate maintainers).
> > >
> > > I think this should be mentioned permanently in the manual, exactly
> > > because we see foo and bar from separate maintainers in the wild.
> >
> > I'm ambivalent about this.
> >
> > On the one hand, as mentioned, "set $? correctly" is a ground rule that
> > shouldn't need to be made explicit everywhere it matters.  I don't want
> > to create an expectation that where the manual _doesn't_ explicitly
> > specify that $? should be set correctly, $?'s value doesn't matter.
> >
> > On the one hand, someone who uses «zle -N zle-line-init foo» may not be
> > aware of a-z-h-w and of the fact that the return value does have an
> > effect.
> >
> > In balance, I think I would rather see it documented globally that $?
> > should always be set to zero/non-zero unless explicitly specified
> > otherwise.  Makes sense?
> 
> That makes sense, but then, it should also be documented what that
> actually means. If your function returns zero/non-zero, what is
> expected to happen? Does this always happen? Or are there different
> classes of functions with different results for zero/non-zero?
> 
> I can already think of two examples that contradict each other:
> * In hook functions, callee returning NON-zero causes the caller to
> refrain from calling more functions.
> * In completion functions, callee returning ZERO causes the caller to
> refrain from calling more functions.
> 
> Those two behave exactly opposite of each other.
> 
> My point being, I think this is best spelled out everywhere.
> Zero/non-zero don't always appear to mean the same thing between
> caller and callee. Clearly it matters, but it doesn't appear to always
> matter in the same way.
> 

I think it's misleading to look at the "caller and callee" system.  They
aren't coupled.  The caller is written to work with any callee that
follows the standard return value semantics, and the callee is written
to work with any caller that expects the standard return value
semantics.

Some callers have a list of callees; some don't.  Some callers who have
a list of callees will call them all; some will call them until the
first success; some will call them until the first failure.  The callee
neither knows nor cares.  The callee simply takes care to return
non-zero whenever it fails, and zero whenever it succeeds.  That's
implied.

What isn't implied is which semantics the caller follows.  The caller
(e.g., add-zsh-hook, add-zle-hook-widget) should document that.

> > > > > Should that `|| return` be removed?
> > > > No, because that would break the case of _deliberately_ returning non-zero
> > > > from one a-z-h-w hook to prevent further a-z-h-w hooks from running.
> > > > Granted, that's not a documented promise, but there's no reason to break
> > > > it, either.
> > >
> > > For hooks added through azhw, yes. But hooks added through zle -N
> > > zle-<hook> have no reason to expect that their return value has any
> > > effect.
> > >
> >
> > I did give some reasons in my previous reply; and compare how drivers
> > should use their turn indicators even when they don't see anyone on the
> > road.
> 
> That's because it's convention everywhere that you signal in the
> direction to which you turn. However, what convention we're supposed
> to follow for return values in Zsh is not quite as clear. See my
> contradictory examples above.
> 

My point was that drivers signal even when there's no one else on the
road.  The analogy to that is that functions should return zero for
success and non-zero otherwise even when they don't know what their
caller would do with that.

> > > How about if azhw would ignore the return value _only_ of any
> > > pre-existing zle -N zle-X widget? For example, this part of the azhw
> > > code could be modified to wrap the original widget function in a
> > > function that always returns zero:
> >
> > At this point, given that the current behaviour has appeared in stable
> > releases for 4.5 years now and is _a priori_ preferable, I think it's
> > better to document the current behaviour and move on.  So, basically, an
> > addition to the incompatible changes section in README (for 5.9) that
> > describes the change in 5.3.  WDYT?
> 
> I'm fine with that. That documentation needs to be findable, though,
> from the docs for both zle -N zle-X and azhw.

Perhaps.  The manual and NEWS/README have different purposes.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Marlon Richert]

On Fri, Jun 25, 2021 at 2:52 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> I think it's misleading to look at the "caller and callee" system.  They
> aren't coupled.  The caller is written to work with any callee that
> follows the standard return value semantics, and the callee is written
> to work with any caller that expects the standard return value
> semantics.
>
> Some callers have a list of callees; some don't.  Some callers who have
> a list of callees will call them all; some will call them until the
> first success; some will call them until the first failure.  The callee
> neither knows nor cares.  The callee simply takes care to return
> non-zero whenever it fails, and zero whenever it succeeds.  That's
> implied.
>
> What isn't implied is which semantics the caller follows.  The caller
> (e.g., add-zsh-hook, add-zle-hook-widget) should document that.
>
> My point was that drivers signal even when there's no one else on the
> road.  The analogy to that is that functions should return zero for
> success and non-zero otherwise even when they don't know what their
> caller would do with that.

It seems I misunderstood your point/metaphor. Yes, I agree with all of
the above.


> Perhaps.  The manual and NEWS/README have different purposes.

I'm reasonably sure that NEWS/README has much lower visibility than
the manual. If we actually want 3rd-party script devs to implement
things correctly, then it needs to be in the manual.


Here's a new version of the patch. I added a note about return values
under ZLE > Hook Widgets.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Bart Schaefer]

On Thu, Jun 24, 2021 at 2:49 PM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> That makes sense, but then, it should also be documented what that
> actually means. If your function returns zero/non-zero, what is
> expected to happen? Does this always happen? Or are there different
> classes of functions with different results for zero/non-zero?

The convention is that returning zero means success (the canonical
example being the "true" command) and nonzero means some kind of
failure ("false").  The actual numeric value of the non-zero return
might mean something.  Theoretically every command (function, builtin,
script, whatever) that "succeeds" should be returning zero; the fact
that this is sometimes ignored (and that for example the return/exit
status of the last command executed within the function body is
retained as $? instead) is programmer laziness.

> I can already think of two examples that contradict each other:
> * In hook functions, callee returning NON-zero causes the caller to
> refrain from calling more functions.
> * In completion functions, callee returning ZERO causes the caller to
> refrain from calling more functions.

These are not really contradictory; you're ignoring important context.
In both cases zero means success and nonzero means failure.  The
context determines what that signifies.

For completion functions, success means that completions have been
generated and further work is not necessary, failure means it's
reasonable to try a different approach.

For hook functions, success means that it's OK to proceed, and failure
means that something bad happened and further work is potentially
dangerous.

You can't just look at the return codes in isolation and say "these
are reversed" without considering what success or failure means in
context.

In either case we could actually extend the model to use different
nonzero values of $? to distinguish "try something else" and "danger,
abort" conditions; it just happens that for hooks, currently anything
nonzero is considered "danger, abort" and for completion it is always
"try something else".

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Marlon Richert]

On 26 Jun 2021, at 17:52, Bart Schaefer <schaefer@brasslantern.com> wrote:
> 
> On Sat, Jun 26, 2021 at 6:14 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>> However, my understanding of Yodl is limited. Is there a simpler way to do this?

Did you notice I asked a question? You left it out of your reply.

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Bart Schaefer]

On Sun, Jun 27, 2021 at 4:19 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> On 26 Jun 2021, at 17:52, Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > On Sat, Jun 26, 2021 at 6:14 AM Marlon Richert <marlon.richert@gmail.com> wrote:
> >> However, my understanding of Yodl is limited. Is there a simpler way to do this?
>
> Did you notice I asked a question? You left it out of your reply.

Sorry, I thought it was implicit that the simpler way is to not do it at all.

"Typically the way to do this would be to include a link to User
Contrib within text that says something like 'see Manipulating Hook
Functions in noderef(...)'."

Re: Does add-zle-hook-widget violate the contract of ZLE hook widgets?

[Marlon Richert]

> On 29 Jun 2021, at 05:06, Bart Schaefer <schaefer@brasslantern.com> wrote:
> 
> On Sun, Jun 27, 2021 at 4:19 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>> 
>>> On 26 Jun 2021, at 17:52, Bart Schaefer <schaefer@brasslantern.com> wrote:
>>> 
>>> On Sat, Jun 26, 2021 at 6:14 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>>>> However, my understanding of Yodl is limited. Is there a simpler way to do this?
>> 
>> Did you notice I asked a question? You left it out of your reply.
> 
> Sorry, I thought it was implicit that the simpler way is to not do it at all.
> 
> "Typically the way to do this would be to include a link to User
> Contrib within text that says something like 'see Manipulating Hook
> Functions in noderef(...)'."

OK, I see. However, since there is also an HTML version of the manual, I think adding a direct link makes a lot more sense. Also, as stated before, having an empty Utilities section makes no sense to me; documents shouldn’t have empty sections. Plus, having the subsections of Utilities listed in the menu would both make it easier to browse that page and make it easier to realize they even exist on that page in the first place. But feel free to split these two things into separate commits. I don’t think this greatly affects the text I’ve written.