PATCH: isearch match highlighting

PATCH: isearch match highlighting

[Peter Stephenson]

This one is worth posting.

With pattern matching in isearch, it's now quite useful to have the
matched string highlighted.  I've made the default underlining rather
than standout because of the annoyance that standout on a cursor that's
shown in inverse video flips it to normal video, then when you've just
matched one character there's nothing at all on the line.  This seems to
be common behaviour.

You could argue that if highlighting is done this way it would make more
sense for the cursor position to be where the user is typing, but I
it's non-trivial to change, hard to be sure the highlighting selected is
going to work, incompatible with usage when there's no highlighting, and
I don't feel like monkeying with it anyway.

I can't help feeling I must have screwed up this one somehow.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.63
diff -u -r1.63 zle.yo
--- Doc/Zsh/zle.yo	26 Apr 2008 19:51:09 -0000	1.63
+++ Doc/Zsh/zle.yo	26 Apr 2008 22:41:58 -0000
@@ -2064,6 +2064,10 @@
 startitem()
 cindex(region, highlighting)
 cindex(highlighting, region)
+item(tt(isearch))(
+When one of the incremental history search widgets is active, the
+area of the command line matched by the search string or pattern.
+)
 item(tt(region))(
 The region between the cursor (point) and the mark as set with
 tt(set-mark-command).  The region is only highlighted if it is active,
@@ -2102,7 +2106,7 @@
 is inverse video.  On some such terminals, where the cursor does not blink
 it appears with standout mode negated, making it less than clear where
 the cursor actually is.  On such terminals one of the other effects
-may be preferable for highlighting the region.
+may be preferable for highlighting the region and matched search string.
 )
 item(tt(underline))(
 The characters in the given context are shown underlined.  Some
@@ -2133,7 +2137,8 @@
 If tt(zle_highlight) is not set or no value applies to a particular
 context, the defaults applied are equivalent to
 
-example(zle_highlight=LPAR()region:standout special:standout+RPAR())
+example(zle_highlight=LPAR()region:standout special:standout
+isearch:underline+RPAR())
 
 i.e. both the region and special characters are shown in standout mode.
 
Index: Src/Zle/zle_hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_hist.c,v
retrieving revision 1.48
diff -u -r1.48 zle_hist.c
--- Src/Zle/zle_hist.c	26 Apr 2008 21:26:28 -0000	1.48
+++ Src/Zle/zle_hist.c	26 Apr 2008 22:41:59 -0000
@@ -907,6 +907,7 @@
     int pat_hl;			/* histline where pattern search started */
     unsigned short pos;		/* The search position in our metafied str */
     unsigned short pat_pos;     /* pos where pattern search started */
+    unsigned short end_pos;	/* The position of the end of the matched str */
     unsigned short cs;		/* The visible search position to the user */
     unsigned short len;		/* The search string's length */
     unsigned short flags;	/* This spot's flags */
@@ -928,7 +929,7 @@
 /**/
 static void
 set_isrch_spot(int num, int hl, int pos, int pat_hl, int pat_pos,
-	       int cs, int len, int dir, int nomatch)
+	       int end_pos, int cs, int len, int dir, int nomatch)
 {
     if (num >= max_spot) {
 	if (!isrch_spots) {
@@ -944,6 +945,7 @@
     isrch_spots[num].pos = (unsigned short)pos;
     isrch_spots[num].pat_hl = pat_hl;
     isrch_spots[num].pat_pos = (unsigned short)pat_pos;
+    isrch_spots[num].end_pos = (unsigned short)end_pos;
     isrch_spots[num].cs = (unsigned short)cs;
     isrch_spots[num].len = (unsigned short)len;
     isrch_spots[num].flags = (dir > 0? ISS_FORWARD : 0)
@@ -953,12 +955,13 @@
 /**/
 static void
 get_isrch_spot(int num, int *hlp, int *posp, int *pat_hlp, int *pat_posp,
-	       int *csp, int *lenp, int *dirp, int *nomatch)
+	       int *end_posp, int *csp, int *lenp, int *dirp, int *nomatch)
 {
     *hlp = isrch_spots[num].hl;
     *posp = (int)isrch_spots[num].pos;
     *pat_hlp = isrch_spots[num].pat_hl;
     *pat_posp = (int)isrch_spots[num].pat_pos;
+    *end_posp = (int)isrch_spots[num].end_pos;
     *csp = (int)isrch_spots[num].cs;
     *lenp = (int)isrch_spots[num].len;
     *dirp = (isrch_spots[num].flags & ISS_FORWARD)? 1 : -1;
@@ -1011,6 +1014,9 @@
 #define FIRST_SEARCH_CHAR	(NORM_PROMPT_POS + 14)
 
 /**/
+int isearch_active, isearch_startpos, isearch_endpos;
+
+/**/
 static void
 doisearch(char **args, int dir, int pattern)
 {
@@ -1088,6 +1094,12 @@
      */
     int dup_ok = 0;
     /*
+     * End position of the match.
+     * When forward matching, this is the position for the cursor.
+     * When backward matching, the cursor position is pos.
+     */
+    int end_pos = 0;
+    /*
      * savekeys records the unget buffer, so that if we have arguments
      * they don't pollute the input.
      * feep indicates we should feep.  This is a well-known word
@@ -1138,7 +1150,7 @@
     pat_pos = pos = zlemetacs;
     for (;;) {
 	/* Remember the current values in case search fails (doesn't push). */
-	set_isrch_spot(top_spot, hl, pos, pat_hl, pat_pos,
+	set_isrch_spot(top_spot, hl, pos, pat_hl, pat_pos, end_pos,
 		       zlemetacs, sbptr, dir, nomatch);
 	if (sbptr == 1 && sbuf[0] == '^') {
 	    zlemetacs = 0;
@@ -1147,11 +1159,6 @@
 	} else if (sbptr > 0) {
 	    /* The matched text, used as flag that we matched */
 	    char *t = NULL;
-	    /*
-	     * When forward matching, position for the cursor.
-	     * When backward matching, the position is pos.
-	     */
-	    int forwardmatchpos = 0;
 	    last_line = zt;
 
 	    sbuf[sbptr] = '\0';
@@ -1233,7 +1240,7 @@
 			     */
 			    if (!skip_pos &&
 				pattryrefs(patprog, zt, -1, -1, 0, NULL, NULL,
-					   &forwardmatchpos))
+					   &end_pos))
 				t = zt;
 			} else {
 			    if (!matchlist && !skip_pos) {
@@ -1268,7 +1275,7 @@
 					newpos = pos + 1;
 				}
 				newpos = isearch_newpos(matchlist, newpos,
-							dir, &forwardmatchpos);
+							dir, &end_pos);
 				/* need a new list next time if off the end */
 				if (newpos < 0) {
 				    freematchlist(matchlist);
@@ -1316,7 +1323,7 @@
 			} else
 			    t = zlinefind(zt, pos, sbuf, dir, sens);
 			if (t)
-			    forwardmatchpos = pos + sbptr - (sbuf[0] == '^');
+			    end_pos = pos + sbptr - (sbuf[0] == '^');
 		    }
 		}
 		if (t) {
@@ -1333,7 +1340,8 @@
 		     && (isrch_spots[top_spot-1].flags >> ISS_NOMATCH_SHIFT))
 			top_spot--;
 		    get_isrch_spot(top_spot, &hl, &pos, &pat_hl, &pat_pos,
-				   &zlemetacs, &sbptr, &dir, &nomatch);
+				   &end_pos, &zlemetacs, &sbptr, &dir,
+				   &nomatch);
 		    if (!nomatch) {
 			feep = 1;
 			nomatch = 1;
@@ -1366,7 +1374,7 @@
 	    if (t || (nosearch && !nomatch)) {
 		zle_setline(he);
 		if (dir == 1)
-		    zlemetacs = forwardmatchpos;
+		    zlemetacs = end_pos;
 		else
 		    zlemetacs = pos;
 		statusline = ibuf + NORM_PROMPT_POS;
@@ -1384,11 +1392,39 @@
 	}
 	sbuf[sbptr] = '_';
 	sbuf[sbptr+1] = '\0';
+	if (!nomatch && sbptr && (sbptr > 1 || sbuf[0] != '^')) {
+#ifdef MULTIBYTE_SUPPORT
+	    int charpos = 0, charcount = 0, ret;
+	    wint_t wc;
+	    mbstate_t mbs;
+
+	    /*
+	     * Count unmetafied character positions for the
+	     * start and end of the match for the benefit of
+	     * highlighting.
+	     */
+	    memset(&mbs, 0, sizeof(mbs));
+	    while (charpos < end_pos) {
+		ret = mb_metacharlenconv_r(zlemetaline + charpos, &wc, &mbs);
+		if (charpos <= pos && pos < charpos + ret)
+		    isearch_startpos = charcount;
+		charcount++;
+		charpos += ret;
+	    }
+	    isearch_endpos = charcount;
+#else
+	    isearch_startpos = ztrsub(zlemetaline + pos, zlemetaline);
+	    isearch_endpos = ztrsub(zlemetaline + end_pos,
+				    zlemetaline);
+#endif
+	    isearch_active = 1;
+	} else
+	    isearch_active = 0;
     ref:
 	zrefresh();
 	if (!(cmd = getkeycmd()) || cmd == Th(z_sendbreak)) {
 	    int i;
-	    get_isrch_spot(0, &hl, &pos, &pat_hl, &pat_pos,
+	    get_isrch_spot(0, &hl, &pos, &pat_hl, &pat_pos, &end_pos,
 			   &i, &sbptr, &dir, &nomatch);
 	    he = quietgethist(hl);
 	    zle_setline(he);
@@ -1410,7 +1446,7 @@
 	    	cmd == Th(z_backwarddeletechar)) {
 	    if (top_spot) {
 		get_isrch_spot(--top_spot, &hl, &pos, &pat_hl, &pat_pos,
-			       &zlemetacs, &sbptr, &dir, &nomatch);
+			       &end_pos, &zlemetacs, &sbptr, &dir, &nomatch);
 		patprog = NULL;
 		nosearch = 1;
 	    } else
@@ -1452,7 +1488,7 @@
 		  cmd == Th(z_historyincrementalpatternsearchbackward)) {
 	    pat_hl = hl;
 	    pat_pos = pos;
-	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos, end_pos,
 			   zlemetacs, sbptr, dir, nomatch);
 	    if (dir != -1)
 		dir = -1;
@@ -1463,7 +1499,7 @@
 		  cmd == Th(z_historyincrementalpatternsearchforward)) {
 	    pat_hl = hl;
 	    pat_pos = pos;
-	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos, end_pos,
 			   zlemetacs, sbptr, dir, nomatch);
 	    if (dir != 1)
 		dir = 1;
