Re: PATCH: remove OFF flags for display attributes in zattr

[Oliver Kiddle <opk@xxxxxxx>]

Bart Schaefer wrote:
> Just confirming that this won't interfere with (for example) leaving
> an attribute turned on at the end of PS1 so that the command line
> input has a different appearance than the command output?  I believe
> this is a somewhat popular trick.

It turns out that the existing code is rather buggy in this regard. To
demonstrate just one case:

    % zle_highlight=(default:bold special:none)
    % PS1="%U%F{23}%# "
    % echo^Gone

It keeps the colour and underline at the end of the prompt but as soon
as it encounters something that wants to turn it off such as the none
attribute for the Ctrl-G character it is turned off and stays off.

The further I delve into this, the more I find oddities that are
arguably bugs. For the message area (completion lists, zle -R etc)
setting zle_highlight=(default:fg=blue) drops the prompt left overs
and reverts to default colours. And the code is inconsistent with
pmpt_attr ? pmpt_attr : rpmpt_attr used on one place and careful
merging of attributes other than the colour elsewhere.

With the patch below, I'm sure there will still be issues but I'm
hopeful that it fixes more than it breaks. This now makes use of
TXT_ERROR to indicate that attributes should be unchanged in a
REFRESH_ELEMENT which was needed for some newlines. It could perhaps
be renamed. I need to better understand TXT_MULTIWORD_MASK because I
suspect that can be replaced with similar uses of TXT_ERROR. Is that
used for things like composing characters?

Attributes left on at the end of the prompt also remain on for whatever
command is run. I don't doubt that there are users out there relying
on this but it's really not a good idea. How far do we want to go with
backward compatibility? Maybe we should add setopt FUTURE and wrap
some of the backward compatibility so that we can drop it when doing a
major release. (That might also provide a path to an eventual 6.0 while
remaining true to strict semantic versioning).

Oliver

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 39be33939..686c6f5b4 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -1267,11 +1267,13 @@ zleread(char **lp, char **rp, int flags, int context, char *init, char *finish)
     fetchttyinfo = 0;
     trashedzle = 0;
     raw_lp = lp;
+    txtcurrentattrs = txtpendingattrs = txtunknownattrs = 0;
     lpromptbuf = promptexpand(lp ? *lp : NULL, 1, NULL, NULL);
     pmpt_attr = txtcurrentattrs;
     raw_rp = rp;
     rpromptbuf = promptexpand(rp ? *rp : NULL, 1, NULL, NULL);
     rpmpt_attr = txtcurrentattrs;
+    prompt_attr = mixattrs(pmpt_attr, rpmpt_attr);
     free_prepostdisplay();
 
     zlereadflags = flags;
@@ -2010,6 +2012,7 @@ reexpandprompt(void)
 	    char *new_lprompt, *new_rprompt;
 	    looping = reexpanding;
 
+	    txtcurrentattrs = txtpendingattrs = txtunknownattrs = 0;
 	    new_lprompt = promptexpand(raw_lp ? *raw_lp : NULL, 1, NULL, NULL);
 	    pmpt_attr = txtcurrentattrs;
 	    free(lpromptbuf);
@@ -2018,9 +2021,9 @@ reexpandprompt(void)
 	    if (looping != reexpanding)
 		continue;
 
-	    rpmpt_attr = pmpt_attr;
 	    new_rprompt = promptexpand(raw_rp ? *raw_rp : NULL, 1, NULL, NULL);
 	    rpmpt_attr = txtcurrentattrs;
+	    prompt_attr = mixattrs(pmpt_attr, rpmpt_attr);
 	    free(rpromptbuf);
 	    rpromptbuf = new_rprompt;
 	} while (looping != reexpanding);
@@ -2067,6 +2070,8 @@ trashzle(void)
 	zrefresh();
 	showinglist = sl;
 	moveto(nlnct, 0);
