Geoff's prompt escape sequences patch (workers/23924)

Geoff's prompt escape sequences patch (workers/23924)

[Bart Schaefer]

I just noticed this never got committed to CVS.  I've tried it and it is
giving 99% of the correct behavior now (well, what I think is the correct
behavior; I never tried ending a prompt by turning ON standout, before).

The only glitch I can find by randomly fooling around is that completion
sometimes erases a standout space to the left of the word, inserts a plain
space, and then draws the completed word in standout, so you end up with
a command line with a "hole" in it.  Any further redraw of the line after
this point (such as completing again on a later word) repaints correctly
and fills the hole.

Fixing that probably means diving into the completion refresh code, so I
think committing what Geoff has so far would be fine.

Re: Geoff's prompt escape sequences patch (workers/23924)

[Geoff Wing]

On Friday 2007-10-19 00:01 +1000, Bart Schaefer output:
:I just noticed this never got committed to CVS.

I was waiting for someone to actually try it :-)

Regards,
Geoff

Re: Geoff's prompt escape sequences patch (workers/23924)

[Mikael Magnusson]

On 18/10/2007, Bart Schaefer <schaefer@brasslantern.com> wrote:
> I just noticed this never got committed to CVS.  I've tried it and it is
> giving 99% of the correct behavior now (well, what I think is the correct
> behavior; I never tried ending a prompt by turning ON standout, before).
>
> The only glitch I can find by randomly fooling around is that completion
> sometimes erases a standout space to the left of the word, inserts a plain
> space, and then draws the completed word in standout, so you end up with
> a command line with a "hole" in it.  Any further redraw of the line after
> this point (such as completing again on a later word) repaints correctly
> and fills the hole.
>
> Fixing that probably means diving into the completion refresh code, so I
> think committing what Geoff has so far would be fine.

I've found another problem that seems to be caused by this patch
(located by git-bisect). When a bg'd job finishes, the cursor is drawn
in the wrong place in the new prompt (#bar characters to the left).
Pressing a key bound to reset-prompt fixes it.

The simplest test case i could find was
zsh -f
PS1="frobnizz %#"      #not really needed but since the default
depends on hostname...
RPS1="foo%(1j.bar.)"
true &

-- 
Mikael Magnusson

Re: Geoff's prompt escape sequences patch (workers/23924)

[Bart Schaefer]

On Nov 16,  3:57pm, Mikael Magnusson wrote:
} Subject: Re: Geoff's prompt escape sequences patch (workers/23924)
}
} I've found another problem that seems to be caused by this patch
} (located by git-bisect). When a bg'd job finishes, the cursor is drawn
} in the wrong place in the new prompt (#bar characters to the left).

It would appear that the starting position for the right prompt is not
being recomputed correctly.  It's not just that the cursor is moved
left; RPS1 still begins at the same column even though it is shorter,
so it doesn't reach all the way to the margin.

The cursor positioning error is probably only apparent when the number
of bytes in the terminfo "move left N" string is less than the number
in "move to position X,Y".

} The simplest test case i could find was
} zsh -f
} PS1="frobnizz %#"      #not really needed but since the default
} depends on hostname...
} RPS1="foo%(1j.bar.)"
} true &

The effect is more obvious if you change to RPS1="%(1j.bar.)foo"

Re: Geoff's prompt escape sequences patch (workers/23924)

[Geoff Wing]

On Saturday 2007-11-17 03:46 +1100, Bart Schaefer output:
:On Nov 16,  3:57pm, Mikael Magnusson wrote:
:} Subject: Re: Geoff's prompt escape sequences patch (workers/23924)
:} I've found another problem that seems to be caused by this patch
:} (located by git-bisect). When a bg'd job finishes, the cursor is drawn
:} in the wrong place in the new prompt (#bar characters to the left).
:
:It would appear that the starting position for the right prompt is not
:being recomputed correctly.  It's not just that the cursor is moved
:left; RPS1 still begins at the same column even though it is shorter,
:so it doesn't reach all the way to the margin.
:The cursor positioning error is probably only apparent when the number
:of bytes in the terminfo "move left N" string is less than the number
:in "move to position X,Y".

Yes, my fault.  reexpandprompt() needs to happen before resetvideo()
which uses the prompt sizes.

:} The simplest test case i could find was
:} zsh -f
:} PS1="frobnizz %#"      #not really needed but since the default
:} depends on hostname...
:} RPS1="foo%(1j.bar.)"
:} true &
:The effect is more obvious if you change to RPS1="%(1j.bar.)foo"

