zsh-workers
 help / color / mirror / code / Atom feed
* Prompt expansion on signals
@ 2021-11-28  9:59 Roman Perepelitsa
  2021-11-28 19:50 ` Bart Schaefer
  0 siblings, 1 reply; 8+ messages in thread
From: Roman Perepelitsa @ 2021-11-28  9:59 UTC (permalink / raw)
  To: Zsh hackers list

Consider the following user configuration:

    setopt prompt_subst
    my_var='%#'
    PS1='$my_var '

For this to work correctly, prompt_subst must be set when prompt is
expanded. However, this requirement can be violated when prompt is
expanded in a signal handler. One example where this happens is upon
the completion of a background job.

    % sleep 1 &
    [1] 3566
    %
    [1]  + done       sleep 1
    %

In this example all went well -- the job completed while zle was idle,
so prompt got expanded with correct options.

If the background job completes while zle is running a function,
prompt gets expanded with the current options of the function.

    % zle-line-init() { emulate -LR zsh; while (($#jobstates)) : }
    % zle -N zle-line-init
    % sleep 1 &
    [1] 3592
    %
    [1]  + done       sleep 1
    $my_var

Note "$my_var" as the last prompt instead of the expected "%".

The same issue arises when the terminal is resized. Here prompt gets
expanded on SIGWINCH with whatever options are in effect.

    % typeset -p LINES
    typeset -i10 LINES=46
    % zle-line-init() { emulate -LR zsh; while ((LINES > 24)) =true }
    $my_var typeset -p LINES
    typeset -i10 LINES=22

Here I redefined zle-line-init while my terminal was 46 lines tall and
then I resized it to 22 lines (more specifically, I split it). Note
"$my_var" in prompt. Also note =true in the loop. It's needed because
zsh postpones the processing of SIGWINCH until the next fork or until
zle becomes idle.

Here's an alternative way to trigger the same code without resizing
the terminal:

    % zle-line-init() { emulate -LR zsh; LINES=0 }
    $my_var

This approach is the easiest to test with.

It appears that one is at risk of prompt expansion with incorrect
options if both of the following conditions are met:

1. Using zle widgets that change options. Using
zsh-syntax-highlighting (just to name a popular plugin) qualifies.
2. Using background jobs without nonotify or resizing the terminal.

Many (most?) zsh users are affected.

Prompt expansion can be affected by options other than prompt_*. For example:

    setopt prompt_subst ksh_arrays
    my_var=('%#')
    PS1='${my_var[0]} '

Expanding this prompt under `emulate -L zsh` (without -R) will produce
incorrect results.

Expanding prompt with incorrect options can be a minor nuisance (like
in the examples above) or a major issue (when prompt is expanded with
prompt_subst when it's not intended).

Is there a solution for this problem that I'm missing? Are there
workarounds? How could it be fixed?

Roman.


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

* Re: Prompt expansion on signals
  2021-11-28  9:59 Prompt expansion on signals Roman Perepelitsa
@ 2021-11-28 19:50 ` Bart Schaefer
  2021-11-28 20:31   ` Roman Perepelitsa
  0 siblings, 1 reply; 8+ messages in thread
From: Bart Schaefer @ 2021-11-28 19:50 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

On Sun, Nov 28, 2021 at 2:01 AM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> For this to work correctly, prompt_subst must be set when prompt is
> expanded. However, this requirement can be violated when prompt is
> expanded in a signal handler.

A more egregious example might be when the global settings have
"unsetopt promptpercent".

Are prompts the only case where this is an issue, though?  The most
visible, certainly, but "zle -F" for example?

What about the reverse situation, where the signal handler is a
function that changes options locally?

> Prompt expansion can be affected by options other than prompt_*.

Are you aware of any cases where this is true but prompt_subst is false?

> Is there a solution for this problem that I'm missing? Are there
> workarounds? How could it be fixed?

I can't think of any workaround, other than perhaps to create a signal
handler that does "zle -I" ... as to a possible fix, since the prompt
parameters are all specials they could be programmed to memoize the
options in effect at the time they are assigned, similar to the way
functions can have "sticky" options when defined within an "emulate
-c", and restore those before expansion.  That introduces several
other questions:  Can each prompt have different options (e.g., RPS1
differ from PS1)?  If a prompt parameter is assigned within a
function, does it memoize the local options?  If PS1 et al. are
imported from the environment, what options are initially memoized and
when?

Perhaps the prompt options should be excluded from those altered by
"emulate -R"?  "... except for certain options describing the
interactive environment ... only those options likely to cause
portability problems in scripts and functions are altered ..."  Are
promptsubst and promptpercent etc. "likely to cause portability
problems"?  I suppose they are if a "select" statement is used and PS3
has been changed.  Either way that doesn't fix an explicit setopt of
those.


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

* Re: Prompt expansion on signals
  2021-11-28 19:50 ` Bart Schaefer