+	treplaceattrs(prompt_attr);
+	applytextattributes(0);
 	if (clearflag && tccan(TCCLEAREOD)) {
 	    tcout(TCCLEAREOD);
 	    clearflag = listshown = 0;
diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index 6cbfe9072..5c284be91 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -149,7 +149,7 @@ char *lpromptbuf, *rpromptbuf;
 /* Text attributes after displaying prompts */
 
 /**/
-zattr pmpt_attr, rpmpt_attr;
+zattr pmpt_attr, rpmpt_attr, prompt_attr;
 
 /* number of lines displayed */
 
@@ -254,13 +254,13 @@ int cost;
 # define zwrite(a, b)		zwcwrite((a), (b))
 #endif
 
-static const REFRESH_ELEMENT zr_cr = { ZWC('\r'), 0 };
+static const REFRESH_ELEMENT zr_cr = { ZWC('\r'), TXT_ERROR };
 #ifdef MULTIBYTE_SUPPORT
-static const REFRESH_ELEMENT zr_dt = { ZWC('.'), 0 };
+static const REFRESH_ELEMENT zr_dt = { ZWC('.'), TXT_ERROR };
 #endif
-static const REFRESH_ELEMENT zr_nl = { ZWC('\n'), 0 };
-static const REFRESH_ELEMENT zr_sp = { ZWC(' '), 0 };
-static const REFRESH_ELEMENT zr_zr = { ZWC('\0'), 0 };
+static const REFRESH_ELEMENT zr_nl = { ZWC('\n'), TXT_ERROR };
+static const REFRESH_ELEMENT zr_sp = { ZWC(' '), TXT_ERROR };
+static const REFRESH_ELEMENT zr_zr = { ZWC('\0'), TXT_ERROR };
 
 /*
  * Constant arrays to be copied into place: these are memcpy'd,
@@ -599,8 +599,10 @@ zwcputc(const REFRESH_ELEMENT *c)
     VARARR(char, mbtmp, MB_CUR_MAX + 1);
 #endif
 
-    treplaceattrs(c->atr);
-    applytextattributes(0);
+    if (c->atr != TXT_ERROR) {
+	treplaceattrs(c->atr);
+	applytextattributes(0);
+    }
 
 #ifdef MULTIBYTE_SUPPORT
     if (c->atr & TXT_MULTIWORD_MASK) {
@@ -1131,9 +1133,8 @@ zrefresh(void)
 	    zputs(lpromptbuf, shout);
 	    if (lpromptwof == winw)
 		zputs("\n", shout);	/* works with both hasam and !hasam */
-	} else {
-	    treplaceattrs(pmpt_attr);
-	    applytextattributes(0);
+	    /* lpromptbuf includes literal escapes so we need to update for it */
+	    txtcurrentattrs = txtpendingattrs = pmpt_attr;
 	}
 	if (clearflag) {
 	    zputc(&zr_cr);
@@ -1177,7 +1178,7 @@ zrefresh(void)
     rpms.sen = *nbuf + winw;
     for (t = tmpline, tmppos = 0; tmppos < tmpll; t++, tmppos++) {
 	unsigned ireg;
-	zattr base_attr = default_attr;
+	zattr base_attr = mixattrs(default_attr, prompt_attr);
 	zattr all_attr;
 	struct region_highlight *rhp;
 	/*
@@ -1265,10 +1266,6 @@ zrefresh(void)
 		rpms.s++;
 	    } else {
 		/* We can fit it without reaching the end of the line. */
-		/*
-		 * As we don't actually output the WEOF, we attach
-		 * any off attributes to the character itself.
-		 */
 		rpms.s->atr = base_attr;
 		if (ichars > 1) {
 		    /*
@@ -1606,8 +1603,9 @@ zrefresh(void)
 
     /* output the right-prompt if appropriate */
 	if (put_rpmpt && !iln && !oput_rpmpt) {
-
 	    moveto(0, winw - rprompt_off - rpromptw);
+	    treplaceattrs(pmpt_attr);
+	    applytextattributes(0);
 	    zputs(rpromptbuf, shout);
 	    if (rprompt_off) {
 		vcs = winw - rprompt_off;
@@ -1615,9 +1613,7 @@ zrefresh(void)
 		zputc(&zr_cr);
 		vcs = 0;
 	    }
-	    /* reset character attributes to that set by the main prompt */
-	    treplaceattrs(pmpt_attr);
-	    applytextattributes(0);
+	    txtcurrentattrs = txtpendingattrs = rpmpt_attr;
 	}
     }
 
@@ -1630,11 +1626,6 @@ individually */
 	    refreshline(iln);
     }
 
-/* reset character attributes */
-    if (clearf && postedit) {
-	treplaceattrs(pmpt_attr ? pmpt_attr : rpmpt_attr);
-	applytextattributes(0);
-    }
     clearf = 0;
     oput_rpmpt = put_rpmpt;
 
diff --git a/Src/zsh.h b/Src/zsh.h
index e834c7e06..073f8b71b 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2694,7 +2694,8 @@ struct ttyinfo {
  */
 #define TXT_MULTIWORD_MASK  0x0400
 
-/* used when, e.g an invalid colour is specified */
+/* Used when, e.g an invalid colour is specified. Also used in REFRESH_ELEMENT
+ * to indicate that attributes should remain unchanged. */
 #define TXT_ERROR 0xF00000F000000003
 
 /* Mask for font weight */