Subject: Re: bug#78184: 30.0.93; ansi-osc-apply-on-region should use end marker

From: Matthias Meulien <[email-masked]>
Date: Sat, 24 May 2025 17:25:22 +0200

Here is an updated patch that extend `ansi-osc-filter-region' to properly
handle unfinished escape sequences. Tests have been added to cover multiple
successive calls to `ansi-osc-filter-region' and `ansi-osc-apply-region'.

Le sam. 24 mai 2025 à 11:03, Eli Zaretskii <[email-masked]> a écrit :

> > From: Troy Brown <[email-masked]>
> > Date: Mon, 12 May 2025 18:32:13 -0400
> > Cc: Matthias Meulien <[email-masked]>, [email-masked]
> >
> > On Sat, May 10, 2025 at 6:52 AM Eli Zaretskii <[email-masked]> wrote:
> > >
> > > > From: Matthias Meulien <[email-masked]>
> > > > Date: Thu, 8 May 2025 17:12:50 +0200
> > > > Cc: Troy Brown <[email-masked]>, [email-masked]
> > > >
> > > > Troy analysis is right.  I was able to reproduce.  Please have a
> look to the attached patch.
> > >
> > > Thanks.  Troy, could you please try the patch and report?
> >
> > Yes, I can confirm that this patch addresses the issue I'm seeing.  It
> > appears the patch was for the master branch, so that is where I
> > applied and tested it.  I assume this change would also be merged into
> > the Emacs 30 branch.
> >
> > I did have a couple notes.  I believe an end marker should also be
> > used in `ansi-osc-filter-region` to honor the original bounds passed
> > into the function, since it also calls `delete-region`.  I'm not aware
> > of it raising an error due to its absence, but I believe it would be
> > correct to use it, and would also match `ansi-color-filter-region`.
> > Also, it seems `ansi-osc-filter-region` doesn't handle "fragments"
> > (where only the beginning of the escape sequence appears in the
> > supplied range), by saving and reusing context, whereas the
> > corresponding `ansi-color-filter-region` does.  Therefore if an OSC
> > control sequence overlapped the supplied bounds, I don't think it
> > would be filtered out on the next call to `ansi-osc-filter-region`.
>
> Thanks.  Matthias, any further comments or changes to the patch?
>