@@ -1473,7 +1509,7 @@
 	} else if(cmd == Th(z_virevrepeatsearch)) {
 	    pat_hl = hl;
 	    pat_pos = pos;
-	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos, end_pos,
 			   zlemetacs, sbptr, dir, nomatch);
 	    dir = -odir;
 	    skip_pos = 1;
@@ -1481,7 +1517,7 @@
 	} else if(cmd == Th(z_virepeatsearch)) {
 	    pat_hl = hl;
 	    pat_pos = pos;
-	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos, end_pos,
 			   zlemetacs, sbptr, dir, nomatch);
 	    dir = odir;
 	    skip_pos = 1;
@@ -1534,7 +1570,7 @@
 		feep = 1;
 		continue;
 	    }
-	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos, end_pos,
 			   zlemetacs, sbptr, dir, nomatch);
 	    if (sbptr >= sibuf - FIRST_SEARCH_CHAR - 2 
 #ifdef MULTIBYTE_SUPPORT
@@ -1569,6 +1605,7 @@
     zsfree(okeymap);
     if (matchlist)
 	freematchlist(matchlist);
+    isearch_active = 0;
     /*
      * Don't allow unused characters provided as a string to the
      * widget to overflow and be used as separated commands.
Index: Src/Zle/zle_refresh.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_refresh.c,v
retrieving revision 1.61
diff -u -r1.61 zle_refresh.c
--- Src/Zle/zle_refresh.c	22 Apr 2008 15:08:14 -0000	1.61
+++ Src/Zle/zle_refresh.c	26 Apr 2008 22:42:00 -0000
@@ -242,8 +242,15 @@
  */
 struct region_highlight *region_highlights;
 /*
+ * Count of special uses of region highlighting, which account
+ * for the first few elements of region_highlights.
+ * 0: region between point and mark
+ * 1: isearch region
+ */
+#define N_SPECIAL_HIGHLIGHTS	(2)
+/*
  * Number of elements in region_highlights.
- * This includes the region between point and mark, element 0.
+ * This includes the special elements above.
  */
 int n_region_highlights;
 
@@ -380,12 +387,14 @@
     char **atrs = getaparam("zle_highlight");
     int special_atr_on_set = 0;
     int region_atr_on_set = 0;
+    int isearch_atr_on_set = 0;
+    struct region_highlight *rhp;
 
     special_atr_on = 0;
     if (!region_highlights) {
 	region_highlights = (struct region_highlight *)
-	    zshcalloc(sizeof(struct region_highlight));
-	n_region_highlights = 1;
+	    zshcalloc(N_SPECIAL_HIGHLIGHTS*sizeof(struct region_highlight));
+	n_region_highlights = N_SPECIAL_HIGHLIGHTS;
     } else {
 	region_highlights->atr = 0;
     }
@@ -394,14 +403,23 @@
 	for (; *atrs; atrs++) {
 	    if (!strcmp(*atrs, "none")) {
 		/* reset attributes for consistency... usually unnecessary */
-		special_atr_on = region_highlights->atr = 0;
-		special_atr_on_set = region_atr_on_set = 1;
+		special_atr_on = 0;
+		special_atr_on_set = region_atr_on_set =
+		    isearch_atr_on_set = 1;
+		for (rhp = region_highlights;
+		     rhp < region_highlights + N_SPECIAL_HIGHLIGHTS;
+		     rhp++) {
+		    rhp->atr = 0;
+		}
 	    } else if (strpfx("special:", *atrs)) {
 		match_highlight(*atrs + 8, &special_atr_on);
 		special_atr_on_set = 1;
 	    } else if (strpfx("region:", *atrs)) {
 		match_highlight(*atrs + 7, &region_highlights->atr);
 		region_atr_on_set = 1;
+	    } else if (strpfx("isearch:", *atrs)) {
+		match_highlight(*atrs + 8, &(region_highlights[1].atr));
+		isearch_atr_on_set = 1;
 	    }
 	}
     }
@@ -411,6 +429,8 @@
 	special_atr_on = TXTSTANDOUT;
     if (!region_atr_on_set)
 	region_highlights->atr = TXTSTANDOUT;
+    if (!isearch_atr_on_set)
+	region_highlights[1].atr = TXTUNDERLINE;
     special_atr_off = special_atr_on << TXT_ATTR_OFF_ON_SHIFT;
 }
 
@@ -432,15 +452,17 @@
 
     /* region_highlights may not have been set yet */
     if (!arrsize)
-	arrsize = 1;
+	arrsize = N_SPECIAL_HIGHLIGHTS;
     arrp = retarr = (char **)zhalloc(arrsize*sizeof(char *));
     /* ignore NULL termination */
     arrsize--;
     if (arrsize) {
 	struct region_highlight *rhp;
 
-	/* ignore point/mark at start */
-	for (rhp = region_highlights+1; arrsize--; rhp++, arrp++) {
+	/* ignore special highlighting */
+	for (rhp = region_highlights + N_SPECIAL_HIGHLIGHTS;
+	     arrsize--;
+	     rhp++, arrp++) {
 	    char digbuf1[DIGBUFSIZE], digbuf2[DIGBUFSIZE];
 	    int atrlen = 0, alloclen, done1;
 	    const struct highlight *hp;
@@ -503,9 +525,9 @@
     struct region_highlight *rhp;
 
     len = aval ? arrlen(aval) : 0;
-    if (n_region_highlights != len + 1) {
-	/* no null termination, but include point/mark region at start */
-	n_region_highlights = len + 1;
+    if (n_region_highlights != len + N_SPECIAL_HIGHLIGHTS) {
+	/* no null termination, but include special highlighting at start */
+	n_region_highlights = len + N_SPECIAL_HIGHLIGHTS;
 	region_highlights = (struct region_highlight *)
 	    zrealloc(region_highlights,
 		     sizeof(struct region_highlight) * n_region_highlights);
@@ -514,7 +536,9 @@
     if (!aval)
 	return;
 
-    for (rhp = region_highlights + 1; *aval; rhp++, aval++) {
+    for (rhp = region_highlights + N_SPECIAL_HIGHLIGHTS;
+	 *aval;
+	 rhp++, aval++) {
 	char *strp, *oldstrp;
 
 	oldstrp = *aval;
@@ -1050,6 +1074,13 @@
     } else {
 	region_highlights->start = region_highlights->end = -1;
     }
+    /* check for isearch string to highlight */
+    if (isearch_active) {
+	region_highlights[1].start = isearch_startpos;
+	region_highlights[1].end = isearch_endpos;
+    } else {
+	region_highlights[1].start = region_highlights[1].end = -1;
+    }
 
     if (clearlist && listshown > 0) {
 	if (tccan(TCCLEAREOD)) {


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PATCH: isearch match highlighting

[Matt Wozniski]

On Sat, Apr 26, 2008 at 6:48 PM, Peter Stephenson wrote:
> This one is worth posting.
>
>  With pattern matching in isearch, it's now quite useful to have the
>  matched string highlighted.  I've made the default underlining rather
>  than standout because of the annoyance that standout on a cursor that's
>  shown in inverse video flips it to normal video, then when you've just
>  matched one character there's nothing at all on the line.  This seems to
>  be common behaviour.
>
>  You could argue that if highlighting is done this way it would make more
>  sense for the cursor position to be where the user is typing, but I
>  it's non-trivial to change, hard to be sure the highlighting selected is
>  going to work, incompatible with usage when there's no highlighting, and
>  I don't feel like monkeying with it anyway.
>
>  I can't help feeling I must have screwed up this one somehow.

Is this working for others?  It doesn't seem to work at all for me.
zsh -f
bindkey -e
^Rb
and the shell goes into a tight loop, using 100% of one of my cores,
and won't die to anything less than a kill -9.  Anybody else see this?
 It's a fresh checkout, built with './configure --enable-multibyte
--with-term-lib=ncurses'

~Matt

Re: PATCH: isearch match highlighting

[Bart Schaefer]

On Apr 26,  8:27pm, Matt Wozniski wrote:
} Subject: Re: PATCH: isearch match highlighting
}
} On Sat, Apr 26, 2008 at 6:48 PM, Peter Stephenson wrote:
} >  I can't help feeling I must have screwed up this one somehow.
} 
} zsh -f
} bindkey -e
} ^Rb
} and the shell goes into a tight loop, using 100% of one of my cores,
} and won't die to anything less than a kill -9.  Anybody else see this?

I can reproduce this.  It's looping in

