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

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

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?

> > > 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