I'll think this through properly a bit later (in a couple of days
or so), though I currently think this fixes it.

Regards,
Geoff

Index: Src/Zle/zle_refresh.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_refresh.c,v
retrieving revision 1.49
diff -u -r1.49 zle_refresh.c
--- Src/Zle/zle_refresh.c	19 Oct 2007 01:33:09 -0000	1.49
+++ Src/Zle/zle_refresh.c	17 Nov 2007 09:22:47 -0000
@@ -446,7 +446,6 @@
     int tmpcs, tmpll;		/* ditto cursor position and line length     */
     int tmpalloced;		/* flag to free tmpline when finished        */
     int remetafy;		/* flag that zle line is metafied            */
-    int fixprompt;		/* we still need to reexpand the prompt      */
     struct rparams rpms;
     
     /* If this is called from listmatches() (indirectly via trashzle()), and *
@@ -540,11 +539,6 @@
 	    listshown = 0;
 	}
 #endif
-	fixprompt = trashedzle;
-	resetvideo();
-	resetneeded = 0;	/* unset */
-	oput_rpmpt = 0;		/* no right-prompt currently on screen */
-
 	/* we probably should only have explicitly set attributes */
 	tsetcap(TCALLATTRSOFF, 0);
 	tsetcap(TCSTANDOUTEND, 0);
@@ -552,6 +546,12 @@
 	/* cheat on attribute unset */
 	txtunset(TXTBOLDFACE|TXTSTANDOUT|TXTUNDERLINE|TXTDIRTY);
 
+	if (trashedzle)
+	    reexpandprompt(); 
+	resetvideo();
+	resetneeded = 0;	/* unset */
+	oput_rpmpt = 0;		/* no right-prompt currently on screen */
+
         if (!clearflag) {
             if (tccan(TCCLEAREOD))
                 tcout(TCCLEAREOD);
@@ -562,8 +562,6 @@
 	}
         if (t0 > -1)
             olnct = (t0 < winh) ? t0 : winh;
-	if (fixprompt)
-	    reexpandprompt(); 
         if (termflags & TERM_SHORT)
             vcs = 0;
 	else if (!clearflag && lpromptbuf[0]) {

Re: Geoff's prompt escape sequences patch (workers/23924)

[Mikael Magnusson]

On 17/11/2007, Geoff Wing <gcw@zsh.org> wrote:
> On Saturday 2007-11-17 03:46 +1100, Bart Schaefer output:
> :On Nov 16,  3:57pm, Mikael Magnusson wrote:
> :} Subject: Re: Geoff's prompt escape sequences patch (workers/23924)
> :} I've found another problem that seems to be caused by this patch
> :} (located by git-bisect). When a bg'd job finishes, the cursor is drawn
> :} in the wrong place in the new prompt (#bar characters to the left).
> :
> :It would appear that the starting position for the right prompt is not
> :being recomputed correctly.  It's not just that the cursor is moved
> :left; RPS1 still begins at the same column even though it is shorter,
> :so it doesn't reach all the way to the margin.
> :The cursor positioning error is probably only apparent when the number
> :of bytes in the terminfo "move left N" string is less than the number
> :in "move to position X,Y".
>
> Yes, my fault.  reexpandprompt() needs to happen before resetvideo()
> which uses the prompt sizes.
>
> :The effect is more obvious if you change to RPS1="%(1j.bar.)foo"
>
> I'll think this through properly a bit later (in a couple of days
> or so), though I currently think this fixes it.

It works here, thanks.

-- 
Mikael Magnusson