@ 2021-11-28 20:31   ` Roman Perepelitsa
  2021-11-28 21:10     ` Oliver Kiddle
  2021-11-29  1:38     ` Bart Schaefer
  0 siblings, 2 replies; 8+ messages in thread
From: Roman Perepelitsa @ 2021-11-28 20:31 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On Sun, Nov 28, 2021 at 8:50 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Sun, Nov 28, 2021 at 2:01 AM Roman Perepelitsa
> <roman.perepelitsa@gmail.com> wrote:
> >
> > For this to work correctly, prompt_subst must be set when prompt is
> > expanded. However, this requirement can be violated when prompt is
> > expanded in a signal handler.
>
> A more egregious example might be when the global settings have
> "unsetopt promptpercent".

I can control these in my config. I cannot control options that are
active when zsh processes SIGCHLD though (unless I never change
options in zle widgets).

To clarify, I'd like to:

1. Be able to resize my terminal and run background jobs.
2. Use zle widgets that set local_options and any other options
together with it.
3. Never have my prompt expanded with wrong options.

I don't know how to satisfy all three. If I drop the first point, then
I do know. That is, I have control of shell options in all cases
except when zsh expands prompt in a signal handler.

These requirements look reasonable to me. Isn't it something that
virtually everyone wants? I was surprised when I realized that I
cannot have these things simultaneously (or don't know how).

> Are prompts the only case where this is an issue, though?  The most
> visible, certainly, but "zle -F" for example?

As far as I'm aware, prompt expansion on signals is the only place
where it's impossible to set options. Perhaps I'm misunderstanding
what you mean by "zle -F" in this context?

> What about the reverse situation, where the signal handler is a
> function that changes options locally?

What do you mean?

> > Prompt expansion can be affected by options other than prompt_*.
>
> Are you aware of any cases where this is true but prompt_subst is false?

Hm... multibyte? There are also special parameters like LC_ALL that
cause the same issue. For example, doing the following in a widget can
break prompt if SIGCHLD arrives at the wrong moment:

    local LC_ALL=C
    ...

> Either way that doesn't fix an explicit setopt of those.

I sometimes change prompt_* options in functions when I want to use `print -P`.

    emulate -L zsh -o prompt_percent no_prompt_subst
    print -Pru2 -- '%F{1}error%f: missing required parameter: %F{3}--foo%f'

This seemed smart at the time...

What do you think about not running signal handlers for SIGCHLD and
SIGWINCH while zle widgets are running? Postpone them. This already
sort of happens when a signal arrives during recursive-edit.

Roman.


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

* Re: Prompt expansion on signals
  2021-11-28 20:31   ` Roman Perepelitsa
@ 2021-11-28 21:10     ` Oliver Kiddle
  2021-11-28 21:36       ` Roman Perepelitsa
  2021-11-29  1:38     ` Bart Schaefer
  1 sibling, 1 reply; 8+ messages in thread
From: Oliver Kiddle @ 2021-11-28 21:10 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

Roman Perepelitsa wrote:
> I sometimes change prompt_* options in functions when I want to use `print -P`.
>
>     emulate -L zsh -o prompt_percent no_prompt_subst
>     print -Pru2 -- '%F{1}error%f: missing required parameter: %F{3}--foo%f'

In some cases, you might be better off using the ${(%)var} prompt
expansion. That sets prompt_percent and unsets prompt_subst and
prompt_bang for the expansion. ${(%%)var} uses current options.

> What do you think about not running signal handlers for SIGCHLD and
> SIGWINCH while zle widgets are running? Postpone them. This already
> sort of happens when a signal arrives during recursive-edit.

It isn't necessarily just hooks and signal handlers that are affected.
Someone might want to use emulate in a zle widget directly. If this is a
plugin, and the author uses default prompt options, it mightn't be clear
to them that this could break for other users.

There are some cases in completion too where it'd be useful to restore
the user's option settings for a particular command. Perhaps we could
have an argument to emulate - user or global perhaps - for restoring the
original options.

Making options sticky when PS1, PS2 etc are set might break someone's
setup where they set their prompt before their options.

Oliver


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

* Re: Prompt expansion on signals
  2021-11-28 21:10     ` Oliver Kiddle
