zsh-workers
 help / color / mirror / code / Atom feed
* add-zle-hook-widget and multiple hooks
@ 2020-06-06  8:40 Daniel Shahaf
  2020-06-06 11:58 ` Mikael Magnusson
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Shahaf @ 2020-06-06  8:40 UTC (permalink / raw)
  To: zsh-workers; +Cc: Eric Freese

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

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

[-- Attachment #2: azhw-LASTWIDGET-shell-v1.txt --]
[-- Type: text/plain, Size: 514 bytes --]

diff --git a/Functions/Misc/add-zle-hook-widget b/Functions/Misc/add-zle-hook-widget
index 9cc35496f..f5f9b615d 100644
--- a/Functions/Misc/add-zle-hook-widget
+++ b/Functions/Misc/add-zle-hook-widget
@@ -41,6 +41,7 @@ zstyle zle-hook types ${hooktypes#zle-}
 function azhw:${^hooktypes} {
     local -a hook_widgets
     local hook
+    readonly LAST_NONHOOK_WIDGET=$LASTWIDGET
     # Values of these styles look like number:name
     # and we run them in number order
     zstyle -a $WIDGET widgets hook_widgets

[-- Attachment #3: azhw-LASTWIDGET-C-v1.txt --]
[-- Type: text/plain, Size: 2549 bytes --]

diff --git a/Functions/Misc/add-zle-hook-widget b/Functions/Misc/add-zle-hook-widget
index 9cc35496f..4d8049083 100644
--- a/Functions/Misc/add-zle-hook-widget
+++ b/Functions/Misc/add-zle-hook-widget
@@ -47,9 +47,9 @@ function azhw:${^hooktypes} {
     for hook in "${(@)${(@on)hook_widgets[@]}#<->:}"; do
 	if [[ "$hook" = user:* ]]; then
 	    # Preserve $WIDGET within the renamed widget
-	    zle "$hook" -N -- "$@"
+	    zle "$hook" -f "nolast" -N -- "$@"
 	else
-	    zle "$hook" -Nw -- "$@"
+	    zle "$hook" -f "nolast" -Nw -- "$@"
 	fi || return
     done
     return 0
diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index ce61db27b..7604d5251 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -678,6 +678,7 @@ bin_zle_flags(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 		else if (!strcmp(*flag, "keepsuffix"))
 		    w->flags |= ZLE_KEEPSUFFIX;
 		*/
+		/* "nolast" is used in bin_zle_call */
 	        else if (!strcmp(*flag, "vichange")) {
 		    if (invicmdmode()) {
 			startvichange(-1);
@@ -703,7 +704,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 {
     Thingy t;
     struct modifier modsave = zmod;
-    int ret, saveflag = 0, setbindk = 0, setlbindk, remetafy;
+    int ret, saveflag = 0, setbindk = 0, setlbindk = 0, remetafy;
     char *wname = *args++, *keymap_restore = NULL, *keymap_tmp;
 
     if (!wname)
@@ -727,12 +728,23 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
     while (*args && **args == '-') {
 	char skip_this_arg[2] = "x";
 	char *num;
+	char *flag;
 	if (!args[0][1] || args[0][1] == '-') {
 	    args++;
 	    break;
 	}
 	while (*++(*args)) {
 	    switch (**args) {
+	    case 'f':
+		flag = args[0][1] ? args[0]+1 : args[1];
+		if (strcmp(flag, "nolast")) {
+		    zwarnnam(name, "%s", "'nolast' expected after -f");
+		    if (remetafy)
+			metafy_line();
+		    return 1;
+		}
+		setlbindk = 1;
+		break;
 	    case 'n':
 		num = args[0][1] ? args[0]+1 : args[1];
 		if (!num) {
@@ -787,7 +799,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
      * a vi range to detect a repeated key */
     setbindk = setbindk ||
 	(t->widget && (t->widget->flags & (WIDGET_INT | ZLE_VIOPER)) == WIDGET_INT);
-    setlbindk = t->widget && (t->widget->flags & ZLE_NOLAST) == ZLE_NOLAST;
+    setlbindk |= t->widget && (t->widget->flags & ZLE_NOLAST) == ZLE_NOLAST;
     ret = execzlefunc(t, args, setbindk, setlbindk);
     unrefthingy(t);
     if (saveflag)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: add-zle-hook-widget and multiple hooks
  2020-06-06  8:40 add-zle-hook-widget and multiple hooks Daniel Shahaf
@ 2020-06-06 11:58 ` Mikael Magnusson
  2020-06-08  6:14   ` Daniel Shahaf
  0 siblings, 1 reply; 8+ messages in thread
From: Mikael Magnusson @ 2020-06-06 11:58 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers, Eric Freese

On 6/6/20, Daniel Shahaf <d.s@daniel.shahaf.name> 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).

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


-- 
Mikael Magnusson

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: add-zle-hook-widget and multiple hooks
  2020-06-06 11:58 ` Mikael Magnusson
