Bug with the new prompt redraw code

Bug with the new prompt redraw code

[Bart Schaefer]

Consider the following:

  PS1_first_line=$'%{\e[1;32m%}[%d]%{\e[0m%}\n'
  PS1_second_line=$'%{\e[1;34m%}[%T]%{\e[1;32m%}%%%{\e[0m%} '

  precmd() {
    PS1="$PS1_first_line$PS1_second_line"
  }
  zle-line-init() {
    PS1="$PS1_second_line"
  }
  zle -N zle-line-init

This was intended to cause zsh to redraw only the second line of the two-
line prompt at any time other than completely starting over with a new
command.

Instead it has the effect of printing a garbage prompt.  This appears to 
occur because (1) zle-line-init runs before the prompt is displayed, which 
seems odd but wouldn't be a big deal except that (2) the prompt printing 
code has stashed a pointer to the value of $PS1, which ends up pointing at 
reclaimed memory when PS1 is re-assigned.

Re: Bug with the new prompt redraw code

[Peter Stephenson]

Bart Schaefer wrote:
> Instead it has the effect of printing a garbage prompt.  This appears to 
> occur because (1) zle-line-init runs before the prompt is displayed, which 
> seems odd but wouldn't be a big deal except that (2) the prompt printing 
> code has stashed a pointer to the value of $PS1, which ends up pointing at 
> reclaimed memory when PS1 is re-assigned.

It sounds like a general problem with reassigning prompts in zle.

How about passing the address of the prompt variable (the internal one,
i.e. prompt, rprompt, etc., not the parameter) to zle instead of the
prompt itself?  That would have the useful side effect that changes
to the variable were reflected when updating.

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: Bug with the new prompt redraw code

[Bart Schaefer]

On Sun, 11 Jul 2004, Peter Stephenson wrote:

> > (1) zle-line-init runs before the prompt is displayed
> > (2) the prompt printing code has stashed a pointer to the value of 
> > $PS1, which ends up pointing at reclaimed memory
> 
> How about passing the address of the prompt variable (the internal one,
> i.e. prompt, rprompt, etc., not the parameter) to zle instead of the
> prompt itself?

That might do it, but it'd be a moderately extensive change, and it has to 
be done carefully, because some other callers of zleread() pass NULL for 
the rprompt and so on.

Also it doesn't address (1) but then maybe that doesn't matter.

Re: Bug with the new prompt redraw code

[Peter Stephenson]

Bart Schaefer wrote:
> On Sun, 11 Jul 2004, Peter Stephenson wrote:
> > > (1) zle-line-init runs before the prompt is displayed
> > > (2) the prompt printing code has stashed a pointer to the value of 
> > > $PS1, which ends up pointing at reclaimed memory
> > 
> > How about passing the address of the prompt variable (the internal one,
> > i.e. prompt, rprompt, etc., not the parameter) to zle instead of the
> > prompt itself?
> 
> That might do it, but it'd be a moderately extensive change,

It's less than I thought.  That's because the caller either passes the
prompt string unmodified, or a locally defined prompt (in vared), or
NULL, so changing it to an address was relatively straightforward.  So
far.

> and it has to 
> be done carefully, because some other callers of zleread() pass NULL for 
> the rprompt and so on.

In any case, you learn to program defensively round here, since (e.g.) six
years ago somebody relied on an undocumented side effect.

> Also it doesn't address (1) but then maybe that doesn't matter.

