zsh-workers
 help / color / mirror / code / Atom feed
* Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
@ 2022-11-16 15:06 Marlon Richert
  2022-11-16 18:24 ` Bart Schaefer
  2023-01-17  0:01 ` [PATCH] " Bart Schaefer
  0 siblings, 2 replies; 9+ messages in thread
From: Marlon Richert @ 2022-11-16 15:06 UTC (permalink / raw)
  To: Zsh hackers list

Currently, when the ZLE calls a widget set with `zle -Fw <widget>`,
this changes the value of $LASTWIDGET. This in turn breaks several of
the widgets listed in the manual under User Contributions. For
example:

% zsh -f
% autoload -Uz copy-earlier-word
% zle -N copy-earlier-word
% bindkey '\e,' copy-earlier-word
% # Pressing ^[, multiple times at this point successfully cycles
through the current words on the command line.
% handler() { local fd=$1; zle -F $fd; exec {fd}<&- }
% zle-line-pre-redraw() { local fd; exec {fd}< <( print ); zle -Fw $fd handler }
% zle -N handler
% zle -N zle-line-pre-redraw
% # Now ^[, still copies the last word on the line, but can no longer
cycle to previous words when pressed again.


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

* Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2022-11-16 15:06 Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET Marlon Richert
@ 2022-11-16 18:24 ` Bart Schaefer
  2022-11-17 13:04   ` Marlon Richert
  2023-01-17  0:01 ` [PATCH] " Bart Schaefer
  1 sibling, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2022-11-16 18:24 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On Wed, Nov 16, 2022 at 7:08 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> Currently, when the ZLE calls a widget set with `zle -Fw <widget>`,
> this changes the value of $LASTWIDGET.

Hrm, normally (running "zle WIDGET ..." explicitly) this would be
skipped (unless "zle WIDGET -w ...").  Too bad -w was overloaded in
this way.  It's not technically overloading because it's used in
different contexts, but mnemonically at least it could be confusing
that "zle -Fw FD WIDGET" and "zle WIDGET -w" would seem to have
inverted meanings of -w if we were to change the default behavior of
-F.

Of course it's also "-Fw" as a single "option", not "-F -w", so we
can't even do something like "zle -F +w" to indicate the desired
behavior.


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

* Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2022-11-16 18:24 ` Bart Schaefer
@ 2022-11-17 13:04   ` Marlon Richert
  2022-11-17 13:28     ` Roman Perepelitsa
  2022-11-17 16:18     ` Bart Schaefer
  0 siblings, 2 replies; 9+ messages in thread
From: Marlon Richert @ 2022-11-17 13:04 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

The -w in `zle -Fw arg` is not the inverse of the -w in `zle -w arg`.
It has a different purpose:
* In `zle -F callback`, callback is a function and gets called as such.
* In `zle -Fw callback`, callback is a widget and gets called as such.

The latter form is necessary if you want to be able to call other ZLE
widgets from the callback or access other ZLE functionality. You
cannot do that with the first form.

It's not the "inverted meaning" here that is the bug. `zle -F` (with
or without -w) just shouldn't ever change $LASTWIDGET in the first
place. It breaks a lot of existing widget functions and there is no
possible workaround.

On Wed, Nov 16, 2022 at 8:25 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Wed, Nov 16, 2022 at 7:08 AM Marlon Richert <marlon.richert@gmail.com> wrote:
> >
> > Currently, when the ZLE calls a widget set with `zle -Fw <widget>`,
> > this changes the value of $LASTWIDGET.
>
> Hrm, normally (running "zle WIDGET ..." explicitly) this would be
> skipped (unless "zle WIDGET -w ...").  Too bad -w was overloaded in
> this way.  It's not technically overloading because it's used in
> different contexts, but mnemonically at least it could be confusing
> that "zle -Fw FD WIDGET" and "zle WIDGET -w" would seem to have
> inverted meanings of -w if we were to change the default behavior of
> -F.
>
> Of course it's also "-Fw" as a single "option", not "-F -w", so we
> can't even do something like "zle -F +w" to indicate the desired
> behavior.


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

* Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2022-11-17 13:04   ` Marlon Richert
@ 2022-11-17 13:28     ` Roman Perepelitsa
  2022-11-17 16:18     ` Bart Schaefer
  1 sibling, 0 replies; 9+ messages in thread
From: Roman Perepelitsa @ 2022-11-17 13:28 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Bart Schaefer, Zsh hackers list

On Thu, Nov 17, 2022 at 2:06 PM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> It's not the "inverted meaning" here that is the bug. `zle -F` (with
> or without -w) just shouldn't ever change $LASTWIDGET in the first
> place. It breaks a lot of existing widget functions and there is no
> possible workaround.

Relevant: https://www.zsh.org/mla/workers/2019/msg00204.html