@ 2021-11-28 21:36       ` Roman Perepelitsa
  0 siblings, 0 replies; 8+ messages in thread
From: Roman Perepelitsa @ 2021-11-28 21:36 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Zsh hackers list

On Sun, Nov 28, 2021 at 10:10 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> Roman Perepelitsa wrote:
> > I sometimes change prompt_* options in functions when I want to use `print -P`.
> >
> >     emulate -L zsh -o prompt_percent no_prompt_subst
> >     print -Pru2 -- '%F{1}error%f: missing required parameter: %F{3}--foo%f'
>
> In some cases, you might be better off using the ${(%)var} prompt
> expansion.

I do this in a few places. I'll probably need to replace all `print
-P` calls with ${(%)} expansions.

As a last resort I'm also considering removing all `emulate -L zsh`
calls from all my code and not using any functions from third-parties
or shipped with zsh, however awful that sounds.

> It isn't necessarily just hooks and signal handlers that are affected.

What do you mean by hooks? The only two places where I don't know how
to control options is prompt expansion on SIGWINCH and SIGCHLD. Are
there more?

> Someone might want to use emulate in a zle widget directly. If this is a
> plugin, and the author uses default prompt options, it mightn't be clear
> to them that this could break for other users.

What do you mean? Is it the same problem or a different one?

Maybe we are talking about different things. Let's assume I'm in
control of all my rc files and their complete transitive closure. I'm
writing all widgets by hand. Every line of zsh code that executes in
my shell is written by me. Given this assumption, I would like to
avoid prompt expansion with wrong options. Right now the only way I
know how to achieve this is to either give up perf-function options
(via local_options + whatever options I want in the function) or to
avoid triggering SIGWINCH and SIGCHLD (which implies never resizing my
terminal and not using background jobs).

> There are some cases in completion too where it'd be useful to restore
> the user's option settings for a particular command. Perhaps we could
> have an argument to emulate - user or global perhaps - for restoring the
> original options.

Would this help in avoiding prompt expansion with incorrect options?

> Making options sticky when PS1, PS2 etc are set might break someone's
> setup where they set their prompt before their options.

Good point.

Making options sticky also wouldn't avoid the issue where prompt is
expanded with incorrect special parameters (LC_ALL, etc.). Overriding
special parameters as locals in functions is convenient. It's a shame
that it has the side effect of introducing a race that breaks prompt.

Is there a downside to postponing the handling of SIGCHLD and SIGWINCH
until all functions return? Postponing SIGWINCH seems safe but
postponing SIGCHLD might blow up the job table. Is this a serious
concern?

Roman.


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

* Re: Prompt expansion on signals
  2021-11-28 20:31   ` Roman Perepelitsa
  2021-11-28 21:10     ` Oliver Kiddle
@ 2021-11-29  1:38     ` Bart Schaefer
  2021-11-29 11:36       ` Roman Perepelitsa
  1 sibling, 1 reply; 8+ messages in thread
From: Bart Schaefer @ 2021-11-29  1:38 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

On Sun, Nov 28, 2021 at 12:32 PM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> On Sun, Nov 28, 2021 at 8:50 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > On Sun, Nov 28, 2021 at 2:01 AM Roman Perepelitsa
> > <roman.perepelitsa@gmail.com> wrote:
> > >
> > > For this to work correctly, prompt_subst must be set when prompt is
> > > expanded. However, this requirement can be violated when prompt is
> > > expanded in a signal handler.

To clarify this point (in part for Oliver), the question is not what
happens when the signal handler is a zsh function and that function
expands prompts.  The problem occurs when a default (C-implementation)
signal handler causes ZLE to redraw the prompt as a side-effect.  "...
when prompt is expanded in a signal handler" confused me for a while
the first time I read through this.

> > A more egregious example might be when the global settings have
> > "unsetopt promptpercent".
>
> I can control these in my config.

I'm not understanding the distinction ... you can control prompt_subst
in your config as well.  I'm just pointing out another case where a
user might want a particular global config that is messed with by
"emulate -R".

> I cannot control options that are
> active when zsh processes SIGCHLD though (unless I never change
> options in zle widgets).

For SIGCHLD in particular you could try "setopt nonotify" in your zle
widgets.  That doesn't help with SIGWINCH, but it might give us an
idea of whether there's a problem with delaying those signals.

I haven't followed the logic through very thoroughly, but what if
instead of delaying the signal handling, we delayed reprinting the
prompt?  Is there a reason an in-progress widget would want the prompt
redisplayed without an explicit call to reset-prompt?

> > Are prompts the only case where this is an issue, though?  The most
> > visible, certainly, but "zle -F" for example?
>
> As far as I'm aware, prompt expansion on signals is the only place
> where it's impossible to set options. Perhaps I'm misunderstanding
> what you mean by "zle -F" in this context?

Sorry, I got the question sort of inside-out (see above about "in a
signal handler" confusion).  The relevant bit is that, like a zle
widget, a zle -F handler might have changed local options and then be
hit with SIGWINCH.

> > What about the reverse situation, where the signal handler is a
> > function that changes options locally?
>
> What do you mean?

Ignore that, more confusion.


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

* Re: Prompt expansion on signals
  2021-11-29  1:38     ` Bart Schaefer