#0  0x0020fbed in memset () from /lib/tls/libc.so.6
#1  0xbfee0bb0 in ?? ()
#2  0x080c03d0 in mb_metacharlenconv_r (s=0x9783eda "", wcp=0xbfee07d0, 
    mbsp=0xbfee07b8) at ../../zsh-4.0/Src/utils.c:4008
#3  0x08116720 in doisearch (args=0x8169a1c, dir=-1, pattern=0)
    at ../../../zsh-4.0/Src/Zle/zle_hist.c:1408

mb_metacharlenconv_r() is returning 0 on the empty string (which a
comment in utils.c says "probably shouldn't happen") so charpos is
never incremented at line 1412 and the loop at line 1407 never stops.

I think this means end_pos is wrong but I don't have time to try to
chase that back through set_isearch_spot() right now, so we'll have
to wait for PWS.

Note that what's broken is the old history-incremental-search-backward.
Everything works fine with history-incremental-pattern-search-backward.

Re: PATCH: isearch match highlighting

[Peter Stephenson]

"Matt Wozniski" wrote:
> zsh -f
> bindkey -e
> ^Rb
> and the shell goes into a tight loop, using 100% of one of my cores,
> and won't die to anything less than a kill -9.

Oops.  pos wasn't yet updated.

Index: Src/Zle/zle_hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_hist.c,v
retrieving revision 1.49
diff -u -r1.49 zle_hist.c
--- Src/Zle/zle_hist.c	26 Apr 2008 22:52:51 -0000	1.49
+++ Src/Zle/zle_hist.c	27 Apr 2008 19:57:27 -0000
@@ -1323,7 +1323,7 @@
 			} else
 			    t = zlinefind(zt, pos, sbuf, dir, sens);
 			if (t)