-- 
Matthias
From e0f8e77433b8b83646f31e65961487bacec72897 Mon Sep 17 00:00:00 2001
From: Matthias Meulien <[email-masked]>
Date: Thu, 8 May 2025 16:51:46 +0200
Subject: [PATCH] ansi-osc.el: Use marker (bug#78184)

* lisp/ansi-osc.el (ansi-osc-apply-on-region)
(ansi-osc-filter-region): Use marker to properly handle
unfinished escape sequence.

* test/lisp/ansi-osc-tests.el (ansi-osc-tests--strings)
(ansi-osc-tests-apply-region-no-handlers)
(ansi-osc-tests-apply-region-no-handlers-multiple-calls)
(ansi-osc-tests-filter-region)
(ansi-osc-tests-filter-region-with-multiple-calls): Cover
bug#78184.
---
 lisp/ansi-osc.el            | 88 +++++++++++++++++++++----------------
 test/lisp/ansi-osc-tests.el | 49 +++++++++++++++++++--
 2 files changed, 97 insertions(+), 40 deletions(-)

diff --git a/lisp/ansi-osc.el b/lisp/ansi-osc.el
index 97d6f6c8754..06359779823 100644
--- a/lisp/ansi-osc.el
+++ b/lisp/ansi-osc.el
@@ -35,18 +35,32 @@
 
 ;;; Code:
 
-(defconst ansi-osc-control-seq-regexp
-  ;; See ECMA 48, section 8.3.89 "OSC - OPERATING SYSTEM COMMAND".
-  "\e\\][\x08-\x0D]*[\x20-\x7E]*\\(\a\\|\e\\\\\\)"
-  "Regexp matching an OSC control sequence.")
+;; According to ECMA 48, section 8.3.89 "OSC - OPERATING SYSTEM COMMAND"
+;; OSC control sequences match:
+;; "\e\\][\x08-\x0D]*[\x20-\x7E]*\\(\a\\|\e\\\\\\)"
+
+(defvar-local ansi-osc--marker nil
+  "Marker pointing to the start of an escape sequence.
+Used by `ansi-osc-filter-region' and `ansi-osc-apply-on-region' to store
+position of an unfinished escape sequence, for the complete sequence to
+be handled in next call.")
 
 (defun ansi-osc-filter-region (begin end)
-  "Filter out all OSC control sequences from region between BEGIN and END."
-  (save-excursion
-    (goto-char begin)
-    ;; Delete escape sequences.
-    (while (re-search-forward ansi-osc-control-seq-regexp end t)
-      (delete-region (match-beginning 0) (match-end 0)))))
+  "Filter out all OSC control sequences from region between BEGIN and END.
+When an unfinished escape sequence is found, the start position is saved
+to `ansi-osc--marker'.  Later call will override BEGIN with the position
+pointed by `ansi-osc--marker'."
+  (let ((end-marker (copy-marker end)))
+    (save-excursion
+      (goto-char (or ansi-osc--marker begin))
+      (when (eq (char-before) ?\e) (backward-char))
+      (while (re-search-forward "\e]" end-marker t)
+        (let ((pos0 (match-beginning 0)))
+          (if (re-search-forward
+               "\\=[\x08-\x0D]*[\x20-\x7E]*\\(\a\\|\e\\\\\\)"
+               end-marker t)
+              (delete-region pos0 (point))
+            (setq ansi-osc--marker (copy-marker pos0))))))))
 
 (defvar-local ansi-osc-handlers '(("2" . ansi-osc-window-title-handler)
                                   ("7" . ansi-osc-directory-tracker)
@@ -54,10 +68,6 @@ ansi-osc-handlers
   "Alist of handlers for OSC escape sequences.
 See `ansi-osc-apply-on-region' for details.")
 
-(defvar-local ansi-osc--marker nil)
-;; The function `ansi-osc-apply-on-region' can set `ansi-osc--marker'
-;; to the start position of an escape sequence without termination.
-
 (defun ansi-osc-apply-on-region (begin end)
   "Interpret OSC escape sequences in region between BEGIN and END.
 This function searches for escape sequences of the forms
@@ -65,29 +75,33 @@ ansi-osc-apply-on-region
     ESC ] command ; text BEL
     ESC ] command ; text ESC \\
 
-Every occurrence of such escape sequences is removed from the
-buffer.  Then, if `command' is a key in the alist that is the
-value of the local variable `ansi-osc-handlers', that key's
-value, which should be a function, is called with `command' and
-`text' as arguments, with point where the escape sequence was
-located."
-  (save-excursion
-    (goto-char (or ansi-osc--marker begin))
-    (when (eq (char-before) ?\e) (backward-char))
-    (while (re-search-forward "\e]" end t)
-      (let ((pos0 (match-beginning 0))
-            (code (and (re-search-forward "\\=\\([0-9A-Za-z]*\\);" end t)
-                       (match-string 1)))
-            (pos1 (point)))
-        (if (re-search-forward "\a\\|\e\\\\" end t)
-            (let ((text (buffer-substring-no-properties
-                         pos1 (match-beginning 0))))
-              (setq ansi-osc--marker nil)
-              (delete-region pos0 (point))
-              (when-let* ((fun (cdr (assoc-string code ansi-osc-handlers))))
-                (funcall fun code text)))
-          (put-text-property pos0 end 'invisible t)
-          (setq ansi-osc--marker (copy-marker pos0)))))))
+Every occurrence of such escape sequences is removed from the buffer.
+Then, if `command' is a key in the alist that is the value of the local
+variable `ansi-osc-handlers', that key's value, which should be a
+function, is called with `command' and `text' as arguments, with point
+where the escape sequence was located.  When an unfinished escape
+sequence is identified, it's hidden and the start position is saved to
+`ansi-osc--marker'.  Later call will override BEGIN with the position
+pointed by `ansi-osc--marker'."
+  (let ((end-marker (copy-marker end)))
+    (save-excursion
+      (goto-char (or ansi-osc--marker begin))
+      (when (eq (char-before) ?\e) (backward-char))
+      (while (re-search-forward "\e]" end-marker t)
+        (let ((pos0 (match-beginning 0))
+              (code (and
+                     (re-search-forward "\\=\\([0-9A-Za-z]*\\);" end-marker t)
+                     (match-string 1)))
+              (pos1 (point)))
+          (if (re-search-forward "\a\\|\e\\\\" end-marker t)
+              (let ((text (buffer-substring-no-properties
+                           pos1 (match-beginning 0))))
+                (setq ansi-osc--marker nil)
+                (delete-region pos0 (point))
+                (when-let* ((fun (cdr (assoc-string code ansi-osc-handlers))))
+                  (funcall fun code text)))
+            (put-text-property pos0 end-marker 'invisible t)
+            (setq ansi-osc--marker (copy-marker pos0))))))))
 
 ;; Window title handling (OSC 2)
 
diff --git a/test/lisp/ansi-osc-tests.el b/test/lisp/ansi-osc-tests.el
index d2fb130e518..d083626f3f9 100644
--- a/test/lisp/ansi-osc-tests.el
+++ b/test/lisp/ansi-osc-tests.el
@@ -30,8 +30,7 @@
 (require 'ert)
 
 (defvar ansi-osc-tests--strings
-  `(
-    ("Hello World" "Hello World")
+  `(("Hello World" "Hello World")
 
     ;; window title
     ("Buffer \e]2;A window title\e\\content" "Buffer content")
@@ -44,6 +43,10 @@ ansi-osc-tests--strings
 
     ;; hyperlink
     ("\e]8;;http://example.com\e\\This is a link\e]8;;\e\\" "This is a link")
+
+    ;; multiple sequences
+    ("Escape \e]2;A window title\e\\sequence followed by \e]2;unfinished sequence"
+     "Escape sequence followed by \e]2;unfinished sequence")
     ))
 ;; Don't output those strings to stdout since they may have
 ;; side-effects on the environment
@@ -54,4 +57,44 @@ ansi-osc-tests-apply-region-no-handlers
       (with-temp-buffer
         (insert input)
         (ansi-osc-apply-on-region (point-min) (point-max))
-        (should (equal (buffer-string) text))))))
+        (should (equal
+                 (buffer-substring-no-properties
+                  (point-min) (point-max))
+                 text))))))
+
+(ert-deftest ansi-osc-tests-apply-region-no-handlers-multiple-calls ()
+  (let ((ansi-osc-handlers nil))
+    (with-temp-buffer
+      (insert
+       (concat "First set the window title \e]2;A window title\e\\"
+               "then change it\e]2;Another "))
+      (ansi-osc-apply-on-region (point-min) (point-max))
+      (let ((pos (point)))
+        (insert "title\e\\, and stop.")
+        (ansi-osc-apply-on-region pos (point-max)))
+      (should
+       (equal
+        (buffer-substring-no-properties (point-min) (point-max))
+        "First set the window title then change it, and stop.")))))
+
+(ert-deftest ansi-osc-tests-filter-region ()
+  (pcase-dolist (`(,input ,text) ansi-osc-tests--strings)
+    (with-temp-buffer
+      (insert input)
+      (ansi-osc-filter-region (point-min) (point-max))
+      (should (equal (buffer-string) text)))))
+
+
+(ert-deftest ansi-osc-tests-filter-region-with-multiple-calls ()
+  (with-temp-buffer
+    (insert
+     (concat "First set the window title \e]2;A window title\e\\"
+             "then change it\e]2;Another "))
+    (ansi-osc-filter-region (point-min) (point-max))
+    (let ((pos (point)))
+      (insert "title\e\\, and stop.")
+      (ansi-osc-filter-region pos (point-max)))
+    (should
+     (equal
+      (buffer-string)
+      "First set the window title then change it, and stop."))))
-- 
2.39.5