@ 2021-11-29 11:36       ` Roman Perepelitsa
  2021-11-29 17:42         ` Roman Perepelitsa
  0 siblings, 1 reply; 8+ messages in thread
From: Roman Perepelitsa @ 2021-11-29 11:36 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

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

On Mon, Nov 29, 2021 at 2:38 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> To clarify this point (in part for Oliver), the question is not what
> happens when the signal handler is a zsh function and that function
> expands prompts.  The problem occurs when a default (C-implementation)
> signal handler causes ZLE to redraw the prompt as a side-effect.  "...
> when prompt is expanded in a signal handler" confused me for a while
> the first time I read through this.

Thank you.

What makes WINCH and CHLD special is that the default signal handler
for them cannot be overridden. User traps for these signals run after
the default handlers not instead of them.

> I'm not understanding the distinction ... you can control prompt_subst
> in your config as well.  I'm just pointing out another case where a
> user might want a particular global config that is messed with by
> "emulate -R".

The case you are bringing up can be fixed by adding "-L" to "emulate",
right? A function that unintentionally resets all global options will
cause other issues apart from breaking prompt.

Aside from signals, prompt expansion happens either with global
options or with options that are active during "zle reset-prompt". If
I have control over my rc files, I have control over these options by
following these rules:

1. Set the desired global options in zshrc.
2. All functions that change options must set local_options.
3. Functions that invoke reset-prompt widget (either directly or
indirectly) must not change options even with local_options.

The nice thing about these rules is that they are already being
followed by all decent plugins and all zsh code bundled with zsh
itself. If I wanted to ensure correct prompt expansion on WINCH and
CHLD, the rules would be stricter:

1. Set the desired global options in zshrc.
2. Functions must not change options, not even with local_options.

Now most plugins and most bundled zsh code are ruled out.

> For SIGCHLD in particular you could try "setopt nonotify" in your zle
> widgets.

This has a surprising effect of suppressing the job notification
altogether. Here's how it goes:

1. CHLD arrives when my zle widget is running with local_options and
no_notify. Notification is not printed.
2. I press ENTER. Notification is not printed because now notify is set.
3. If I run "jobs", notification finally gets printed and the job is
removed from the job table.

> I haven't followed the logic through very thoroughly, but what if
> instead of delaying the signal handling, we delayed reprinting the
> prompt?  Is there a reason an in-progress widget would want the prompt
> redisplayed without an explicit call to reset-prompt?

I like this idea. I'm attaching a patch that implements it. Also
available here:
https://github.com/romkatv/zsh/commit/cdcb141c880799847f2b068fef5d097736d484d6

WDYT?

Roman.

[-- Attachment #2: postpone-zle-refresh.txt --]
[-- Type: text/plain, Size: 2717 bytes --]

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 9edf30e01..6a491f8b0 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -83,6 +83,11 @@ int done;
 /**/
 int mark;
 
+/* Zle needs to be refreshed */
+
+/**/
+static int zleneedsrefresh;
+
 /*
  * Status ($?) saved before function entry.  This is the
  * status we need to use in prompts.
@@ -1417,6 +1422,8 @@ execzlefunc(Thingy func, char **args, int set_bindk, int set_lbindk)
     Thingy save_bindk = bindk;
     Thingy save_lbindk = lbindk;
 
+    zleactive++;
+
     if (set_bindk)
 	bindk = func;
     if (zlemetaline) {
@@ -1585,6 +1592,11 @@ execzlefunc(Thingy func, char **args, int set_bindk, int set_lbindk)
     if (isrepeat)
         viinrepeat = !invicmdmode();
 
+    if (--zleactive == 1 && zleneedsrefresh) {
+	zleneedsrefresh = 0;
+	zrefresh();
+		}
+
     return ret;
 }
 
@@ -2143,7 +2155,10 @@ zle_main_entry(int cmd, va_list ap)
 	break;
 
     case ZLE_CMD_REFRESH:
-	zrefresh();
+	if (zleactive > 1)
+	    zleneedsrefresh = 1;
+	else
+	    zrefresh();
 	break;
 
     case ZLE_CMD_SET_KEYMAP:
diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index d9d9503e2..6b2a8d4d5 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -1184,8 +1184,7 @@ zrefresh(void)
 	if (winchanged) {
 	    moveto(0, 0);
 	    t0 = olnct;		/* this is to clear extra lines even when */
-	    winchanged = 0;	/* the terminal cannot TCCLEAREOD	  */
-	    listshown = 0;
+	    listshown = 0;	/* the terminal cannot TCCLEAREOD	  */
 	}
 #endif
 	/* we probably should only have explicitly set attributes */