-			    end_pos = pos + sbptr - (sbuf[0] == '^');
+			    end_pos = (t - zt) + sbptr - (sbuf[0] == '^');
 		    }
 		}
 		if (t) {


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PATCH: isearch match highlighting

[Matt Wozniski]

On Sun, Apr 27, 2008 at 3:57 PM, Peter Stephenson wrote:
> "Matt Wozniski" wrote:
>  > zsh -f
>  > bindkey -e
>  > ^Rb
>  > and the shell goes into a tight loop, using 100% of one of my cores,
>  > and won't die to anything less than a kill -9.
>
>  Oops.  pos wasn't yet updated.

Yep, works now - and looks very nice, though I wonder if we could set
it up in a way that would let us change the fg or bg color of the
text, rather than just reverse/standout/underline?

That said, I found a regression in history-incremental-search-backward
that's in CVS but not 4.3.6, so my guess is that it's caused by
something in this patch (though I didn't back this patch out to
double-check that assumption - I'm not very good at working with CVS).

zsh -f
mastermind% bindkey -e
mastermind% ^R-ff^H^He
failing bck-i-search: -e_
if you backspace again and retype the e, it changes to a succeeding bck-i-search
then, ^G and do
mastermind% ^R-f^He
bck-i-search: -e_

So, whatever the bug is, it seems to require more than one backspace
to trigger it for some reason...

~Matt

Re: PATCH: isearch match highlighting

[Peter Stephenson]

On Sun, 27 Apr 2008 21:49:01 -0400
"Matt Wozniski" <godlygeek@gmail.com> wrote:
> Yep, works now - and looks very nice, though I wonder if we could set
> it up in a way that would let us change the fg or bg color of the
> text, rather than just reverse/standout/underline?

I should get around to that before long.  I don't think it should be too
hard.  I'd like to use standard termcap AF/AB codes for this instead of the
hacked-in (surprise!) colors we currently use in completion, but it doesn't
seem to be that easy to set defaults etc. that way.  Maybe Geoff knows
about this.

> That said, I found a regression in history-incremental-search-backward
> that's in CVS but not 4.3.6, so my guess is that it's caused by
> something in this patch (though I didn't back this patch out to
> double-check that assumption - I'm not very good at working with CVS).
> 
> zsh -f
> mastermind% bindkey -e
> mastermind% ^R-ff^H^He
> failing bck-i-search: -e_
> if you backspace again and retype the e, it changes to a succeeding bck-i-search
> then, ^G and do
> mastermind% ^R-f^He
> bck-i-search: -e_
> 
> So, whatever the bug is, it seems to require more than one backspace
> to trigger it for some reason...

I've seen something like this, but unfortunately I can't get it to happen
reproducibly.  Something might not be being initialised properly.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: PATCH: isearch match highlighting

[Matt Wozniski]

On Mon, Apr 28, 2008 at 5:20 AM, Peter Stephenson wrote:
> On Sun, 27 Apr 2008 21:49:01 -0400
>  "Matt Wozniski" wrote:
>  > Yep, works now - and looks very nice, though I wonder if we could set
>  > it up in a way that would let us change the fg or bg color of the
>  > text, rather than just reverse/standout/underline?
>
>  I should get around to that before long.  I don't think it should be too
>  hard.  I'd like to use standard termcap AF/AB codes for this instead of the
>  hacked-in (surprise!) colors we currently use in completion, but it doesn't
>  seem to be that easy to set defaults etc. that way.  Maybe Geoff knows
>  about this.

Excellent - I wait with bated breath; that will be even more cool.  :)

>  > That said, I found a regression in history-incremental-search-backward
>  > that's in CVS but not 4.3.6, so my guess is that it's caused by
>  > something in this patch (though I didn't back this patch out to
>  > double-check that assumption - I'm not very good at working with CVS).
<snip testcase>
>
>  I've seen something like this, but unfortunately I can't get it to happen
>  reproducibly.  Something might not be being initialised properly.

Yep, spot on - After a backspace (backward-delete-char or
vi-backward-delete-char), we pop back up to the previous search
location.  If the search was failing at that location, we set skip_pos
to not try again at the current position, but if we then backspace
again to a valid state, we don't unset skip_pos - This is why it
needed two backspaces to trigger; we needed to go from a failing
state, to another failing state, to a good state that was being
treated as kinda-sorta-failed.

I think that the right fix is just unconditionally resetting skip_pos
right before we conditionally set it.

Index: Src/Zle/zle_hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_hist.c,v
retrieving revision 1.50
diff -U7 -r1.50 zle_hist.c
--- Src/Zle/zle_hist.c  27 Apr 2008 20:01:50 -0000      1.50
+++ Src/Zle/zle_hist.c  28 Apr 2008 10:48:05 -0000
@@ -1445,14 +1445,15 @@
        } else if(cmd == Th(z_vibackwarddeletechar) ||
                cmd == Th(z_backwarddeletechar)) {
            if (top_spot) {
                get_isrch_spot(--top_spot, &hl, &pos, &pat_hl, &pat_pos,
                               &end_pos, &zlemetacs, &sbptr, &dir, &nomatch);
                patprog = NULL;
                nosearch = 1;
+               skip_pos = 0;
            } else
                feep = 1;
            if (nomatch) {
                memcpy(ibuf, nomatch == 2 ? INVALID_TEXT : FAILING_TEXT,
                       BAD_TEXT_LEN);
                statusline = ibuf;
                skip_pos = 1;

Re: PATCH: isearch match highlighting

[Peter Stephenson]

"Matt Wozniski" wrote:
> I think that the right fix is just unconditionally resetting skip_pos
> right before we conditionally set it.

Thanks, that looks fine and I even know basically why: I just added the
"nosearch" variable to optimise out a search when we're backtracking
since we know whether it succeeded or failed at the position in
question.  That means skip_pos isn't handled quite the same way it used
to be, since it's in the optimised out code, and in particular it isn't
reset when we don't search (before it would have been even on a
failure).  As you've reset it at exactly the point nosearch is set I
think that should cover everything.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: PATCH: isearch match highlighting

[Peter Stephenson]

On Mon, 28 Apr 2008 10:20:01 +0100
Peter Stephenson <pws@csr.com> wrote:
> On Sun, 27 Apr 2008 21:49:01 -0400
> "Matt Wozniski" <godlygeek@gmail.com> wrote:
> > Yep, works now - and looks very nice, though I wonder if we could set
> > it up in a way that would let us change the fg or bg color of the
> > text, rather than just reverse/standout/underline?
> 
> I should get around to that before long.  I don't think it should be too
> hard.  I'd like to use standard termcap AF/AB codes for this instead of the
> hacked-in (surprise!) colors we currently use in completion, but it doesn't
> seem to be that easy to set defaults etc. that way.  Maybe Geoff knows
> about this.

This seems now to be Not Necessarily Completely Broken (TM).

It is currently hardwired to use ANSI sequences at least for setting the
default colour, though it will use termcap sequences where available for
non-default colours above 7, which is the limit of the ANSI set.  There is a
facility at the lower level to force it to use termcap for all non-default
colours including 0 to 7, but it's not wired to anything above it at the
moment.

I would like at the least to make the use of the ANSI escapes
configurable.  They are in completion listings, but unfortunately that's
tied to the variable interface for GNU ls colouring which doesn't really
fit the case here.  I suppose special values in zle_highlight would be
suitable (with bindkey escapes).

You'll be glad to know you can use the system without ever having to worry
about the spelling of colo[u]r.  I will try to keep it that way.

I've just remembered I haven't documented the fact that only colours 0 to
255 are allowed internally.  I can't be bothered to remake the patch, so
I'll alter that before committing.  The numbers of supported colours
reported by a few terminals I tried are:

xterm: 8
rxvt-unicode: 88
konsole: 8
gnome-terminal: 8
Linux console: 8

so mostly just the ANSI set.  I did try the extended colours on
rxvt-unicode and they seemed to work.  I have a vague feeling I saw
a terminal claim to support 256.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.64
diff -u -r1.64 zle.yo
--- Doc/Zsh/zle.yo	26 Apr 2008 22:52:51 -0000	1.64
+++ Doc/Zsh/zle.yo	29 Apr 2008 16:40:32 -0000
@@ -2097,6 +2097,24 @@
 this to appear with other types of highlighting; it is used to override
 a default.
 )
+item(tt(fg=)var(colour))(
+The foreground colour should be set to var(colour), an integer.  Not all
+terminals support this, and of those that do not all provide facilities to
+test the support, hence the user should decide based on the terminal type.
+Most terminals with colour support accept the numbers 0 to 7, and may
+generate additional colours if the tt(bold) attributes is also present.  On
+recent terminals and on systems with an up-to-date terminal database the
+number of colours supported may be tested by with the command `tt(echotc
+Co)'; if this succeeds, it indicates a limit on the number of colours which
+will be enforced by the line editor.
+
+Colour is also known as color.
+)
+item(tt(bg=)var(colour))(
+The background colour should be set to var(colour), an integer.  This
+works similarly to the foreground colour, except the background is
+not usually affected by the bold attribute.
+)
 item(tt(bold))(
 The characters in the given context are shown in a bold font.
 )
Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.82
diff -u -r1.82 init.c
--- Src/init.c	26 Apr 2008 17:46:47 -0000	1.82
+++ Src/init.c	29 Apr 2008 16:40:33 -0000
@@ -79,6 +79,11 @@
 /**/
 mod_export int hasam, hasxn;
 
+/* Value of the Co (max_colors) entry: may not be set */
+
+/**/
+mod_export int tccolours;
+
 /* Pointer to read-key function from zle */
 
 /**/
@@ -531,7 +536,7 @@
     "cl", "le", "LE", "nd", "RI", "up", "UP", "do",
     "DO", "dc", "DC", "ic", "IC", "cd", "ce", "al", "dl", "ta",
     "md", "so", "us", "me", "se", "ue", "ch",
-    "ku", "kd", "kl", "kr", "sc", "rc", "bc"
+    "ku", "kd", "kl", "kr", "sc", "rc", "bc", "AF", "AB"
 };
 
 /* Initialise termcap */
@@ -590,6 +595,7 @@
 
 	tclines = tgetnum("li");
 	tccolumns = tgetnum("co");
+	tccolours = tgetnum("Co");
 
 	/* if there's no termcap entry for cursor up, use single line mode: *
 	 * this is flagged by termflags which is examined in zle_refresh.c  *
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.127
diff -u -r1.127 zsh.h
--- Src/zsh.h	26 Apr 2008 19:51:09 -0000	1.127
+++ Src/zsh.h	29 Apr 2008 16:40:33 -0000
@@ -1949,7 +1949,9 @@
 #define TCSAVECURSOR   29
 #define TCRESTRCURSOR  30
 #define TCBACKSPACE    31
-#define TC_COUNT       32
+#define TCFGCOLOUR     32
+#define TCBGCOLOUR     33
+#define TC_COUNT       34
 
 #define tccan(X) (tclen[X])
 
@@ -1957,32 +1959,63 @@
  * Text attributes for displaying in ZLE
  */
 
-#define TXTBOLDFACE   0x01
-#define TXTSTANDOUT   0x02
-#define TXTUNDERLINE  0x04
-#define TXTDIRTY      0x80
+#define TXTBOLDFACE   0x0001
+#define TXTSTANDOUT   0x0002
+#define TXTUNDERLINE  0x0004
+#define TXTFGCOLOUR   0x0008
+#define TXTBGCOLOUR   0x0010
+#define TXTDIRTY      0x0020
 
-#define TXT_ATTR_ON_MASK   0x07
+#define TXT_ATTR_ON_MASK   0x001F
 
 #define txtisset(X)  (txtattrmask & (X))
 #define txtset(X)    (txtattrmask |= (X))
 #define txtunset(X)  (txtattrmask &= ~(X))
 
-#define TXTNOBOLDFACE	0x10
-#define TXTNOSTANDOUT	0x20
-#define TXTNOUNDERLINE	0x40
+#define TXTNOBOLDFACE	0x0040
+#define TXTNOSTANDOUT	0x0080
+#define TXTNOUNDERLINE	0x0100
+#define TXTNOFGCOLOUR	0x0200
+#define TXTNOBGCOLOUR	0x0400
 
-#define TXT_ATTR_OFF_MASK  0x70
+#define TXT_ATTR_OFF_MASK  0x07C0
 /* Bits to shift off right to get on */
-#define TXT_ATTR_OFF_ON_SHIFT (4)
-
+#define TXT_ATTR_OFF_ON_SHIFT 6
+#define TXT_ATTR_OFF_FROM_ON(attr)	\
+    (((attr) & TXT_ATTR_ON_MASK) << TXT_ATTR_OFF_ON_SHIFT)
 /*
  * Indicates to zle_refresh.c that the character entry is an
  * index into the list of multiword symbols.
  */
-#define TXT_MULTIWORD_MASK  0x100
+#define TXT_MULTIWORD_MASK  0x0800
+
+/* Mask for colour to use in foreground */
+#define TXT_ATTR_FG_COL_MASK     0x000FF000
+/* Bits to shift the foreground colour */
+#define TXT_ATTR_FG_COL_SHIFT    (12)
+/* Mask for colour to use in background */
+#define TXT_ATTR_BG_COL_MASK     0x0FF00000
+/* Bits to shift the background colour */
+#define TXT_ATTR_BG_COL_SHIFT    (20)
+
+/* Flag to use termcap AF sequence to set colour, if available */
+#define TXT_ATTR_FG_TERMCAP      0x10000000
+/* Flag to use termcap AB sequence to set colour, if available */
+#define TXT_ATTR_BG_TERMCAP      0x20000000
+
+/* Things to turn on, including values for the colour elements */
+#define TXT_ATTR_ON_VALUES_MASK	\
+    (TXT_ATTR_ON_MASK|TXT_ATTR_FG_COL_MASK|TXT_ATTR_BG_COL_MASK|\
+     TXT_ATTR_FG_TERMCAP|TXT_ATTR_BG_TERMCAP)
+
+/* Mask out everything to do with activating colours */
+#define TXT_ATTR_COLOUR_ON_MASK			\
+    (TXTFGCOLOUR|TXTBGCOLOUR|			\
+     TXT_ATTR_FG_COL_MASK|TXT_ATTR_BG_COL_MASK| \
+     TXT_ATTR_FG_TERMCAP|TXT_ATTR_BG_TERMCAP)
 
 #define txtchangeisset(T,X)	((T) & (X))
+#define txtchangeget(T,A)	(((T) & A ## _MASK) >> A ## _SHIFT)
 #define txtchangeset(X, Y)	(txtchange |= (X), txtchange &= ~(Y))
 
 /****************************************/
Index: Src/Zle/zle_refresh.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_refresh.c,v
retrieving revision 1.63
diff -u -r1.63 zle_refresh.c
--- Src/Zle/zle_refresh.c	29 Apr 2008 08:43:39 -0000	1.63
+++ Src/Zle/zle_refresh.c	29 Apr 2008 16:40:34 -0000
@@ -207,7 +207,7 @@
  * displayed on screen.
  */
 
-static int special_atr_on, special_atr_off;
+static int special_atr_on;
 
 /* Flags for the region_highlight structure */
 enum {
@@ -357,19 +357,41 @@
 	const struct highlight *hl;
 
 	found = 0;
-	for (hl = highlights; hl->name; hl++) {
-	    if (strpfx(hl->name, teststr)) {
-		const char *val = teststr + strlen(hl->name);
-
-		if (*val == ',')
-		    val++;
-		else if (*val)
-		    break;
+	if (strpfx("fg=", teststr) || strpfx("bg=", teststr)) {
+	    int is_fg = (teststr[0] == 'f');
+	    int colour = (int)zstrtol(teststr+3, (char **)&teststr, 10);
+	    int shft, on;
+	    if (*teststr == ',')
+		teststr++;
+	    else if (*teststr)
+		break;
+	    found = 1;
+	    /* skip out of range colours but keep scanning attributes */
+	    if (colour >= 256)
+		continue;
+	    if (is_fg) {
+		shft = TXT_ATTR_FG_COL_SHIFT;
+		on = TXTFGCOLOUR;
+	    } else {
+		shft = TXT_ATTR_BG_COL_SHIFT;
+		on = TXTBGCOLOUR;
+	    }
+	    *on_var |= on | (colour << shft);
+	} else {
+	    for (hl = highlights; hl->name; hl++) {
+		if (strpfx(hl->name, teststr)) {
+		    const char *val = teststr + strlen(hl->name);
+
+		    if (*val == ',')
+			val++;
+		    else if (*val)
+			break;
 
-		*on_var |= hl->mask_on;
-		*on_var &= ~hl->mask_off;
-		teststr = val;
-		found = 1;
+		    *on_var |= hl->mask_on;
+		    *on_var &= ~hl->mask_off;
+		    teststr = val;
+		    found = 1;
+		}
 	    }
 	}
     }
@@ -431,7 +453,6 @@
 	region_highlights->atr = TXTSTANDOUT;
     if (!isearch_atr_on_set)
 	region_highlights[1].atr = TXTUNDERLINE;
-    special_atr_off = special_atr_on << TXT_ATTR_OFF_ON_SHIFT;
 }
 
 
@@ -610,19 +631,23 @@
 
     if (lastatr & ~c->atr) {
 	/* Stuff on we don't want, turn it off */
-	settextattributes((lastatr & ~c->atr) << TXT_ATTR_OFF_ON_SHIFT);
+	settextattributes(TXT_ATTR_OFF_FROM_ON(lastatr & ~c->atr));
 	lastatr = 0;
     }
 
     /*
      * Don't output "on" attributes in a string of characters with
-     * the same attributes.
+     * the same attributes.  Be careful in case a different colour
+     * needs setting.
      */
     if ((c->atr & TXT_ATTR_ON_MASK) &&
 	(!curatrp ||
-	 ((*curatrp & TXT_ATTR_ON_MASK) != (c->atr & TXT_ATTR_ON_MASK)))) {
+	 ((*curatrp & TXT_ATTR_ON_VALUES_MASK) !=
+	  (c->atr & TXT_ATTR_ON_VALUES_MASK)))) {
+	/* Record just the control flags we might need to turn off... */
 	lastatr = c->atr & TXT_ATTR_ON_MASK;
-	settextattributes(lastatr);
+	/* ...but set including the values for colour attributes */
+	settextattributes(c->atr & TXT_ATTR_ON_VALUES_MASK);
     }
 
 #ifdef MULTIBYTE_SUPPORT
@@ -656,9 +681,11 @@
     if (curatrp) {
 	/*
 	 * Remember the current attributes:  those that are turned
-	 * on, less those that are turned off again.
+	 * on, less those that are turned off again.  Include
+	 * colour attributes here in case the colour changes to
+	 * another non-default one.
 	 */
-	*curatrp = (c->atr & TXT_ATTR_ON_MASK) &
+	*curatrp = (c->atr & TXT_ATTR_ON_VALUES_MASK) &
 	    ~((c->atr & TXT_ATTR_OFF_MASK) >> TXT_ATTR_OFF_ON_SHIFT);
     }
 }
@@ -915,6 +942,54 @@
     rpms->sen = rpms->s + winw;
 }
 
+/*
+ * HERE: these need to be made configurable, somehow.
+ * Ideally we need to make the complist stuff use the
+ * same system, but that may be too much tied to the GNU ls
+ * interface to make that possible.
+ */
+/* Start of escape sequence for foreground colour */
+#define TC_COL_FG_START	"\033[3"
+/* Start of escape sequence for background colour */
+#define TC_COL_BG_START	"\033[4"
+/* End of either escape sequence */
+#define TC_COL_END	"m"
+/* Numeric code (to be turned into ASCII) to reset default colour */
+#define TC_COL_DEFAULT	9
+
+static void
+setcolourattribute(int colour, char *start, int tc, int def,
+		   int use_termcap)
+{
+    char out[16], *ptr;
+    /*
+     * If we're not restoring the default, and either have a
+     * colour value that is too large for ANSI, or have been told
+     * to use the termcap sequence (which at the time of writing
+     * we never are), try to use the termcap sequence.
+     */
+    if (!def && (colour > 7 || use_termcap)) {
+	/*
+	 * We can if it's available, and either we couldn't get
+	 * the maximum number of colours, or the colour is in range.
+	 */
+	if (tccan(tc) && (tccolours < 0 || colour < tccolours))
+	    tcoutarg(tc, colour);
+	/* for 0 to 7 assume standard ANSI works, otherwise it won't. */
+	if (colour > 7)
+	    return;
+    }
+
+    strcpy(out, start);
+    if (def)
+	colour = TC_COL_DEFAULT;
+
+    ptr = out + strlen(start);
+    *ptr++ = colour + '0';
+    strcpy(ptr, TC_COL_END);
+    tputs(out, 1, putshout);
+}
+
 /**/
 static void
 settextattributes(int atr)
@@ -931,6 +1006,18 @@
 	tsetcap(TCSTANDOUTBEG, 0);
     if (txtchangeisset(atr, TXTUNDERLINE))
 	tsetcap(TCUNDERLINEBEG, 0);
+    if (txtchangeisset(atr, TXTFGCOLOUR|TXTNOFGCOLOUR)) {
+	setcolourattribute(txtchangeget(atr, TXT_ATTR_FG_COL),
+			   TC_COL_FG_START, TCFGCOLOUR,
+			   txtchangeisset(atr, TXTNOFGCOLOUR),
+			   txtchangeisset(atr, TXT_ATTR_FG_TERMCAP));
+    }
+    if (txtchangeisset(atr, TXTBGCOLOUR|TXTNOBGCOLOUR)) {
+	setcolourattribute(txtchangeget(atr, TXT_ATTR_BG_COL),
+			   TC_COL_BG_START, TCBGCOLOUR,
+			   txtchangeisset(atr, TXTNOBGCOLOUR),
+			   txtchangeisset(atr, TXT_ATTR_BG_TERMCAP));
+    }
 }
 
 #ifdef MULTIBYTE_SUPPORT
@@ -1209,6 +1296,7 @@
     rpms.sen = *nbuf + winw;
     for (t = tmpline, tmppos = 0; tmppos < tmpll; t++, tmppos++) {
 	int base_atr_on = 0, base_atr_off = 0, ireg;
+	int all_atr_on, all_atr_off;
 	struct region_highlight *rhp;
 	/*
 	 * Calculate attribute based on region.
@@ -1223,12 +1311,27 @@
 		offset = predisplaylen; /* increment over it */
 	    if (rhp->start + offset <= tmppos &&
 		tmppos < rhp->end + offset) {
-		base_atr_on |= rhp->atr;
+		if (base_atr_on & (TXTFGCOLOUR|TXTBGCOLOUR)) {
+		    /* keep colour already set */
+		    base_atr_on |= rhp->atr & ~TXT_ATTR_COLOUR_ON_MASK;
+		} else {
+		    /* no colour set yet */
+		    base_atr_on |= rhp->atr;
+		}
 		if (tmppos == rhp->end + offset - 1 ||
 		    tmppos == tmpll - 1)
-		    base_atr_off |= rhp->atr << TXT_ATTR_OFF_ON_SHIFT;
+		    base_atr_off |= TXT_ATTR_OFF_FROM_ON(rhp->atr);
 	    }
 	}
+	if (special_atr_on & (TXTFGCOLOUR|TXTBGCOLOUR)) {
+	    /* keep colours from special attributes */
+	    all_atr_on = special_atr_on |
+		(base_atr_on & ~TXT_ATTR_COLOUR_ON_MASK);
+	} else {
+	    /* keep colours from standard attributes */
+	    all_atr_on = special_atr_on | base_atr_on;
+	}
+	all_atr_off = TXT_ATTR_OFF_FROM_ON(all_atr_on);
 
 	if (t == scs)			/* if cursor is here, remember it */
 	    rpms.nvcs = rpms.s - nbuf[rpms.nvln = rpms.ln];
@@ -1264,11 +1367,11 @@
 		    rpms.s->chr = ZWC(' ');
 		    if (!started)
 			started = 1;
-		    rpms.s->atr = special_atr_on | base_atr_on;
+		    rpms.s->atr = all_atr_on;
 		    rpms.s++;
 		} while (rpms.s < rpms.sen);
 		if (started)
-		    rpms.s[-1].atr |= special_atr_off | base_atr_off;
+		    rpms.s[-1].atr |= all_atr_off;
 		if (nextline(&rpms, 1))
 		    break;
 		if (t == scs) {
@@ -1292,8 +1395,7 @@
 		 * occurrence.
 		 */
 		rpms.s->chr = ZWC('?');
-		rpms.s->atr = special_atr_on | special_atr_off |
-		    base_atr_on | base_atr_off;
+		rpms.s->atr = all_atr_on | all_atr_off;
 		rpms.s++;
 	    } else {
 		/* We can fit it without reaching the end of the line. */
@@ -1334,18 +1436,17 @@
 #endif
 	    ) {	/* other control character */
 	    rpms.s->chr = ZWC('^');
-	    rpms.s->atr = special_atr_on | base_atr_on;
+	    rpms.s->atr = all_atr_on;
 	    rpms.s++;
 	    if (rpms.s == rpms.sen) {
 		/* text wrapped */
-		rpms.s[-1].atr |= special_atr_off | base_atr_off;
+		rpms.s[-1].atr |= all_atr_off;
 		if (nextline(&rpms, 1))
 		    break;
 	    }
 	    rpms.s->chr = (((unsigned int)*t & ~0x80u) > 31) ?
 		ZWC('?') : (*t | ZWC('@'));
-	    rpms.s->atr = special_atr_on | special_atr_off |
-		base_atr_on | base_atr_off;
+	    rpms.s->atr = all_atr_on | all_atr_off;
 	    rpms.s++;
 	}
 #ifdef MULTIBYTE_SUPPORT
@@ -1370,12 +1471,12 @@
 		    rpms.s->chr = wc;
 		    if (!started)
 			started = 1;
-		    rpms.s->atr = special_atr_on | base_atr_on;
+		    rpms.s->atr = all_atr_on;
 		    rpms.s++;
 		    if (rpms.s == rpms.sen) {
 			/* text wrapped */
 			if (started) {
-			    rpms.s[-1].atr |= special_atr_off | base_atr_off;
+			    rpms.s[-1].atr |= all_atr_off;
 			    started = 0;
 			}
 			if (nextline(&rpms, 1))
@@ -1385,7 +1486,7 @@
 		dispptr++;
 	    }
 	    if (started)
-		rpms.s[-1].atr |= special_atr_off | base_atr_off;
+		rpms.s[-1].atr |= all_atr_off;
 	    if (*dispptr) /* nextline said stop processing */
 		break;
 	}
@@ -1417,11 +1518,14 @@
 	more_end = 1;
 
     if (statusline) {
-	int outll, outsz;
+	int outll, outsz, all_atr_on, all_atr_off;
 	char *statusdup = ztrdup(statusline);
 	ZLE_STRING_T outputline =
 	    stringaszleline(statusdup, 0, &outll, &outsz, NULL); 
 
+	all_atr_on = special_atr_on;
+	all_atr_off = TXT_ATTR_OFF_FROM_ON(all_atr_on);
+
 	rpms.tosln = rpms.ln + 1;
 	nbuf[rpms.ln][winw + 1] = zr_zr;	/* text not wrapped */
 	snextline(&rpms);
@@ -1440,7 +1544,7 @@
 		}
 		if (width > rpms.sen - rpms.s) {
 		    rpms.s->chr = ZWC('?');
-		    rpms.s->atr = special_atr_on | special_atr_off;
+		    rpms.s->atr = all_atr_on | all_atr_off;
 		    rpms.s++;
 		} else {
 		    rpms.s->chr = *u;
@@ -1457,7 +1561,7 @@
 #endif
 	    if (ZC_icntrl(*u)) { /* simplified processing in the status line */
 		rpms.s->chr = ZWC('^');
-		rpms.s->atr = special_atr_on;
+		rpms.s->atr = all_atr_on;
 		rpms.s++;
 		if (rpms.s == rpms.sen) {
 		    nbuf[rpms.ln][winw + 1] = zr_nl;/* text wrapped */
@@ -1465,7 +1569,7 @@
 		}
 		rpms.s->chr = (((unsigned int)*u & ~0x80u) > 31)
 		    ? ZWC('?') : (*u | ZWC('@'));
-		rpms.s->atr = special_atr_on | special_atr_off;
+		rpms.s->atr = all_atr_on | all_atr_off;
 		rpms.s++;
 	    } else {
 		rpms.s->chr = *u;
@@ -2037,7 +2141,7 @@
 	     */
 	    int now_off = ol->atr & ~nl->atr & TXT_ATTR_ON_MASK;
 	    if (now_off)
-		settextattributes(now_off << TXT_ATTR_OFF_ON_SHIFT);
+		settextattributes(TXT_ATTR_OFF_FROM_ON(now_off));
 
 	    zputc(nl);
 	    nl++, ol++;
@@ -2324,7 +2428,7 @@
     if (tmpcs < 0) {
 #ifdef DEBUG
 	fprintf(stderr, "BUG: negative cursor position\n");
-	fflush(stderr); 
+	fflush(stderr);
 #endif
 	tmpcs = 0;
     }
@@ -2336,6 +2440,7 @@
 
     for (t0 = 0; t0 < tmpll; t0++) {
 	int base_atr_on = 0, base_atr_off = 0, ireg;
+	int all_atr_on, all_atr_off;
 	struct region_highlight *rhp;
 	/*
 	 * Calculate attribute based on region.
@@ -2350,12 +2455,27 @@
 		offset = predisplaylen; /* increment over it */
 	    if (rhp->start + offset <= t0 &&
 		t0 < rhp->end + offset) {
-		base_atr_on |= rhp->atr;
+		if (base_atr_on & (TXTFGCOLOUR|TXTBGCOLOUR)) {
+		    /* keep colour already set */
+		    base_atr_on |= rhp->atr & ~TXT_ATTR_COLOUR_ON_MASK;
+		} else {
+		    /* no colour set yet */
+		    base_atr_on |= rhp->atr;
+		}
 		if (t0 == rhp->end + offset - 1 ||
 		    t0 == tmpll - 1)
-		    base_atr_off |= rhp->atr << TXT_ATTR_OFF_ON_SHIFT;
+		    base_atr_off |= TXT_ATTR_OFF_FROM_ON(rhp->atr);
 	    }
 	}
+	if (special_atr_on & (TXTFGCOLOUR|TXTBGCOLOUR)) {
+	    /* keep colours from special attributes */
+	    all_atr_on = special_atr_on |
+		(base_atr_on & ~TXT_ATTR_COLOUR_ON_MASK);
+	} else {
+	    /* keep colours from standard attributes */
+	    all_atr_on = special_atr_on | base_atr_on;
+	}
+	all_atr_off = TXT_ATTR_OFF_FROM_ON(all_atr_on);
 
 	if (tmpline[t0] == ZWC('\t')) {
 	    REFRESH_ELEMENT sp = zr_sp;
@@ -2365,11 +2485,10 @@
 	    vp[-1].atr |= base_atr_off;
 	} else if (tmpline[t0] == ZWC('\n')) {
 	    vp->chr = ZWC('\\');
-	    vp->atr = special_atr_on | base_atr_on;
+	    vp->atr = all_atr_on;
 	    vp++;
 	    vp->chr = ZWC('n');
-	    vp->atr = special_atr_on | special_atr_off |
-		base_atr_on | base_atr_off;
+	    vp->atr = all_atr_on | all_atr_off;
 	    vp++;
 #ifdef MULTIBYTE_SUPPORT
 	} else if (iswprint(tmpline[t0]) &&
@@ -2406,12 +2525,11 @@
 	    ZLE_INT_T t = tmpline[++t0];
 
 	    vp->chr = ZWC('^');
-	    vp->atr = special_atr_on | base_atr_on;
+	    vp->atr = all_atr_on;
 	    vp++;
 	    vp->chr = (((unsigned int)t & ~0x80u) > 31) ?
 		ZWC('?') : (t | ZWC('@'));
-	    vp->atr = special_atr_on | special_atr_off | base_atr_on |
-		base_atr_off;
+	    vp->atr = all_atr_on | all_atr_off;
 	    vp++;
 	}
 #ifdef MULTIBYTE_SUPPORT
@@ -2431,13 +2549,13 @@
 		    vp->chr = wc;
 		    if (!started)
 			started = 1;
-		    vp->atr = special_atr_on | base_atr_on;
+		    vp->atr = all_atr_on;
 		    vp++;
 		}
 		dispptr++;
 	    }
 	    if (started)
-		vp[-1].atr |= special_atr_off | base_atr_off;
+		vp[-1].atr |= all_atr_off;
 	}
 #else
 	else {
-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: PATCH: isearch match highlighting

[Matt Wozniski]

On Tue, Apr 29, 2008 at 1:10 PM, Peter Stephenson <pws@csr.com> wrote:
> On Mon, 28 Apr 2008 10:20:01 +0100
>  Peter Stephenson <pws@csr.com> wrote:
>  > On Sun, 27 Apr 2008 21:49:01 -0400
>  > "Matt Wozniski" <godlygeek@gmail.com> wrote:
>  > > Yep, works now - and looks very nice, though I wonder if we could set
>  > > it up in a way that would let us change the fg or bg color of the
>  > > text, rather than just reverse/standout/underline?
>  >
>  > I should get around to that before long.  I don't think it should be too
>  > hard.  I'd like to use standard termcap AF/AB codes for this instead of the
>  > hacked-in (surprise!) colors we currently use in completion, but it doesn't
>  > seem to be that easy to set defaults etc. that way.  Maybe Geoff knows
>  > about this.
>
>  This seems now to be Not Necessarily Completely Broken (TM).

Seems to work quite well to me!

>  I've just remembered I haven't documented the fact that only colours 0 to
>  255 are allowed internally.  I can't be bothered to remake the patch, so
>  I'll alter that before committing.  The numbers of supported colours
>  reported by a few terminals I tried are:
>
>  xterm: 8
>  rxvt-unicode: 88
>  konsole: 8
>  gnome-terminal: 8
>  Linux console: 8
>
>  so mostly just the ANSI set.  I did try the extended colours on
>  rxvt-unicode and they seemed to work.  I have a vague feeling I saw
>  a terminal claim to support 256.

xterm-256color: 256
screen-256color: 256

Both xterm and screen can be compiled with 256 color support.
Extended colors seem to work fine in them.

~Matt

PS: Found a typo in the man page while playing with this:

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.65
diff -r1.65 zle.yo
2053c2053
< by the array parameter tt(zsh_highlight), if it has been set by the user.
---
> by the array parameter tt(zle_highlight), if it has been set by the user.

Re: PATCH: isearch match highlighting

[Matt Wozniski]

On Tue, Apr 29, 2008 at 4:12 PM, Matt Wozniski <godlygeek@gmail.com> wrote:
> On Tue, Apr 29, 2008 at 1:10 PM, Peter Stephenson <pws@csr.com> wrote:
>  >  I've just remembered I haven't documented the fact that only colours 0 to
>  >  255 are allowed internally.  I can't be bothered to remake the patch, so
>  >  I'll alter that before committing.  The numbers of supported colours
>  >  reported by a few terminals I tried are:
>  >
>  >  xterm: 8
>  >  rxvt-unicode: 88
>  >  konsole: 8
>  >  gnome-terminal: 8
>  >  Linux console: 8
>  >
>  >  so mostly just the ANSI set.  I did try the extended colours on
>  >  rxvt-unicode and they seemed to work.  I have a vague feeling I saw
>  >  a terminal claim to support 256.
>
>  xterm-256color: 256
>  screen-256color: 256
>
>  Both xterm and screen can be compiled with 256 color support.

Actually, FYI, gnome-terminal and konsole also support 256 colors;
TERM=gnome-256color and TERM=konsole-256color. Konsole, in fact,
supports all 24-bit colors through its own extended syntax (CSI 38 ; 2
; <rrr> ; <ggg> ; <bbb> m), though I don't know of any apps that use
it...  Though we could allow it here if we had the same "send this
string to start highlighting and this string to end" sort of thing
like we can do in prompts...

~Matt

Re: PATCH: isearch match highlighting

[Peter Stephenson]

On Tue, 29 Apr 2008 18:10:22 +0100
Peter Stephenson <pws@csr.com> wrote:
> I would like at the least to make the use of the ANSI escapes
> configurable.  They are in completion listings, but unfortunately that's
> tied to the variable interface for GNU ls colouring which doesn't really
> fit the case here.  I suppose special values in zle_highlight would be
> suitable (with bindkey escapes).

I've done this.  Looks suspiciously like overkill, but it's probably best
to be general.

I've also added handling of names for ANSI colours.

Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.65
diff -u -r1.65 zle.yo
--- Doc/Zsh/zle.yo	29 Apr 2008 17:19:26 -0000	1.65
+++ Doc/Zsh/zle.yo	1 May 2008 10:35:05 -0000
@@ -2088,6 +2088,39 @@
 )
 enditem()
 
+tt(zle_highlight) may contain additional fields for controlling how
+terminal sequences to change colours are output.  Each of the following is
+followed by a colon and a string in the same form as for key bindings.
+This will not be necessary for the vast majority of terminals as the
+defaults shown in parentheses are widely used.
+
+startitem()
+cindex(escape sequences, terminal, for highlighting)
+cindex(terminal escape sequences for highlighting)
+item(tt(fg_start_code) (tt(\e[3)))(
+The start of the escape sequence for the foreground colour.
+This is followed by an ASCII digit representing the colour.
+)
+item(tt(fg_default_code) (tt(9)))(
+The number to use instead of the colour to reset the default foreground
+colour.
+)
+item(tt(fg_end_code) (tt(m)))(
+The end of the escape sequence for the foreground colour.
+)
+item(tt(bg_start_code) (tt(\e[4)))(
+The start of the escape sequence for the background colour.
+This is followed by an ASCII digit representing the colour.
+)
+item(tt(bg_default_code) (tt(9)))(
+The number to use instead of the colour to reset the default
+background colour.
+)
+item(tt(bg_end_code) (tt(m)))(
+The end of the escape sequence for the background colour.
+)
+enditem()
+
 The available types of highlighting are the following.  Note that
 not all types of highlighting are available on all terminals:
 
@@ -2098,11 +2131,19 @@
 a default.
 )
 item(tt(fg=)var(colour))(
-The foreground colour should be set to var(colour), a decimal integer.  Not
-all terminals support this, and of those that do not all provide facilities
-to test the support, hence the user should decide based on the terminal
-type.  Most terminals with colour support accept the numbers 0 to 7, and
-may generate additional colours if the tt(bold) attributes is also present.
+The foreground colour should be set to var(colour), a decimal integer
+or the name of one of the eight most widely-supported colours.
+
+Not all terminals support this and, of those that do, not all provide
+facilities to test the support, hence the user should decide based on the
+terminal type.  Most terminals with colour support accept the numbers 0 to
+7, and may generate additional colours if the tt(bold) attributes is also
+present.  Most terminals also have a standard range of colours for those
+numbers (though the interpretation of the colour can vary); these colours
+can be set by one of the names tt(black), tt(red), tt(green), tt(yellow),
+tt(blue), tt(magenta), tt(cyan) and tt(white).  Abbreviations are
+allowed; tt(b) or tt(bl) selects black.
+
 On recent terminals and on systems with an up-to-date terminal database the
 number of colours supported may be tested by with the command `tt(echotc
 Co)'; if this succeeds, it indicates a limit on the number of colours which
@@ -2112,7 +2153,7 @@
 Colour is also known as color.
 )
 item(tt(bg=)var(colour))(
-The background colour should be set to var(colour), a decimal integer.
+The background colour should be set to var(colour).
 This works similarly to the foreground colour, except the background is
 not usually affected by the bold attribute.
 )
@@ -2144,12 +2185,18 @@
 `tt(^)' followed by the base character.
 )
 item(Unprintable multibyte characters)(
-If the tt(MULTIBYTE) option is in effect,
-multibyte characters not in the ASCII character set that are reported as
-having zero width are shown as a hexadecimal number between
-angle brackets.  The number is the code point of the character in
-the wide character set; this may or may not be Unicode, depending
-on the operating system.
+This item applies to control characters not in the ASCII range,
+plus other characters as follows.  If the tt(MULTIBYTE) option is in
+effect, multibyte characters not in the ASCII character set that are
+reported as having zero width are treated as combining characters when the
+option tt(COMBINING_CHARS) is on.  If the option is off, or if a character
+appears where a combining character is not valid, the character
+is treated as unprintable.
+
+Unprintable multibyte characters are shown as a hexadecimal number between
+angle brackets.  The number is the code point of the character in the wide
+character set; this may or may not be Unicode, depending on the operating
+system.
 )
 enditem()
 
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.110
diff -u -r1.110 zle_main.c
--- Src/Zle/zle_main.c	20 Apr 2008 21:17:29 -0000	1.110
+++ Src/Zle/zle_main.c	1 May 2008 10:35:06 -0000
@@ -1903,6 +1903,7 @@
     addhookfunc("before_trap", (Hookfn) zlebeforetrap);
     addhookfunc("after_trap", (Hookfn) zleaftertrap);
     (void)addhookdefs(m, zlehooks, sizeof(zlehooks)/sizeof(*zlehooks));
+    zle_refresh_boot();
     return 0;
 }
 
Index: Src/Zle/zle_refresh.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_refresh.c,v
retrieving revision 1.64
diff -u -r1.64 zle_refresh.c
--- Src/Zle/zle_refresh.c	29 Apr 2008 17:19:27 -0000	1.64
+++ Src/Zle/zle_refresh.c	1 May 2008 10:35:06 -0000
@@ -326,6 +326,11 @@
 #define ZR_START_ELLIPSIS_SIZE	\
     ((int)(sizeof(zr_start_ellipsis)/sizeof(zr_start_ellipsis[0])))
 
+/* Defines standard ANSI colour names in index order */
+static const char *ansi_colours[] = {
+    "black", "red", "green", "yellow", "blue", "magenta", "cyan", "white",
+    NULL
+};
 
 /* Defines the available types of highlighting */
 struct highlight {
@@ -342,6 +347,101 @@
     { NULL, 0, 0 }
 };
 
+/* Structure and array for holding special colour terminal sequences */
+
+/* Start of escape sequence for foreground colour */
+#define TC_COL_FG_START	"\033[3"
+/* End of escape sequence for foreground colour */
+#define TC_COL_FG_END	"m"
+/* Code to reset foreground colour */
+#define TC_COL_FG_DEFAULT	"9"
+
+/* Start of escape sequence for background colour */
+#define TC_COL_BG_START	"\033[4"
+/* End of escape sequence for background colour */
+#define TC_COL_BG_END	"m"
+/* Code to reset background colour */
+#define TC_COL_BG_DEFAULT	"9"
+
+struct colour_sequences {
+    char *start;		/* Escape sequence start */
+    char *end;			/* Escape sequence terminator */
+    char *def;			/* Code to reset default colour */
+};
+struct colour_sequences fg_bg_sequences[2];
+
+#define COL_SEQ_FG	(0)
+#define COL_SEQ_BG	(1)
+#define COL_SEQ_COUNT	(2)
+
+/*
+ * We need a buffer for colour sequence compostion.  It may
+ * vary depending on the sequences set.  However, it's inefficient
+ * allocating it separately every time we send a colour sequence,
+ * so do it once per refresh.
+ */
+static char *colseq_buf;
+
+static void
+set_default_colour_sequences(void)
+{
+    fg_bg_sequences[COL_SEQ_FG].start = ztrdup(TC_COL_FG_START);
+    fg_bg_sequences[COL_SEQ_FG].end = ztrdup(TC_COL_FG_END);
+    fg_bg_sequences[COL_SEQ_FG].def = ztrdup(TC_COL_FG_DEFAULT);
+
+    fg_bg_sequences[COL_SEQ_BG].start = ztrdup(TC_COL_BG_START);
+    fg_bg_sequences[COL_SEQ_BG].end = ztrdup(TC_COL_BG_END);
+    fg_bg_sequences[COL_SEQ_BG].def = ztrdup(TC_COL_BG_DEFAULT);
+}
+
+static void
+free_colour_sequences(void)
+{
+    int i;
+
+    for (i = 0; i < COL_SEQ_COUNT; i++) {
+	zsfree(fg_bg_sequences[i].start);
+	zsfree(fg_bg_sequences[i].end);
+	zsfree(fg_bg_sequences[i].def);
+    }
+}
+
+/*
+ * Return index of ANSI colour for which *teststrp is an abbreviation.
+ * Any non-alphabetic character ends the abbreviation.
+ */
+
+static int
+match_colour(const char **teststrp)
+{
+    const char *teststr = *teststrp, *end, **cptr;
+    int len;
+
+    for (end = teststr; ialpha(*end); end++)
+	;
+    len = end - teststr;
+    *teststrp = end;
+
+    for (cptr = ansi_colours; *cptr; cptr++) {
+	if (!strncmp(teststr, *cptr, len))
+	    return cptr - ansi_colours;
+    }
+
+    return -1;
+}
+
+static void
+set_colour_code(char *str, char **var)
+{
+    char *keyseq;
+    int len;
+
+    zsfree(*var);
+    keyseq = getkeystring(str, &len, GETKEYS_BINDKEY, NULL);
+    *var = metafy(keyseq, len, META_DUP);
+}
+
+
 /*
  * Match a set of highlights in the given teststr.
  * Set *on_var to reflect the values found.
@@ -359,15 +459,20 @@
 	found = 0;
 	if (strpfx("fg=", teststr) || strpfx("bg=", teststr)) {
 	    int is_fg = (teststr[0] == 'f');
-	    int colour = (int)zstrtol(teststr+3, (char **)&teststr, 10);
-	    int shft, on;
+	    int colour, shft, on;
+
+	    teststr += 3;
+	    if (ialpha(*teststr))
+		colour = match_colour(&teststr);
+	    else
+		colour = (int)zstrtol(teststr, (char **)&teststr, 10);
 	    if (*teststr == ',')
 		teststr++;
 	    else if (*teststr)
 		break;
 	    found = 1;
 	    /* skip out of range colours but keep scanning attributes */
-	    if (colour >= 256)
+	    if (colour < 0 || colour >= 256)
 		continue;
 	    if (is_fg) {
 		shft = TXT_ATTR_FG_COL_SHIFT;
@@ -404,12 +509,14 @@
  */
 
 /**/
-void zle_set_highlight(void)
+static void
+zle_set_highlight(void)
 {
     char **atrs = getaparam("zle_highlight");
     int special_atr_on_set = 0;
     int region_atr_on_set = 0;
     int isearch_atr_on_set = 0;
+    int lenfg, lenbg, len;
     struct region_highlight *rhp;
 
     special_atr_on = 0;
@@ -442,6 +549,18 @@
 	    } else if (strpfx("isearch:", *atrs)) {
 		match_highlight(*atrs + 8, &(region_highlights[1].atr));
 		isearch_atr_on_set = 1;
+	    } else if (strpfx("fg_start_code:", *atrs)) {
+		set_colour_code(*atrs + 14, &fg_bg_sequences[COL_SEQ_FG].start);
+	    } else if (strpfx("fg_default_code:", *atrs)) {
+		set_colour_code(*atrs + 16, &fg_bg_sequences[COL_SEQ_FG].def);
+	    } else if (strpfx("fg_end_code:", *atrs)) {
+		set_colour_code(*atrs + 12, &fg_bg_sequences[COL_SEQ_FG].end);
+	    } else if (strpfx("bg_start_code:", *atrs)) {
+		set_colour_code(*atrs + 14, &fg_bg_sequences[COL_SEQ_BG].start);
+	    } else if (strpfx("bg_default_code:", *atrs)) {
+		set_colour_code(*atrs + 16, &fg_bg_sequences[COL_SEQ_BG].def);
+	    } else if (strpfx("bg_end_code:", *atrs)) {
+		set_colour_code(*atrs + 12, &fg_bg_sequences[COL_SEQ_BG].end);
 	    }
 	}
     }
@@ -453,9 +572,35 @@
 	region_highlights->atr = TXTSTANDOUT;
     if (!isearch_atr_on_set)
 	region_highlights[1].atr = TXTUNDERLINE;
+
+    /* Allocate buffer for colour code composition */
+    lenfg = strlen(fg_bg_sequences[COL_SEQ_FG].def);
+    /* always need 1 character for non-default code */
+    if (lenfg < 1)
+	lenfg = 1;
+    lenfg += strlen(fg_bg_sequences[COL_SEQ_FG].start) +
+	strlen(fg_bg_sequences[COL_SEQ_FG].end);
+
+    lenbg = strlen(fg_bg_sequences[COL_SEQ_BG].def);
+    /* always need 1 character for non-default code */
+    if (lenbg < 1)
+	lenbg = 1;
+    lenbg += strlen(fg_bg_sequences[COL_SEQ_BG].start) +
+	strlen(fg_bg_sequences[COL_SEQ_BG].end);
+
+    len = lenfg > lenbg ? lenfg : lenbg;
+    colseq_buf = (char *)zalloc(len+1);
 }
 
 
+/**/
+static void
+zle_free_highlight(void)
+{
+    /* Free buffer for colour code composition */
+    free(colseq_buf);
+}
+
 /*
  * Interface to the region_highlight ZLE parameter.
  * Converts betwen a format like "P32 42 underline,bold" to
@@ -942,26 +1087,12 @@
     rpms->sen = rpms->s + winw;
 }
 
-/*
- * HERE: these need to be made configurable, somehow.
- * Ideally we need to make the complist stuff use the
- * same system, but that may be too much tied to the GNU ls
- * interface to make that possible.
- */
-/* Start of escape sequence for foreground colour */
-#define TC_COL_FG_START	"\033[3"
-/* Start of escape sequence for background colour */
-#define TC_COL_BG_START	"\033[4"
-/* End of either escape sequence */
-#define TC_COL_END	"m"
-/* Numeric code (to be turned into ASCII) to reset default colour */
-#define TC_COL_DEFAULT	9
 
 static void
-setcolourattribute(int colour, char *start, int tc, int def,
+setcolourattribute(int colour, int fg_bg, int tc, int def,
 		   int use_termcap)
 {
-    char out[16], *ptr;
+    char *ptr;
     /*
      * If we're not restoring the default, and either have a
      * colour value that is too large for ANSI, or have been told
@@ -980,14 +1111,17 @@
 	    return;
     }
 
-    strcpy(out, start);
-    if (def)
-	colour = TC_COL_DEFAULT;
-
-    ptr = out + strlen(start);
-    *ptr++ = colour + '0';
-    strcpy(ptr, TC_COL_END);
-    tputs(out, 1, putshout);
+    strcpy(colseq_buf, fg_bg_sequences[fg_bg].start);
+
+    ptr = colseq_buf + strlen(colseq_buf);
+    if (def) {
+	strcpy(ptr, fg_bg_sequences[fg_bg].def);
+	while (*ptr)
+	    ptr++;
+    } else
+	*ptr++ = colour + '0';
+    strcpy(ptr, fg_bg_sequences[fg_bg].end);
+    tputs(colseq_buf, 1, putshout);
 }
 
 /**/
@@ -1008,13 +1142,13 @@
 	tsetcap(TCUNDERLINEBEG, 0);
     if (txtchangeisset(atr, TXTFGCOLOUR|TXTNOFGCOLOUR)) {
 	setcolourattribute(txtchangeget(atr, TXT_ATTR_FG_COL),
-			   TC_COL_FG_START, TCFGCOLOUR,
+			   COL_SEQ_FG, TCFGCOLOUR,
 			   txtchangeisset(atr, TXTNOFGCOLOUR),
 			   txtchangeisset(atr, TXT_ATTR_FG_TERMCAP));
     }
     if (txtchangeisset(atr, TXTBGCOLOUR|TXTNOBGCOLOUR)) {
 	setcolourattribute(txtchangeget(atr, TXT_ATTR_BG_COL),
-			   TC_COL_BG_START, TCBGCOLOUR,
+			   COL_SEQ_BG, TCBGCOLOUR,
 			   txtchangeisset(atr, TXTNOBGCOLOUR),
 			   txtchangeisset(atr, TXT_ATTR_BG_TERMCAP));
     }
@@ -1796,6 +1930,8 @@
     if (tmpalloced)
 	zfree(tmpline, tmpll * sizeof(*tmpline));
 
+    zle_free_highlight();
+
     /* if we have a new list showing, note it; if part of the list has been
     overwritten, redisplay it. We have to metafy line back before calling
     completion code */
@@ -2711,6 +2847,15 @@
     vcs = pos;
 }
 
+/* Provided for loading the module in a modular fashion */
+
+/**/
+void
+zle_refresh_boot(void)
+{
+    set_default_colour_sequences();
+}
+
 /* Provided for unloading the module in a modular fashion */
 
 /**/
@@ -2722,4 +2867,6 @@
     if (region_highlights)
 	zfree(region_highlights,
 	      sizeof(struct region_highlight) * n_region_highlights);
+
+    free_colour_sequences();
 }

Re: PATCH: isearch match highlighting

[Nikolai Weibull]

On Thu, May 1, 2008 at 12:37 PM, Peter Stephenson <pws@csr.com> wrote:
> On Tue, 29 Apr 2008 18:10:22 +0100

> Peter Stephenson <pws@csr.com> wrote:

> > I would like at the least to make the use of the ANSI escapes
>  > configurable.  They are in completion listings, but unfortunately that's
>  > tied to the variable interface for GNU ls colouring which doesn't really
>  > fit the case here.  I suppose special values in zle_highlight would be
>  > suitable (with bindkey escapes).

>  I've done this.  Looks suspiciously like overkill, but it's probably best
>  to be general.

Doesn't terminfo/termcap provide this kind of information?

Re: PATCH: isearch match highlighting

[Peter Stephenson]

On Fri, 2 May 2008 12:49:13 +0200
"Nikolai Weibull" <now@bitwi.se> wrote:
> On Thu, May 1, 2008 at 12:37 PM, Peter Stephenson <pws@csr.com> wrote:
> > On Tue, 29 Apr 2008 18:10:22 +0100
> 
> > Peter Stephenson <pws@csr.com> wrote:
> 
> > > I would like at the least to make the use of the ANSI escapes
> >  > configurable.  They are in completion listings, but unfortunately that's
> >  > tied to the variable interface for GNU ls colouring which doesn't really
> >  > fit the case here.  I suppose special values in zle_highlight would be
> >  > suitable (with bindkey escapes).
> 
> >  I've done this.  Looks suspiciously like overkill, but it's probably best
> >  to be general.
> 
> Doesn't terminfo/termcap provide this kind of information?

Not that I can see.  You can query the maximum number of colours, and there
are termlib and termcap sequences for setting a particular colour. I
haven't found a sequence for restoring the default, and no guarantee that
the termcap sequences, where supported, produce the same colours as the
ANSI ones in the same range, though it's probably a good bet.

pws