Re: Prompt expansion on signals

[Roman Perepelitsa <roman.perepelitsa@xxxxxxxxx>]

On Mon, Nov 29, 2021 at 2:38 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> 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.

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);
     }
 }