Especially this part:

> The same branch also adds -W option to zle widget command. This
> option instructs zle to keep LASTWIDGET unchanged.

This part of the patch wasn't applied and the code is long gone (I
didn't attach it to the email) but it's fairly straightforward to
implement.

Roman.


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

* Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2022-11-17 13:04   ` Marlon Richert
  2022-11-17 13:28     ` Roman Perepelitsa
@ 2022-11-17 16:18     ` Bart Schaefer
  2023-01-11  7:45       ` Marlon Richert
  1 sibling, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2022-11-17 16:18 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On Thu, Nov 17, 2022 at 5:05 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> The -w in `zle -Fw arg` is not the inverse of the -w in `zle -w arg`.
> It has a different purpose:

I KNOW that.  I was just pointing out potential future confusion.


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

* Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2022-11-17 16:18     ` Bart Schaefer
@ 2023-01-11  7:45       ` Marlon Richert
  0 siblings, 0 replies; 9+ messages in thread
From: Marlon Richert @ 2023-01-11  7:45 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On Thu, Nov 17, 2022 at 6:18 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Thu, Nov 17, 2022 at 5:05 AM Marlon Richert <marlon.richert@gmail.com> wrote:
> >
> > The -w in `zle -Fw arg` is not the inverse of the -w in `zle -w arg`.
> > It has a different purpose:
>
> I KNOW that.  I was just pointing out potential future confusion.

Aha. Sorry, that wasn't clear to me.


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

* [PATCH] Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2022-11-16 15:06 Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET Marlon Richert
  2022-11-16 18:24 ` Bart Schaefer
@ 2023-01-17  0:01 ` Bart Schaefer
  2023-01-17  9:22   ` Peter Stephenson
  1 sibling, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2023-01-17  0:01 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

Is this good enough, or is there a need to consider that the handler
might delete the previous last widget?  What does/should happen in
that case (or when a widget deletes itself, for that matter)?

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 686c6f5b4..cfa0a739d 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -737,6 +737,7 @@ raw_getbyte(long do_keytmout, char *cptr, int full)
             ) {
             /* Handle the fd. */
             char *fdbuf;
+            Thingy save_lbindk = lbindk;
             {
                 char buf[BDIGBUFSIZE];
                 convbase(buf, lwatch_fd->fd, 10);
@@ -779,6 +780,7 @@ raw_getbyte(long do_keytmout, char *cptr, int full)
                  */
                 errtry = 1;
             }
+            lbindk = save_lbindk;
             }
         }
         /* Function may have invalidated the display. */


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

* Re: [PATCH] Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2023-01-17  0:01 ` [PATCH] " Bart Schaefer
@ 2023-01-17  9:22   ` Peter Stephenson
  2023-01-17 18:00     ` Bart Schaefer
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2023-01-17  9:22 UTC (permalink / raw)
  To: Zsh hackers list

> On 17/01/2023 00:01 Bart Schaefer <schaefer@brasslantern.com> wrote:
> Is this good enough, or is there a need to consider that the handler
> might delete the previous last widget?  What does/should happen in
> that case (or when a widget deletes itself, for that matter)?

I can't think of a good way of handling that without a fair measure
of complexity both in the shell and for the user.  I think just keeping
the old name and letting the user deal with it further is good enough.

pws


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

* Re: [PATCH] Re: Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET
  2023-01-17  9:22   ` Peter Stephenson
@ 2023-01-17 18:00     ` Bart Schaefer
  0 siblings, 0 replies; 9+ messages in thread
From: Bart Schaefer @ 2023-01-17 18:00 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Tue, Jan 17, 2023 at 1:23 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > On 17/01/2023 00:01 Bart Schaefer <schaefer@brasslantern.com> wrote:
> > Is this good enough, or is there a need to consider that the handler
> > might delete the previous last widget?
>
> I think just keeping
> the old name and letting the user deal with it further is good enough.

My concern is that the Thingy pointer becomes invalid.  If that won't
happen, nothing further is needed here.  If that COULD happen, then
the implementation of $LASTWIDGET needs to start storing the name
rather than the Thingy.


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

end of thread, other threads:[~2023-01-17 18:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-16 15:06 Bug: Callback to widget set with `zle -Fw <widget>` shouldn't change $LASTWIDGET Marlon Richert
2022-11-16 18:24 ` Bart Schaefer
2022-11-17 13:04   ` Marlon Richert
2022-11-17 13:28     ` Roman Perepelitsa
2022-11-17 16:18     ` Bart Schaefer
2023-01-11  7:45       ` Marlon Richert
2023-01-17  0:01 ` [PATCH] " Bart Schaefer
2023-01-17  9:22   ` Peter Stephenson
2023-01-17 18:00     ` Bart Schaefer

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