Subject: [PATCH 1/2] ansi-term: ignore CSI commands with subparams

From: Johannes Altmanninger <[email-masked]>
Date: Thu, 17 Apr 2025 20:46:32 +0200

* lisp/term.el (term-handle-ansi-escape): Check if parameters contain
subparameters (values separated by colon) and ignore those commands.
This prevents incorrect interpretation of advanced terminal features
that aren't yet properly supported, such as curly underlines (\e[4:3m).

Consider ignoring whitespace changes when reviewing this patch.
The essence of this change is the addition at the beginning of
term-handle-ansi-escape, to skip all CSI commands that contain at
least two subparameters (e.g. the ones that contain at least one ":").

Some background / longer description:

If I run

	emacs --eval '(ansi-term "/bin/bash")'

and type

	printf '\e[4:3mHELLO\e[m\n'

then I get text with a straight underline.

This is not what I expect, because "\e[4:3m" is supposed to turn on
curly/wavy underlines, see https://sw.kovidgoyal.net/kitty/underlines/.

My program wants to use curly underlines to indicate errors. Rendering
those as straight underlines is noisy and confusing because it suggests
a different meaning (in my program).

I'd rather ansi-term ignore the curly underline command until it's
implemented.

FWIW Emacs itself already supports various underline styles, even in
a TTY -- see e.g. 9f589eb9240 (Add support for colored and styled
underlines on tty frames, 2023-04-20). It would be fairly easy to
add support for curly underlines to ansi-term.

In general, I wonder what's story on Emacs terminals. ansi-term
has some other compatibility issues, for example it cannot parse
OSC (Operating System Command) sequences, which causes friction
with the fish shell. We can fix this fairly easily; alternatively
maybe default to another terminal implementation such as vterm
(https://github.com/akermu/emacs-libvterm) that offers better
xterm-compatibility.
---
 lisp/term.el | 230 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 127 insertions(+), 103 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index a971300c055..8270a580435 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -3224,11 +3224,15 @@ term-emulate-terminal
                                 (if private (substring ctl-params 1) ctl-params)))
                           (term-handle-ansi-escape
                            proc
-                           (mapcar ;; We don't distinguish empty params
-                            ;; from 0 (according to [ECMA-48] we
-                            ;; should, but all commands we support
-                            ;; default to 0 values anyway).
-                            #'string-to-number
+                           (mapcar
+                             (lambda (param)
+                               (mapcar
+                                 ;; We don't distinguish empty params
+                                 ;; from 0 (according to [ECMA-48] we
+                                 ;; should, but all commands we support
+                                 ;; default to 0 values anyway).
+                                 #'string-to-number
+                                 (split-string param ":")))
                             (split-string ctl-params ";"))
                            (aref str (1- ctl-end))
                            private))))