I could be convinced it does, but see if this makes the problem go away
first.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.37
diff -u -r1.37 zle.yo
--- Doc/Zsh/zle.yo	2 Jul 2004 15:59:14 -0000	1.37
+++ Doc/Zsh/zle.yo	11 Jul 2004 22:34:21 -0000
@@ -1740,13 +1740,13 @@
 tindex(reset-prompt)
 item(tt(reset-prompt) (unbound) (unbound) (unbound))(
 Force the prompts on both the left and right of the screen to be
-re-expanded, then redisplay the edit buffer.  Note that this
-does not reflect changes to the prompt variables themselves, only changes
+re-expanded, then redisplay the edit buffer.  This
+reflects changes both to the prompt variables themselves and changes
 in the expansion of the values (for example, changes in time or
 directory, or changes to the value of variables referred to by the
 prompt).
 
-Otherwise, the prompt is only expaned each time zle starts, and
+Otherwise, the prompt is only expanded each time zle starts, and
 when the display as been interrupted by output from another part of the
 shell (such as a job notification) which causes the command line to be
 reprinted.
Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.41
diff -u -r1.41 init.c
--- Src/init.c	2 Jun 2004 22:14:26 -0000	1.41
+++ Src/init.c	11 Jul 2004 22:34:42 -0000
@@ -1148,7 +1148,7 @@
 
 /**/
 unsigned char *
-autoload_zleread(char *lp, char *rp, int ha, int con)
+autoload_zleread(char **lp, char **rp, int ha, int con)
 {
     zlereadptr = fallback_zleread;
     if (load_module("zsh/zle"))
@@ -1158,12 +1158,12 @@
 
 /**/
 mod_export unsigned char *
-fallback_zleread(char *lp, UNUSED(char *rp), UNUSED(int ha), UNUSED(int con))
+fallback_zleread(char **lp, UNUSED(char **rp), UNUSED(int ha), UNUSED(int con))
 {
     char *pptbuf;
     int pptlen;
 
-    pptbuf = unmetafy(promptexpand(lp, 0, NULL, NULL), &pptlen);
+    pptbuf = unmetafy(promptexpand(lp ? *lp : NULL, 0, NULL, NULL), &pptlen);
     write(2, (WRITE_ARG_2_T)pptbuf, pptlen);
     free(pptbuf);
 
Index: Src/input.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/input.c,v
retrieving revision 1.10
diff -u -r1.10 input.c
--- Src/input.c	15 Dec 2003 22:45:29 -0000	1.10
+++ Src/input.c	11 Jul 2004 22:34:48 -0000
@@ -222,21 +222,21 @@
 static int
 inputline(void)
 {
-    char *ingetcline, *ingetcpmptl = NULL, *ingetcpmptr = NULL;
+    char *ingetcline, **ingetcpmptl = NULL, **ingetcpmptr = NULL;
     int context = ZLCON_LINE_START;
 
     /* If reading code interactively, work out the prompts. */
     if (interact && isset(SHINSTDIN)) {
 	if (!isfirstln) {
-	    ingetcpmptl = prompt2;
+	    ingetcpmptl = &prompt2;
 	    if (rprompt2)
-		ingetcpmptr = rprompt2;
+		ingetcpmptr = &rprompt2;
 	    context = ZLCON_LINE_CONT;
 	}
 	else {
-	    ingetcpmptl = prompt;
+	    ingetcpmptl = &prompt;
 	    if (rprompt)
-		ingetcpmptr = rprompt;
+		ingetcpmptr = &rprompt;
 	}
     }
     if (!(interact && isset(SHINSTDIN) && SHTTY != -1 && isset(USEZLE))) {
@@ -255,7 +255,8 @@
 	     */
 	    char *pptbuf;
 	    int pptlen;
-	    pptbuf = unmetafy(promptexpand(ingetcpmptl, 0, NULL, NULL), &pptlen);
+	    pptbuf = unmetafy(promptexpand(ingetcpmptl ? *ingetcpmptl : NULL,
+					   0, NULL, NULL), &pptlen);
 	    write(2, (WRITE_ARG_2_T)pptbuf, pptlen);
 	    free(pptbuf);
 	}
Index: Src/loop.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/loop.c,v
retrieving revision 1.14
diff -u -r1.14 loop.c
--- Src/loop.c	22 Jun 2004 13:10:02 -0000	1.14
+++ Src/loop.c	11 Jul 2004 22:34:54 -0000
@@ -245,7 +245,7 @@
 		    int oef = errflag;
 
 		    isfirstln = 1;
-		    str = (char *)zleread(prompt3, NULL, 0, ZLCON_SELECT);
+		    str = (char *)zleread(&prompt3, NULL, 0, ZLCON_SELECT);
 		    if (errflag)
 			str = NULL;
 		    errflag = oef;
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.57
diff -u -r1.57 zsh.h
--- Src/zsh.h	22 Jun 2004 13:10:02 -0000	1.57
+++ Src/zsh.h	11 Jul 2004 22:35:13 -0000
@@ -1800,7 +1800,7 @@
 
 typedef void (*ZleVoidFn) _((void));
 typedef void (*ZleVoidIntFn) _((int));
-typedef unsigned char * (*ZleReadFn) _((char *, char *, int, int));
+typedef unsigned char * (*ZleReadFn) _((char **, char **, int, int));
 
 /***************************************/
 /* Hooks in core.                      */
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.47
diff -u -r1.47 zle_main.c
--- Src/Zle/zle_main.c	2 Jul 2004 15:59:14 -0000	1.47
+++ Src/Zle/zle_main.c	11 Jul 2004 22:35:48 -0000
@@ -150,7 +150,7 @@
 /**/
 mod_export char *zlenoargs[1] = { NULL };
 
-static char *raw_lp, *raw_rp;
+static char **raw_lp, **raw_rp;
 
 #ifdef FIONREAD
 static int delayzsetterm;
@@ -742,7 +742,7 @@
 
 /**/
 unsigned char *
-zleread(char *lp, char *rp, int flags, int context)
+zleread(char **lp, char **rp, int flags, int context)
 {
     unsigned char *s;
     int old_errno = errno;
@@ -761,7 +761,8 @@
 	char *pptbuf;
 	int pptlen;
 
-	pptbuf = unmetafy(promptexpand(lp, 0, NULL, NULL), &pptlen);
+	pptbuf = unmetafy(promptexpand(lp ? *lp : NULL, 0, NULL, NULL),
+			  &pptlen);
 	write(2, (WRITE_ARG_2_T)pptbuf, pptlen);
 	free(pptbuf);
 	return (unsigned char *)shingetline();
@@ -788,10 +789,10 @@
     eofsent = 0;
     resetneeded = 0;
     raw_lp = lp;
-    lpromptbuf = promptexpand(lp, 1, NULL, NULL);
+    lpromptbuf = promptexpand(lp ? *lp : NULL, 1, NULL, NULL);
     pmpt_attr = txtchange;
     raw_rp = rp;
-    rpromptbuf = promptexpand(rp, 1, NULL, NULL);
+    rpromptbuf = promptexpand(rp ? *rp : NULL, 1, NULL, NULL);
     rpmpt_attr = txtchange;
     free_prepostdisplay();
 
@@ -1169,7 +1170,7 @@
     if (OPT_ISSET(ops,'h'))
 	hbegin(2);
     isfirstln = OPT_ISSET(ops,'e');
-    t = (char *) zleread(p1, p2, OPT_ISSET(ops,'h') ? ZLRF_HISTORY : 0,
+    t = (char *) zleread(&p1, &p2, OPT_ISSET(ops,'h') ? ZLRF_HISTORY : 0,
 			 ZLCON_VARED);
     if (OPT_ISSET(ops,'h'))
 	hend(NULL);
@@ -1315,9 +1316,9 @@
 reexpandprompt(void)
 {
     free(lpromptbuf);
-    lpromptbuf = promptexpand(raw_lp, 1, NULL, NULL);
+    lpromptbuf = promptexpand(raw_lp ? *raw_lp : NULL, 1, NULL, NULL);
     free(rpromptbuf);
-    rpromptbuf = promptexpand(raw_rp, 1, NULL, NULL);
+    rpromptbuf = promptexpand(raw_rp ? *raw_rp : NULL, 1, NULL, NULL);
 }
 
 /**/

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: Bug with the new prompt redraw code

[Bart Schaefer]

On Sun, 11 Jul 2004, Peter Stephenson wrote:

> > > > (1) zle-line-init runs before the prompt is displayed
> 
> > Also it doesn't address (1) but then maybe that doesn't matter.
> 
> I could be convinced it does, but see if this makes the problem go away
> first.

I can confirm that this does print the prompt that was assigned during
zle-line-init, rather than the garbage that was output before the patch.

So, what about (1)?  To repeat, the intent was to display a multi-line 
prompt immediately after a command finishes, but then refresh only the 
last line of that prompt on subsequent zle redisplays (the original 
example involved SIGWINCH handling).

An almost-equivalent thing can be achieved by using "print -P" from the
precmd function instead of by assignment to PS1, but there's a question
of whether other commands in zle-line-init should affect values that are
substituted into the prompt (e.g. $psvar).

Re: Bug with the new prompt redraw code

[Peter Stephenson]

Bart Schaefer wrote:
> On Sun, 11 Jul 2004, Peter Stephenson wrote:
> 
> > > > > (1) zle-line-init runs before the prompt is displayed
> 
> So, what about (1)?  To repeat, the intent was to display a multi-line 
> prompt immediately after a command finishes, but then refresh only the 
> last line of that prompt on subsequent zle redisplays (the original 
> example involved SIGWINCH handling).
> 
> An almost-equivalent thing can be achieved by using "print -P" from the
> precmd function instead of by assignment to PS1, but there's a question
> of whether other commands in zle-line-init should affect values that are
> substituted into the prompt (e.g. $psvar).

It sounds like zrefresh() should be moved earlier, out of zlecore().

This means zle-line-init is basically the last thing executed before the
editor starts looking for input.  Hence zle-line-init will only affect
the next prompt, but the previous prompt will have been drawn.

Maybe that's right.

Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.48
diff -u -r1.48 zle_main.c
--- Src/Zle/zle_main.c	11 Jul 2004 22:53:04 -0000	1.48
+++ Src/Zle/zle_main.c	12 Jul 2004 17:48:57 -0000
@@ -680,8 +680,6 @@
     FD_ZERO(&foofd);
 #endif
 
-    zrefresh();
-
     while (!done && !errflag) {
 
 	statusline = NULL;
@@ -834,6 +832,8 @@
     initmodifier(&zmod);
     prefixflag = 0;
 
+    zrefresh();
+
     if ((initthingy = rthingy_nocreate("zle-line-init"))) {
 	char *args[2];
 	args[0] = initthingy->nam;
@@ -1303,6 +1303,7 @@
 {
     int locerror;
 
+    zrefresh();
     zlecore();
 
     locerror = errflag;

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: Bug with the new prompt redraw code

[Bart Schaefer]

On Mon, 12 Jul 2004, Peter Stephenson wrote:

> Bart Schaefer wrote:
> > So, what about (1)?  To repeat, the intent was to display a multi-line 
> > prompt immediately after a command finishes, but then refresh only the 
> > last line of that prompt on subsequent zle redisplays (the original 
> > example involved SIGWINCH handling).
> 
> It sounds like zrefresh() should be moved earlier, out of zlecore().
> 
> This means zle-line-init is basically the last thing executed before the
> editor starts looking for input.  Hence zle-line-init will only affect
> the next prompt, but the previous prompt will have been drawn.

This does sound like the approach that I asked for, and the C code seems 
to do the intended things, but it doesn't quite achieve the visible effect 
that I wanted.

The PS1 prompt as set at precmd time does indeed display, regardless of 
any change made in zle-line-init.

However, upon receiving SIGWINCH, the old prompt is still redrawn.  The 
new prompt doesn't kick in until "zle reset-prompt" is run ... but if one 
runs reset-prompt from zle-line-init, it immediately erases the old prompt 
and redraws, so we're almost back where we started.  The specific case of 
redrawing only the last line of a multi-line prompt *does* work, but only 
when the one-line PS1 assigned in zle-line-init is indistinguishable from 
the last line of the multi-line PS1 assigned in precmd, if you see what I 
mean.

I tried using a TRAPWINCH but that doesn't get executed until _after_ zle 
has had it's shot at the signal, so one always gets at least one spurious 
redraw of the old PS1 before anything can be done about it.

At this point it's probably not worth messing with further, though there 
does not seem to be any harm in committing 20153.