Subject: Re: 23.2; Fix removing unrecognized ANSI sequences in ansi-color-apply

From: Stefan Monnier <[email-masked]>
Date: Fri, 26 Nov 2010 13:55:16 -0500

> In ansi-color-apply, (string-match "\033" string start) finds the wrong
> portion of context if unrecognized ANSI sequences is not removed before
> the match.

You mean, because \033 can appear in that unrecognized ANSI sequence?

> This can cause, for example, eshell's prompt to disappear if
> ansi-color-apply is used in eshell-preoutput-filter-functions.
> The attached patch tries to fix this.

I don't quite understand your patch.  And your saying "tries to fix"
doesn't make me more confident.

> diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
> index 00162c9..40c0066 100644
> *** a/lisp/ansi-color.el
> --- b/lisp/ansi-color.el
> ***************
> *** 341,352 ****
>         (put-text-property start (length string) 'ansi-color t string)
>         (put-text-property start (length string) 'face face string))
>       ;; save context, add the remainder of the string to the result
> !     (let (fragment)
> !       (if (string-match "\033" string start)
>   	  (let ((pos (match-beginning 0)))
> ! 	    (setq fragment (substring string pos))
> ! 	    (push (substring string start pos) result))
> ! 	(push (substring string start) result))
>         (if (or face fragment)
>   	  (setq ansi-color-context (list face fragment))
>   	(setq ansi-color-context nil)))
> --- 341,355 ----
>         (put-text-property start (length string) 'ansi-color t string)
>         (put-text-property start (length string) 'face face string))
>       ;; save context, add the remainder of the string to the result
> !     (let ((remaining (substring string start))
> ! 	  fragment)
> !       (while (string-match ansi-color-drop-regexp remaining)
> ! 	(setq remaining (replace-match "" nil nil remaining)))
> !       (if (string-match "\033" remaining)
>   	  (let ((pos (match-beginning 0)))
> ! 	    (setq fragment (substring remaining pos))
> ! 	    (push (substring remaining 0 pos) result))
> ! 	(push remaining result))
>         (if (or face fragment)
>   	  (setq ansi-color-context (list face fragment))
>   	(setq ansi-color-context nil)))

This appears to "drop control sequences" even tho they weren't dropped
before, so it does more than "ignore false-positive \033 from
unrecognized control sequences".  IIUC those unrecognized control
sequences are dropped elsewhere, right?  So are maybe suggesting that
they are currently not dropped at the right time (i.e. dropped too
late), or that the \033 handling takes place too early, or that we
currently forget to drop those control sequences or ... ?

        Stefan