Re: add-zle-hook-widget and multiple hooks

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

[moving Eric to bcc]

Mikael Magnusson wrote on Sat, 06 Jun 2020 13:58 +0200:
> On 6/6/20, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > When two or more zle-line-pre-redraw hooks are registered using
> > add-zle-hook-widget, the value of $LASTWIDGET when the each hook is
> > called is the name of the former hook:
> >
> >     [Eric Freese wrote in
> > https://github.com/zsh-users/zsh-autosuggestions/issues/529#issuecomment-632113840]
> >     $ zsh -df
> >     % autoload add-zle-hook-widget
> >     % f() {}
> >     % g() { zle -M "$(typeset -p LASTWIDGET)" }
> >     % add-zle-hook-widget line-pre-redraw f
> >     % add-zle-hook-widget line-pre-redraw g
> >     % x<CURSOR>
> >     typeset -r LASTWIDGET=f
> >
> > The issue here is that g would like to to know what widget was invoked
> > immediately before the redraw.  In the example, that'd be self-insert.
> >
> > I've attached two proofs of concept.  WDYT?
> >
> > I'll add docs, etc, once an approach is chosen.
> >
> > Cheers,
> >
> > Daniel
> >
> > P.S.  For the latter patch, note that «zle $widget -f» is distinct from
> > «zle -f».  
> 
> I think the warning message when -f is not followed by nolast should
> be phrased in the same way it would if there were other valid flags,
> since it would have to change when more are added anyway (we will
> probably not want to enumerate all possible flags in this warning
> message).

Thanks for the review.

If we add more flags that are too many to list, then we should change
the error message, yes; nevertheless, _right now_ only one flag is
supported, so the error message might as well say that.  That'd be
a feature, not a bug.  Compare:

% ssh $foo svn info --show-item=dept 
svn: E205000: 'dept' is not a valid value for --show-item; did you mean 'depth'?
% ssh $bar svn info --show-item=dept 
svn: E205000: 'dept' is not a valid value for --show-item

When I'm on $bar and get that error message, I know not to bother
trying the correct spelling because it won't work.  Same here:  If 5.9
supports -f nolast, 5.10 supports -f somethingelse as well, and then
somebody tries -f somethingelse in 5.9, a generic error message will
be less helpful to them than a specific one.

> Also, I think rather than reusing the concept of the -f option, it
> would be better to use another flag (maybe -l for LASTWIDGET) which is
> analogous to the existing -w option:

_Why_ would that be better?

>     Normally, calling a widget in this way does not set the special
>     parameter  WIDGET  and related parameters, so that the environ‐
>     ment appears as if the top-level widget called by the user were
>     still  active.   With the option -w, WIDGET and related parame‐
>     ters are set to reflect the widget being executed  by  the  zle
>     call.

Cheers,

Daniel