bug#24223: 25.1; Error in ansi-color-filter-apply when control sequences are dropped

bug#24223: 25.1; Error in ansi-color-filter-apply when control sequences are dropped

[Ivan Andrus]

In `ansi-color-filter-apply' the block "eliminate unrecognized escape
sequences" occurs after the "find the next escape sequence" block.  This
is problematic because the variable start is set in the "next sequence"
block and used in the "save context, add the remainder of the string to
the result" block.  However, dropping control sequences with
`ansi-color-drop-regexp' can cause the string to shorten and start to
point past the end of the string.

I merely swapped the order of the first two blocks mentioned and was
able to get past the error.  But there are likely other places that need
to be fixed as well.

I have commit access, and can make the change (as well as tracking down
other places where it might be necessary), but I would like some
feedback on whether this is the right way solve it.  Presumably there
was a reason escape sequences were dropped after start was set up.

-Ivan

diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 788a7bd..7cea2d9 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -258,14 +258,14 @@ ansi-color-filter-apply
     (if (cadr ansi-color-context)
         (setq string (concat (cadr ansi-color-context) string)
               ansi-color-context nil))
-    ;; find the next escape sequence
-    (while (setq end (string-match ansi-color-regexp string start))
-      (setq result (concat result (substring string start end))
-            start (match-end 0)))
     ;; eliminate unrecognized escape sequences
     (while (string-match ansi-color-drop-regexp string)
       (setq string
             (replace-match "" nil nil string)))