@@ -1193,9 +1192,10 @@ zrefresh(void)
 	tsetcap(TCSTANDOUTEND, 0);
 	tsetcap(TCUNDERLINEEND, 0);
 	txtattrmask = 0;
-
-	if (trashedzle && !clearflag)
-	    reexpandprompt(); 
+	if ((trashedzle && !clearflag) || winchanged) {
+	    winchanged = 0;
+	    reexpandprompt();
+	}
 	resetvideo();
 	resetneeded = 0;	/* unset */
 	oput_rpmpt = 0;		/* no right-prompt currently on screen */
diff --git a/Src/utils.c b/Src/utils.c
index f9127c70c..0154f3112 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1850,12 +1850,10 @@ mod_export struct ttyinfo shttyinfo;
 /**/
 mod_export int resetneeded;
 
-#ifdef TIOCGWINSZ
 /* window size changed */
 
 /**/
 mod_export int winchanged;
-#endif
 
 static int
 adjustlines(int signalled)
@@ -1982,11 +1980,7 @@ adjustwinsize(int from)
 #endif /* TIOCGWINSZ */
 
     if (zleactive && resetzle) {
-#ifdef TIOCGWINSZ
-	winchanged =
-#endif /* TIOCGWINSZ */
-	    resetneeded = 1;
-	zleentry(ZLE_CMD_RESET_PROMPT);
+	winchanged = resetneeded = 1;
 	zleentry(ZLE_CMD_REFRESH);
     }
 }

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

* Re: Prompt expansion on signals
  2021-11-29 11:36       ` Roman Perepelitsa
@ 2021-11-29 17:42         ` Roman Perepelitsa
  0 siblings, 0 replies; 8+ messages in thread
From: Roman Perepelitsa @ 2021-11-29 17:42 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On Mon, Nov 29, 2021 at 12:36 PM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> I'm attaching a patch that implements it. Also
> available here:
> https://github.com/romkatv/zsh/commit/cdcb141c880799847f2b068fef5d097736d484d6

This works rather badly when a signal arrives during a long-running
widget like history-incremental-search-backward. Given that my patch
delays only zleredisplay but not "job done" messages, we end up in a
sorry state of having no prompt. Moreover, the next typed character
triggers zleredisplay that may cause prompt reexpansion with wrong
options.

Maybe the "job done" message should also be delayed? That would
probably be OK in practice but I'm not sure. And what about WINCH if
it arrives during history-incremental-search-backward? With my patch
it won't be processed until the widget is done. Doesn't seem ideal.

Maybe a different approach? Perhaps a new zle hook -- something like
zle-prompt-pre-expand -- that zle would invoke every time before
expanding prompt. The name of the prompt would be passed as an
argument ("PS1", "RPS1", etc.). The hook could set REPLY scalar (which
would be initially unset) to an alternative prompt value; it could
also set reply array (also initially unset) to the list of options
that zle must apply on top of the current options before expanding
prompt (alternatively, the hook itself could change options if zle
would guarantee that these changes will be reverted after prompt is
expanded). People who now set PS1 in precmd will be able to do that in
zle-prompt-pre-expand. It will allow their prompt-building code to
react to changes in TTY dimensions and background jobs. It would
simplify my zsh theme's implementation by quite a bit.

Roman.


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

end of thread, other threads:[~2021-11-29 17:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-28  9:59 Prompt expansion on signals Roman Perepelitsa
2021-11-28 19:50 ` Bart Schaefer
2021-11-28 20:31   ` Roman Perepelitsa
2021-11-28 21:10     ` Oliver Kiddle
2021-11-28 21:36       ` Roman Perepelitsa
2021-11-29  1:38     ` Bart Schaefer
2021-11-29 11:36       ` Roman Perepelitsa
2021-11-29 17:42         ` Roman Perepelitsa

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