@@ -3589,117 +3593,137 @@ term--handle-colors-list
 ;; i.e. we have previously seen Escape followed by ?[.
 
 (defun term-handle-ansi-escape (proc params char &optional private)
-  (cond
-   ((and private (not (memq char '(?h ?l))))
-    ;; Recognize private capabilities only for mode entry and exit
-    nil)
-   ((or (eq char ?H)  ;; cursor motion (terminfo: cup,home)
+  (let
+    ((supported-params
+      (cl-remove-if
+       (lambda (subparams) (> (length subparams) 1))
+       params)
+      ))
+    ;; We'll pass on only parameters without colon, e.g. parameters with at
+    ;; most one sub-parameter.
+    ;; However if all parameters were unsupported, make sure to ignore the
+    ;; entire sequence.
+    (when
+      (not
+       (and
+         (null supported-params)
+         (not (null params))))
+      (let
+	((params
+	  (mapcar
+           (lambda (subparams) (if (null subparams) 0 (car subparams)))
+	   supported-params)))
+	(cond
+	((and private (not (memq char '(?h ?l))))
+	;; Recognize private capabilities only for mode entry and exit
+	nil)
+	((or (eq char ?H)  ;; cursor motion (terminfo: cup,home)
 	;; (eq char ?f) ;; xterm seems to handle this sequence too, not
 	;; needed for now
 	)
-    (term-goto
-     (1- (max 1 (min (or (nth 0 params) 0) term-height)))
-     (1- (max 1 (min (or (nth 1 params) 0) term-width)))))
-   ;; \E[A - cursor up (terminfo: cuu, cuu1)
-   ((eq char ?A)
-    (term-handle-deferred-scroll)
-    (let ((tcr (term-current-row))
-          (scroll-amount (car params)))
-      (term-down
-       (if (< (- tcr scroll-amount) term-scroll-start)
+	(term-goto
+	(1- (max 1 (min (or (nth 0 params) 0) term-height)))
+	(1- (max 1 (min (or (nth 1 params) 0) term-width)))))
+	;; \E[A - cursor up (terminfo: cuu, cuu1)
+	((eq char ?A)
+	(term-handle-deferred-scroll)
+	(let ((tcr (term-current-row))
+	     (scroll-amount (car params)))
+	 (term-down
+	  (if (< (- tcr scroll-amount) term-scroll-start)
 	   ;; If the amount to move is before scroll start, move
 	   ;; to scroll start.
 	   (- term-scroll-start tcr)
-         (if (>= scroll-amount tcr)
+	    (if (>= scroll-amount tcr)
 	     (- tcr)
-           (- (max 1 scroll-amount))))
-       t)))
-   ;; \E[B - cursor down (terminfo: cud)
-   ((eq char ?B)
-    (let ((tcr (term-current-row))
-          (scroll-amount (car params)))
-      (unless (>= tcr term-scroll-end)
+	      (- (max 1 scroll-amount))))
+	  t)))
+	;; \E[B - cursor down (terminfo: cud)
+	((eq char ?B)
+	(let ((tcr (term-current-row))
+	     (scroll-amount (car params)))
+	 (unless (>= tcr term-scroll-end)
 	(term-down
-         (min (- term-scroll-end tcr) (max 1 scroll-amount))
-         t))))
-   ;; \E[C - cursor right (terminfo: cuf, cuf1)
-   ((eq char ?C)
-    (term-move-columns
-     (max 1
-          (if (>= (+ (car params) (term-current-column)) term-width)
+	    (min (- term-scroll-end tcr) (max 1 scroll-amount))
+	    t))))
+	;; \E[C - cursor right (terminfo: cuf, cuf1)
+	((eq char ?C)
+	(term-move-columns
+	(max 1
+	     (if (>= (+ (car params) (term-current-column)) term-width)
 	      (- term-width (term-current-column)  1)
-            (car params)))))
-   ;; \E[D - cursor left (terminfo: cub)
-   ((eq char ?D)
-    (term-move-columns (- (max 1 (car params)))))
-   ;; \E[G - cursor motion to absolute column (terminfo: hpa)
-   ((eq char ?G)
-    (term-move-columns (- (max 0 (min term-width (car params)))
-                          (term-current-column))))
-   ;; \E[J - clear to end of screen (terminfo: ed, clear)
-   ((eq char ?J)
-    (term-erase-in-display (car params)))
-   ;; \E[K - clear to end of line (terminfo: el, el1)
-   ((eq char ?K)
-    (term-erase-in-line (car params)))
-   ;; \E[L - insert lines (terminfo: il, il1)
-   ((eq char ?L)
-    (term-insert-lines (max 1 (car params))))
-   ;; \E[M - delete lines (terminfo: dl, dl1)
-   ((eq char ?M)
-    (term-delete-lines (max 1 (car params))))
-   ;; \E[P - delete chars (terminfo: dch, dch1)
-   ((eq char ?P)
-    (term-delete-chars (max 1 (car params))))
-   ;; \E[@ - insert spaces (terminfo: ich)
-   ((eq char ?@)
-    (term-insert-spaces (max 1 (car params))))
-   ;; \E[?h - DEC Private Mode Set
-
-   ;; N.B. we previously had a bug in which we'd decode \e[?<NR>h or
-   ;; \e[?<NR>l as a command with zero in the params field and so
-   ;; didn't recognize DEC private escape sequences.  However, the
-   ;; termcap and terminfo files had the non-? (question mark means DEC
-   ;; private) versions, so things kind of worked anyway.  To preserve
-   ;; compatibility, we recognize both private- and non-private
-   ;; messages for capabilities we added before we fixed the bug but
-   ;; require the private flag for capabilities we added after.
-   ((eq char ?h)
-    (cond ((eq (car params) 4) ;; (terminfo: smir)
-           (setq term-insert-mode t))
-          ((and private (eq (car params) 7)) ;; (terminfo: smam)
-           (setq term-auto-margins t))
-          ((eq (car params) 47) ;; (terminfo: smcup)
-           (term-switch-to-alternate-sub-buffer t))))
-   ;; \E[?l - DEC Private Mode Reset
-   ((eq char ?l)
-    (cond ((eq (car params) 4) ;; (terminfo: rmir)
-           (setq term-insert-mode nil))
-          ((and private (eq (car params) 7)) ;; (terminfo: rmam)
-           (setq term-auto-margins nil))
-          ((eq (car params) 47) ;; (terminfo: rmcup)
-           (term-switch-to-alternate-sub-buffer nil))))
-
-   ;; Modified to allow ansi coloring -mm
-   ;; \E[m - Set/reset modes, set bg/fg
-   ;;(terminfo: smso,rmso,smul,rmul,rev,bold,dim,sitm,ritm,blink,sgr0,invis,op,setab,setaf)
-   ((eq char ?m)
-    (term--handle-colors-list params))
-
-   ;; \E[6n - Report cursor position (terminfo: u7)
-   ((eq char ?n)
-    (term-handle-deferred-scroll)
-    (process-send-string proc
+	       (car params)))))
+	;; \E[D - cursor left (terminfo: cub)
+	((eq char ?D)
+	(term-move-columns (- (max 1 (car params)))))
+	;; \E[G - cursor motion to absolute column (terminfo: hpa)
+	((eq char ?G)
+	(term-move-columns (- (max 0 (min term-width (car params)))
+			     (term-current-column))))
+	;; \E[J - clear to end of screen (terminfo: ed, clear)
+	((eq char ?J)
+	(term-erase-in-display (car params)))
+	;; \E[K - clear to end of line (terminfo: el, el1)
+	((eq char ?K)
+	(term-erase-in-line (car params)))
+	;; \E[L - insert lines (terminfo: il, il1)
+	((eq char ?L)
+	(term-insert-lines (max 1 (car params))))
+	;; \E[M - delete lines (terminfo: dl, dl1)
+	((eq char ?M)
+	(term-delete-lines (max 1 (car params))))
+	;; \E[P - delete chars (terminfo: dch, dch1)
+	((eq char ?P)
+	(term-delete-chars (max 1 (car params))))
+	;; \E[@ - insert spaces (terminfo: ich)
+	((eq char ?@)
+	(term-insert-spaces (max 1 (car params))))
+	;; \E[?h - DEC Private Mode Set
+
+	;; N.B. we previously had a bug in which we'd decode \e[?<NR>h or
+	;; \e[?<NR>l as a command with zero in the params field and so
+	;; didn't recognize DEC private escape sequences.  However, the
+	;; termcap and terminfo files had the non-? (question mark means DEC
+	;; private) versions, so things kind of worked anyway.  To preserve
+	;; compatibility, we recognize both private- and non-private
+	;; messages for capabilities we added before we fixed the bug but
+	;; require the private flag for capabilities we added after.
+	((eq char ?h)
+	(cond ((eq (car params) 4) ;; (terminfo: smir)
+	      (setq term-insert-mode t))
+	     ((and private (eq (car params) 7)) ;; (terminfo: smam)
+	      (setq term-auto-margins t))
+	     ((eq (car params) 47) ;; (terminfo: smcup)
+	      (term-switch-to-alternate-sub-buffer t))))
+	;; \E[?l - DEC Private Mode Reset
+	((eq char ?l)
+	(cond ((eq (car params) 4) ;; (terminfo: rmir)
+	      (setq term-insert-mode nil))
+	     ((and private (eq (car params) 7)) ;; (terminfo: rmam)
+	      (setq term-auto-margins nil))
+	     ((eq (car params) 47) ;; (terminfo: rmcup)
+	      (term-switch-to-alternate-sub-buffer nil))))
+
+	;; Modified to allow ansi coloring -mm
+	;; \E[m - Set/reset modes, set bg/fg
+	;;(terminfo: smso,rmso,smul,rmul,rev,bold,dim,sitm,ritm,blink,sgr0,invis,op,setab,setaf)
+	((eq char ?m)
+	(term--handle-colors-list params))
+
+	;; \E[6n - Report cursor position (terminfo: u7)
+	((eq char ?n)
+	(term-handle-deferred-scroll)
+	(process-send-string proc
 			 ;; (terminfo: u6)
 			 (format "\e[%s;%sR"
 				 (1+ (term-current-row))
 				 (1+ (term-horizontal-column)))))
-   ;; \E[r - Set scrolling region (terminfo: csr)
-   ((eq char ?r)
-    (term-set-scroll-region
-     (1- (or (nth 0 params) 0))
-     (1- (or (nth 1 params) 0))))
-   (t)))
+	;; \E[r - Set scrolling region (terminfo: csr)
+	((eq char ?r)
+	(term-set-scroll-region
+	(1- (or (nth 0 params) 0))
+	(1- (or (nth 1 params) 0))))
+	(t))))))
 
 (defun term--reset-scroll-region ()
   "Set the scroll region to the full height of the terminal."
-- 
2.49.0