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

[Marlon Richert <marlon.richert@xxxxxxxxx>]

On Thu, Jun 24, 2021 at 9:30 PM Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> 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.

From 9cbc49f44a8e1ffe0ecd37378ed81fc0090ab411 Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlon.richert@xxxxxxxxx>
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)