* bug#7485: 23.2; Fix removing unrecognized ANSI sequences in ansi-color-apply
@ 2010-11-26 14:48 Leo
2010-11-26 18:55 ` Stefan Monnier
0 siblings, 1 reply; 4+ messages in thread
From: Leo @ 2010-11-26 14:48 UTC (permalink / raw)
To: 7485
[-- Attachment #1: Type: text/plain, Size: 324 bytes --]
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.
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.
Leo
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix-ansi.diff --]
[-- Type: text/x-diff, Size: 1519 bytes --]
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 146c6c9..8d5cbe1 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2010-11-26 Leo <sdl.web@gmail.com>
+
+ * ansi-color.el (ansi-color-apply): Also eliminate unrecognized
+ ANSI sequences for remaining string.
+
2010-10-03 Chong Yidong <cyd@stupidchicken.com>
* minibuffer.el (completion--some, completion--do-completion)
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,12 +341,15 @@ See `ansi-color-unfontify-region' for a way around this."
(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 ((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 string pos))
- (push (substring string start pos) result))
- (push (substring string start) result))
+ (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)))
^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#7485: 23.2; Fix removing unrecognized ANSI sequences in ansi-color-apply
2010-11-26 14:48 bug#7485: 23.2; Fix removing unrecognized ANSI sequences in ansi-color-apply Leo
@ 2010-11-26 18:55 ` Stefan Monnier
2010-11-26 22:41 ` Leo
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Monnier @ 2010-11-26 18:55 UTC (permalink / raw)
To: Leo; +Cc: bug-gnu-emacs
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#7485: 23.2; Fix removing unrecognized ANSI sequences in ansi-color-apply
2010-11-26 18:55 ` Stefan Monnier
@ 2010-11-26 22:41 ` Leo
2017-07-04 1:48 ` npostavs
0 siblings, 1 reply; 4+ messages in thread
From: Leo @ 2010-11-26 22:41 UTC (permalink / raw)
To: Stefan Monnier; +Cc: bug-gnu-emacs
On 2010-11-26 18:55 +0000, Stefan Monnier wrote:
>> 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?
Yes.
>> 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.
which means I only briefly tested the patch and it seems not to make
things worse while fixing a bug that annoys me ;)
>> 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
Unrecognized control sequences also begin with char \033. The FRAGMENT
part of ansi-color-context wasn't supposed to contain actual text,
otherwise they might get lost. It should always be a fragment of a whole
ansi control sequence if non-nil.
Let me show how this stops eshell prompt from showing.
Assume ansi-color-apply is in eshell-preoutput-filter-functions. Suppose
the output of shell command 'ack linux' ends with an unrecognized
control sequence ^[[K. The whole output string that goes through
ansi-color-apply is
output of 'ack linux' + 'eshell prompt'.
After 'ack linux' finishes, the FRAGMENT of ansi-color-context becomes:
#("^[[K\n~/.emacs.d $ " 4 17
(read-only t face eshell-prompt rear-nonsticky
(face read-only)))
The string after being processed by ansi-color-apply is thus without an
eshell prompt.
Leo
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#7485: 23.2; Fix removing unrecognized ANSI sequences in ansi-color-apply
2010-11-26 22:41 ` Leo
@ 2017-07-04 1:48 ` npostavs
0 siblings, 0 replies; 4+ messages in thread
From: npostavs @ 2017-07-04 1:48 UTC (permalink / raw)
To: Leo; +Cc: 7485, Stefan Monnier
close 7485
quit
Leo <sdl.web@gmail.com> writes:
> Unrecognized control sequences also begin with char \033. The FRAGMENT
> part of ansi-color-context wasn't supposed to contain actual text,
> otherwise they might get lost. It should always be a fragment of a whole
> ansi control sequence if non-nil.
>
> Let me show how this stops eshell prompt from showing.
>
> Assume ansi-color-apply is in eshell-preoutput-filter-functions. Suppose
> the output of shell command 'ack linux' ends with an unrecognized
> control sequence ^[[K. The whole output string that goes through
> ansi-color-apply is
>
> output of 'ack linux' + 'eshell prompt'.
>
> After 'ack linux' finishes, the FRAGMENT of ansi-color-context becomes:
>
> #("^[[K\n~/.emacs.d $ " 4 17
> (read-only t face eshell-prompt rear-nonsticky
> (face read-only)))
>
> The string after being processed by ansi-color-apply is thus without an
> eshell prompt.
I think this patch is no longer applicable now that I've pushed
[1:35ed01dfb3], ansi-color-apply now skips over every escape sequence,
not just color escape sequences. Although I'm unable to produce the
problem of missing prompts even before my patch (e.g., in 25.2):
~/src $ printf 'ack linux\e[K\n'
ack linux
~/src $
[1: 35ed01dfb3]: 2017-07-03 10:09:40 -0400
Fix and simplify ansi escape detection (Bug#21381)
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=35ed01dfb3f811a997e26d843e9971eb6b81b125
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-07-04 1:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-26 14:48 bug#7485: 23.2; Fix removing unrecognized ANSI sequences in ansi-color-apply Leo
2010-11-26 18:55 ` Stefan Monnier
2010-11-26 22:41 ` Leo
2017-07-04 1:48 ` npostavs
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).