@ 2020-06-08  6:14   ` Daniel Shahaf
  2020-06-08 17:52     ` Mikael Magnusson
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Shahaf @ 2020-06-08  6:14 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: zsh-workers

[moving Eric to bcc]

Mikael Magnusson wrote on Sat, 06 Jun 2020 13:58 +0200:
> On 6/6/20, Daniel Shahaf <d.s@daniel.shahaf.name> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: add-zle-hook-widget and multiple hooks
  2020-06-08  6:14   ` Daniel Shahaf
@ 2020-06-08 17:52     ` Mikael Magnusson
  2020-06-10 13:34       ` Daniel Shahaf
  0 siblings, 1 reply; 8+ messages in thread
From: Mikael Magnusson @ 2020-06-08 17:52 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 6/8/20, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> [moving Eric to bcc]
>
> Mikael Magnusson wrote on Sat, 06 Jun 2020 13:58 +0200:
>> On 6/6/20, Daniel Shahaf <d.s@daniel.shahaf.name> 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?

(sorry if the following is a bit rambly)

Just seems more consistent to me to have -l and -w, rather than -w and
-f nolast/unrelated/flags. The point of the generic -f is that there
had to be some option to initiate that mode of modifying global(ish)
state without actually calling a widget, and adding 4 options instead
of just one option that did 4 very related things felt weird. In a
sense -f is like -N there, it is its own mode of operation. Eg, you
can say zle -f yank kill and both words are arguments for -f (in a
sense).

In this case you're only adding a single flag and that flag is related
to an existing option, and it is (for now) just a very long way to
spell -l. It doesn't start a separate mode of operation, just modifies
the current operation slightly. It's not like we're short on letters
for options in the zle widgetname -opt namespace either (only 4 are
used, 2 of which are already capital letters). I think if we added
more flags to your -f scheme, the convention would by necessity be -f
nolast -f unrelated, which is inconsistent with the zle -f flag.

-- 
Mikael Magnusson

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: add-zle-hook-widget and multiple hooks
  2020-06-08 17:52     ` Mikael Magnusson
@ 2020-06-10 13:34       ` Daniel Shahaf
  2020-06-18 11:42         ` Daniel Shahaf
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Shahaf @ 2020-06-10 13:34 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: zsh-workers

Mikael Magnusson wrote on Mon, 08 Jun 2020 19:52 +0200:
> On 6/8/20, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > [moving Eric to bcc]
> >
> > Mikael Magnusson wrote on Sat, 06 Jun 2020 13:58 +0200:  
> >> On 6/6/20, Daniel Shahaf <d.s@daniel.shahaf.name> 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?  
> 
> (sorry if the following is a bit rambly)
> 
> Just seems more consistent to me to have -l and -w, rather than -w and
> -f nolast/unrelated/flags.

