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

From: Eli Zaretskii <[email-masked]>
Date: Fri, 18 Apr 2025 09:05:33 +0300

> Cc: Johannes Altmanninger <[email-masked]>
> 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).

Why cannot you solve this problem by a suitable configuration of faces
in Emacs?

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

We could have this behavior as an option, conditioned on some user
option, perhaps.

> 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.

You don't tell in which Emacs version you see these issues.  Please
tell, it might be important.

Jared, any comments to the above and/or to the patch?

In any case, we are unable to accept such a large contribution without
you assigning the copyright for your changes to the FSF.  If you are
willing to do that, I will send you the form to fill and the
instructions to go with it, to start your legal paperwork rolling.

Thanks.

> ---
>  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
> 
> 
> 
> 
>