+    ;; find the next escape sequence
+    (while (setq end (string-match ansi-color-regexp string start))
+      (setq result (concat result (substring string start end))
+            start (match-end 0)))
     ;; save context, add the remainder of the string to the result
     (let (fragment)
       (if (string-match "\033" string start)




In GNU Emacs 25.1.2 (x86_64-apple-darwin15.5.0, NS appkit-1404.47 Version 10.11.6 (Build 15G31))
 of 2016-08-04 built on iandrus-osx
Repository revision: 784291b06c4e4fe52f4c1aa15bc99874abc517c4
Windowing system distributor 'Apple', version 10.3.1404
Configured using:
 'configure --with-ns --with-modules
 PKG_CONFIG_PATH=/opt/X11/lib/pkgconfig'

Configured features:
JPEG RSVG IMAGEMAGICK DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS NS MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

bug#24223: 25.1; Error in ansi-color-filter-apply when control sequences are dropped

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

Ivan Andrus <darthandrus@gmail.com> writes:

> I merely swapped the order of the first two blocks mentioned and was
> able to get past the error.  But there are likely other places that need
> to be fixed as well.
>
> I have commit access, and can make the change (as well as tracking down
> other places where it might be necessary), but I would like some
> feedback on whether this is the right way solve it.  Presumably there
> was a reason escape sequences were dropped after start was set up.

I think this method of using 2 different regexps to match the escape
sequences is an over complication, I propose to simplify:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 8692 bytes --]

From 91b386f8da95c0e69095ea8d64eeb1af19daf3cc Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Thu, 15 Jun 2017 23:23:44 -0400
Subject: [PATCH v1] Fix and simplify ansi escape detection (Bug#21381)

* lisp/ansi-color.el (ansi-color-regexp, ansi-color-drop-regexp):
Remove.
(ansi-color-control-seq-regexp): New constant, matches all escape
sequences.
(ansi-color-filter-apply, ansi-color-apply)
(ansi-color-filter-region, ansi-color-apply-on-region): Use it instead
of matching color sequences separately from ignored sequences.
Differentiate color sequences simply by checking the last character.
---
 lisp/ansi-color.el | 115 ++++++++++++++++++++++-------------------------------
 1 file changed, 48 insertions(+), 67 deletions(-)

diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 47437bb7c8..a1b4933175 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -150,17 +150,14 @@ (defcustom ansi-color-names-vector
   :version "24.4" ; default colors copied from `xterm-standard-colors'
   :group 'ansi-colors)
 
-(defconst ansi-color-regexp "\033\\[\\([0-9;]*m\\)"
-  "Regexp that matches SGR control sequences.")
-
-(defconst ansi-color-drop-regexp
-  "\033\\[\\([ABCDsuK]\\|[12][JK]\\|=[0-9]+[hI]\\|[0-9;]*[Hf]\\|\\?[0-9]+[hl]\\)"
-  "Regexp that matches ANSI control sequences to silently drop.")
+(defconst ansi-color-control-seq-regexp
+  ;; See ECMA 48, section 5.4 "Control Sequences".
+  "\e\\[[\x30-\x3F]*[\x20-\x2F]*[\x40-\x7E]"
+  "Regexp matching an ANSI control sequence.")
 
 (defconst ansi-color-parameter-regexp "\\([0-9]*\\)[m;]"
   "Regexp that matches SGR control sequence parameters.")
 
-
 ;; Convenience functions for comint modes (eg. shell-mode)
 
 
@@ -259,22 +256,20 @@ (defun ansi-color-filter-apply (string)
         (setq string (concat (cadr ansi-color-context) string)
               ansi-color-context nil))
     ;; find the next escape sequence
-    (while (setq end (string-match ansi-color-regexp string start))
-      (setq result (concat result (substring string start end))
-            start (match-end 0)))
-    ;; eliminate unrecognized escape sequences
-    (while (string-match ansi-color-drop-regexp string)
-      (setq string
-            (replace-match "" nil nil string)))
+    (while (setq end (string-match ansi-color-control-seq-regexp string start))
+      (push (substring string start end) result)
+      (setq start (match-end 0)))
     ;; 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)
-		  result (concat result (substring string start pos))))
-	(setq result (concat result (substring string start))))
+      (push (substring string start
+                       (if (string-match "\033" string start)
+                           (let ((pos (match-beginning 0)))
+                             (setq fragment (substring string pos))
+                             pos)
+                         nil))
+            result)
       (setq ansi-color-context (if fragment (list nil fragment))))
-    result))
+    (apply #'concat (nreverse result))))
 
 (defun ansi-color--find-face (codes)
   "Return the face corresponding to CODES."
@@ -306,35 +301,29 @@ (defun ansi-color-apply (string)
 
 This function can be added to `comint-preoutput-filter-functions'."
   (let ((codes (car ansi-color-context))
-	(start 0) end escape-sequence result
-	colorized-substring)
+	(start 0) end result)
     ;; If context was saved and is a string, prepend it.
     (if (cadr ansi-color-context)
         (setq string (concat (cadr ansi-color-context) string)
               ansi-color-context nil))
     ;; Find the next escape sequence.
-    (while (setq end (string-match ansi-color-regexp string start))
-      (setq escape-sequence (match-string 1 string))
-      ;; Colorize the old block from start to end using old face.
-      (when codes
-	(put-text-property start end 'font-lock-face (ansi-color--find-face codes) string))
-      (setq colorized-substring (substring string start end)
-	    start (match-end 0))
-      ;; Eliminate unrecognized ANSI sequences.
-      (while (string-match ansi-color-drop-regexp colorized-substring)
-	(setq colorized-substring
-	      (replace-match "" nil nil colorized-substring)))
-      (push colorized-substring result)
-      ;; Create new face, by applying escape sequence parameters.
-      (setq codes (ansi-color-apply-sequence escape-sequence codes)))
+    (while (setq end (string-match ansi-color-control-seq-regexp string start))
+      (let ((esc-end (match-end 0)))
+        ;; Colorize the old block from start to end using old face.
+        (when codes
+          (put-text-property start end 'font-lock-face
+                             (ansi-color--find-face codes) string))
+        (push (substring string start end) result)
+        (setq start (match-end 0))
+        ;; If this is a color escape sequence,
+        (when (eq (aref string (1- esc-end)) ?m)
+          ;; create a new face from it.
+          (setq codes (ansi-color-apply-sequence
+                       (substring string end esc-end) codes)))))
     ;; if the rest of the string should have a face, put it there
     (when codes
       (put-text-property start (length string)
                          'font-lock-face (ansi-color--find-face codes) string))
-    ;; eliminate unrecognized escape sequences
-    (while (string-match ansi-color-drop-regexp string)
-      (setq string
-            (replace-match "" nil nil string)))
     ;; save context, add the remainder of the string to the result
     (let (fragment)
       (if (string-match "\033" string start)
@@ -367,13 +356,9 @@ (defun ansi-color-filter-region (begin end)
 	(start (or (cadr ansi-color-context-region) begin)))
     (save-excursion
       (goto-char start)
-      ;; Delete unrecognized escape sequences.
-      (while (re-search-forward ansi-color-drop-regexp end-marker t)
-        (replace-match ""))
-      (goto-char start)
-      ;; Delete SGR escape sequences.
-      (while (re-search-forward ansi-color-regexp end-marker t)
-        (replace-match ""))
+      ;; Delete escape sequences.
+      (while (re-search-forward ansi-color-control-seq-regexp end-marker t)
+        (delete-region (match-beginning 0) (match-end 0)))
       ;; save context, add the remainder of the string to the result
       (if (re-search-forward "\033" end-marker t)
 	  (setq ansi-color-context-region (list nil (match-beginning 0)))
@@ -400,28 +385,24 @@ (defun ansi-color-apply-on-region (begin end)
   (let ((codes (car ansi-color-context-region))
 	(start-marker (or (cadr ansi-color-context-region)
 			  (copy-marker begin)))
-	(end-marker (copy-marker end))
-	escape-sequence)
-    ;; First, eliminate unrecognized ANSI control sequences.
-    (save-excursion
-      (goto-char start-marker)
-      (while (re-search-forward ansi-color-drop-regexp end-marker t)
-	(replace-match "")))
+	(end-marker (copy-marker end)))
     (save-excursion
       (goto-char start-marker)
-      ;; Find the next SGR sequence.
-      (while (re-search-forward ansi-color-regexp end-marker t)
-	;; Colorize the old block from start to end using old face.
-	(funcall ansi-color-apply-face-function
-		 start-marker (match-beginning 0)
-		 (ansi-color--find-face codes))
-        ;; store escape sequence and new start position
-        (setq escape-sequence (match-string 1)
-	      start-marker (copy-marker (match-end 0)))
-	;; delete the escape sequence
-	(replace-match "")
-	;; Update the list of ansi codes.
-	(setq codes (ansi-color-apply-sequence escape-sequence codes)))
+      ;; Find the next escape sequence.
+      (while (re-search-forward ansi-color-control-seq-regexp end-marker t)
+        ;; Remove escape sequence.
+        (let ((esc-seq (delete-and-extract-region
+                        (match-beginning 0) (point))))
+          ;; Colorize the old block from start to end using old face.
+          (funcall ansi-color-apply-face-function
+                   (prog1 (marker-position start-marker)
+                     ;; Store new start position.
+                     (set-marker start-marker (point)))
+                   (match-beginning 0) (ansi-color--find-face codes))
+          ;; If this is a color sequence,
+          (when (eq (aref esc-seq (1- (length esc-seq))) ?m)
+            ;; update the list of ansi codes.
+            (setq codes (ansi-color-apply-sequence esc-seq codes)))))
       ;; search for the possible start of a new escape sequence
       (if (re-search-forward "\033" end-marker t)
 	  (progn
-- 
2.11.1

bug#24223: 25.1; Error in ansi-color-filter-apply when control sequences are dropped

[npostavs]

tags 24223 fixed
close 24223 26.1
quit

npostavs@users.sourceforge.net writes:

> I think this method of using 2 different regexps to match the escape
> sequences is an over complication, I propose to simplify:

Pushed to master.

[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