I can't parse the last word.  In any case, one of the reason I used the
same option letter is that both -f flags set bits of the same bitfield
type.  Even if right now there's no bit that both -f flags set,
I suspect one might be added in the future.  (For example, a use-case for
«zle -f nolast» doesn't seem inconceivable, even if I don't have one on
me.)

> The point of the generic -f is that there
> had to be some option to initiate that mode of modifying global(ish)
> state without actually calling a widget, and adding 4 options instead
> of just one option that did 4 very related things felt weird. In a
> sense -f is like -N there, it is its own mode of operation. Eg, you
> can say zle -f yank kill and both words are arguments for -f (in a
> sense).

Yes, «zle -f» is a subcommand (like «git foo»).  All zle subcommands are
named as option flags, except for the "invoke a widget" subcommand which
is anonymous.

> In this case you're only adding a single flag and that flag is related
> to an existing option, and it is (for now) just a very long way to
> spell -l. It doesn't start a separate mode of operation, just modifies
> the current operation slightly.

That's what option flags usually do.  For example, that's what the -l
flag to ls(1) does.

> It's not like we're short on letters for options in the zle widgetname
> -opt namespace either (only 4 are used, 2 of which are already capital
> letters).

The difference will come once we support both «-f nolast» and «-f foo».
Then each such foo we add will save an option letter, and the API will
be easier to remember due to using more meaningful names.

> I think if we added more flags to your -f scheme, the
> convention would by necessity be -f nolast -f unrelated, which is
> inconsistent with the zle -f flag.

They could also be «-f nolast:unrelated», just like the $PATH envvar in
relation to the $path array.  We could even teach the toplevel -f to
support this syntax alongside the existing one for consistency, or teach
it to ignore flags called "-f" in its positional arguments, or add a «-f
foo» value that's an alias of -w, etc..

Over here the jury's still out.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: add-zle-hook-widget and multiple hooks
  2020-06-10 13:34       ` Daniel Shahaf
@ 2020-06-18 11:42         ` Daniel Shahaf
  2020-06-27  2:54           ` Daniel Shahaf
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Shahaf @ 2020-06-18 11:42 UTC (permalink / raw)
  To: zsh-workers

Daniel Shahaf wrote on Wed, 10 Jun 2020 13:34 +0000:
> Over here the jury's still out.

In the absence of further feedback I've gone ahead completed the C patch
using the originally posted syntax.  Any comments, reservations, etc.,
speak up.

diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo
index 0909cd4f5..6b0f99b63 100644
--- a/Doc/Zsh/contrib.yo
+++ b/Doc/Zsh/contrib.yo
@@ -347,7 +347,7 @@ 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.
 Widgets are invoked in the order they were added, with
-example(tt(zle )var(widgetname)tt( -Nw -- "$@"))
+example(tt(zle )var(widgetname)tt( -Nw -f "nolast" -- "$@"))
 
 vindex(WIDGET, in hooks)
 Note that this means that the `tt(WIDGET)' special parameter tracks the
diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo
index c928b8ca2..5b84b3acf 100644
--- a/Doc/Zsh/zle.yo
+++ b/Doc/Zsh/zle.yo
@@ -415,7 +415,7 @@ xitem(tt(zle) tt(-K) var(keymap))
 xitem(tt(zle) tt(-F) [ tt(-L) | tt(-w) ] [ var(fd) [ var(handler) ] ])
 xitem(tt(zle) tt(-I))
 xitem(tt(zle) tt(-T) [ tt(tc) var(function) | tt(-r) tt(tc) | tt(-L) ] )
-item(tt(zle) var(widget) [ tt(-n) var(num) ] [ tt(-Nw) ] [ tt(-K) var(keymap) ] var(args) ...)(
+item(tt(zle) var(widget) [ tt(-n) var(num) ] [ tt(-f) var(flag) ] [ tt(-Nw) ] [ tt(-K) var(keymap) ] var(args) ...)(
 The tt(zle) builtin performs a number of different actions concerning
 ZLE.
 
@@ -683,7 +683,7 @@ optional argument for debugging or testing.  Note that this
 transformation is not applied to other non-printing characters such as
 carriage returns and newlines.
 )
-item(var(widget) [ tt(-n) var(num) ] [ tt(-Nw) ] [ tt(-K) var(keymap) ] var(args) ...)(
+item(var(widget) [ tt(-n) var(num) ] [ tt(-f) var(flag) ] [ tt(-Nw) ] [ tt(-K) var(keymap) ] var(args) ...)(
 Invoke the specified var(widget).  This can only be done when ZLE is
 active; normally this will be within a user-defined widget.
 
@@ -702,6 +702,9 @@ appears as if the top-level widget called by the user were still
 active.  With the option tt(-w), tt(WIDGET) and related parameters are set
 to reflect the widget being executed by the tt(zle) call.
 
+Normally, when var(widget) returns the special parameter tt(LASTWIDGET) will
+point to it.  This can be inhibited by passing the option tt(-f nolast).
+
 Any further arguments will be passed to the widget; note that as
 standard argument handling is performed, any general argument list
 should be preceded by tt(-)tt(-).  If it is a shell
diff --git a/Functions/Misc/add-zle-hook-widget b/Functions/Misc/add-zle-hook-widget
index 9cc35496f..4d8049083 100644
--- a/Functions/Misc/add-zle-hook-widget
+++ b/Functions/Misc/add-zle-hook-widget
@@ -47,9 +47,9 @@ function azhw:${^hooktypes} {
     for hook in "${(@)${(@on)hook_widgets[@]}#<->:}"; do
 	if [[ "$hook" = user:* ]]; then
 	    # Preserve $WIDGET within the renamed widget
-	    zle "$hook" -N -- "$@"
+	    zle "$hook" -f "nolast" -N -- "$@"
 	else
-	    zle "$hook" -Nw -- "$@"
+	    zle "$hook" -f "nolast" -Nw -- "$@"
 	fi || return
     done
     return 0
diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index ce61db27b..63bd45c8d 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -678,6 +678,7 @@ bin_zle_flags(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 		else if (!strcmp(*flag, "keepsuffix"))
 		    w->flags |= ZLE_KEEPSUFFIX;
 		*/
+		/* If you add magic strings here, be consistent with bin_zle_call() */
 	        else if (!strcmp(*flag, "vichange")) {
 		    if (invicmdmode()) {
 			startvichange(-1);
@@ -703,7 +704,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 {
     Thingy t;
     struct modifier modsave = zmod;
-    int ret, saveflag = 0, setbindk = 0, setlbindk, remetafy;
+    int ret, saveflag = 0, setbindk = 0, setlbindk = 0, remetafy;
     char *wname = *args++, *keymap_restore = NULL, *keymap_tmp;
 
     if (!wname)
@@ -727,12 +728,24 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
     while (*args && **args == '-') {
 	char skip_this_arg[2] = "x";
 	char *num;
+	char *flag;
 	if (!args[0][1] || args[0][1] == '-') {
 	    args++;
 	    break;
 	}
 	while (*++(*args)) {
 	    switch (**args) {
+	    case 'f':
+		flag = args[0][1] ? args[0]+1 : args[1];
+		if (strcmp(flag, "nolast")) {
+		    zwarnnam(name, "%s", "'nolast' expected after -f");
+		    if (remetafy)
+			metafy_line();
+		    return 1;
+		}
+		/* If you add magic strings here, be consistent with bin_zle_flags() */
+		setlbindk = 1;
+		break;
 	    case 'n':
 		num = args[0][1] ? args[0]+1 : args[1];
 		if (!num) {
@@ -787,7 +800,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
      * a vi range to detect a repeated key */
     setbindk = setbindk ||
 	(t->widget && (t->widget->flags & (WIDGET_INT | ZLE_VIOPER)) == WIDGET_INT);
-    setlbindk = t->widget && (t->widget->flags & ZLE_NOLAST) == ZLE_NOLAST;
+    setlbindk |= t->widget && (t->widget->flags & ZLE_NOLAST) == ZLE_NOLAST;
     ret = execzlefunc(t, args, setbindk, setlbindk);
     unrefthingy(t);
     if (saveflag)
diff --git a/Test/X04zlehighlight.ztst b/Test/X04zlehighlight.ztst
index 475a2e309..dadd1c0aa 100644
--- a/Test/X04zlehighlight.ztst
+++ b/Test/X04zlehighlight.ztst
@@ -154,6 +154,17 @@
 0:overlapping region_highlight with near-color (hex-triplets at input)
 >0m27m24mCDE|340|tCDE|3160|rCDE|39|CDE|340|ueCDE|39|
 
+  zpty_start
+  zpty_input 'f () { zle clear-screen; zle g -f nolast; BUFFER=": ${(q)LASTWIDGET}" }; zle -N f'
+  zpty_input 'g () { }; zle -N g'
+  zpty_input 'bindkey "\C-a" f'
+  zpty_enable_zle
+  zpty_input $'\C-a'
+  zpty_line 1 p
+  zpty_stop
+0:zle $widgetname -f nolast
+>0m27m24m0m27m24m: clear-screen
+
 %clean
 
   zmodload -ui zsh/zpty

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: add-zle-hook-widget and multiple hooks
  2020-06-18 11:42         ` Daniel Shahaf
@ 2020-06-27  2:54           ` Daniel Shahaf
  2020-06-27  3:02             ` Daniel Shahaf
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Shahaf @ 2020-06-27  2:54 UTC (permalink / raw)
  To: zsh-workers

Daniel Shahaf wrote on Thu, 18 Jun 2020 11:42 +0000:
> Daniel Shahaf wrote on Wed, 10 Jun 2020 13:34 +0000:
> > Over here the jury's still out.  
> 
> In the absence of further feedback I've gone ahead completed the C patch
> using the originally posted syntax.  Any comments, reservations, etc.,
> speak up.

I missed some boilerplate in the argument parsing in C; interdiff:

diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index 63bd45c8d..cd3f2c356 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -737,12 +737,14 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 	    switch (**args) {
 	    case 'f':
 		flag = args[0][1] ? args[0]+1 : args[1];
-		if (strcmp(flag, "nolast")) {
+		if (flag == NULL || strcmp(flag, "nolast")) {
 		    zwarnnam(name, "%s", "'nolast' expected after -f");
 		    if (remetafy)
 			metafy_line();
 		    return 1;
 		}
+		if (!args[0][1])
+		    *++args = skip_this_arg;
 		/* If you add magic strings here, be consistent with bin_zle_flags() */
 		setlbindk = 1;
 		break;

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: add-zle-hook-widget and multiple hooks
  2020-06-27  2:54           ` Daniel Shahaf
@ 2020-06-27  3:02             ` Daniel Shahaf
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Shahaf @ 2020-06-27  3:02 UTC (permalink / raw)
  To: zsh-workers

One more thing about the choice of syntax: I don't think it'd be a good
API for -w to mean "Set $WIDGET" and for -l to mean "Don't set
$LASTWIDGET".  Nobody will remember which of the two is the "Do" and
which is the "Don't".

With -f we can have longer, descriptive names.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-06-27  3:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-06  8:40 add-zle-hook-widget and multiple hooks Daniel Shahaf
2020-06-06 11:58 ` Mikael Magnusson
2020-06-08  6:14   ` Daniel Shahaf
2020-06-08 17:52     ` Mikael Magnusson
2020-06-10 13:34       ` Daniel Shahaf
2020-06-18 11:42         ` Daniel Shahaf
2020-06-27  2:54           ` Daniel Shahaf
2020-06-27  3:02             ` Daniel Shahaf

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).