unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
@ 2021-08-24  4:02 Jim Porter
  2021-08-24 12:07 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-08-24  4:02 UTC (permalink / raw)
  To: 50179

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

Note: My copyright assignment paperwork is out of date, but I've
already sent a message to assign@ to get it updated. Thus, these
patches won't be able to merge right away. However, I wanted to start
the review process now so that there's plenty of time for
back-and-forth before Emacs 28.1 is released.

With the administrative issues out of the way... these patches provide
support for "bright" ANSI colors (SGR 90-97 and 100-107 for foreground
and background, respectively)[1]. Most of the complexity here is due
to the new defcustoms `*-bold-is-bright'. Enabling this results in
ANSI "bold" text (SGR 1) to be rendered in the bright color palette
(as well as being bold). This is a pretty common option in terminal
emulators; all the ones I looked at[2] support it, and it's often the
default behavior. For me, the main benefit of this option is so I can
easily match the color palettes between Emacs and my terminal
emulator.

I've split this into two patches, one for 'ansi-color' and one for
'term-mode'. Despite the similarity in functionality, the
implementations are pretty different. It might be nice if they could
be unified somehow, but that may be more trouble than it's worth...

- Jim

[1] https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_(Select_Graphic_Rendition)_parameters
[2] gnome-terminal, alacritty, and PuTTY

[-- Attachment #2: 0001-Add-support-for-bright-ANSI-colors-in-ansi-color.patch --]
[-- Type: application/octet-stream, Size: 11382 bytes --]

From 7bdaf244ec1aba9cabaeb8be3cbff7c5b798f4e8 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 23 Aug 2021 17:51:05 -0700
Subject: [PATCH 1/2] Add support for "bright" ANSI colors in ansi-color

* lisp/ansi-color.el (ansi-bright-color-names-vector): New defcustom.
(ansi-color-bold-is-bright): New defcustom.
(ansi-color--find-face): Sort ANSI codes and check
'ansi-color-bold-is-bright'.
(ansi-color-apply-sequence): Support bright ANSI colors.
(ansi-color--fill-color-map): New function.
(ansi-color-make-color-map): Add bright ANSI colors.
(ansi-color-get-face-1): Add BRIGHT parameter.
* test/lisp/ansi-color-tests.el
(ansi-color-apply-on-region-bold-is-bright-test): New function.
---
 etc/NEWS                      |   7 ++
 lisp/ansi-color.el            | 116 ++++++++++++++++++++++++++--------
 test/lisp/ansi-color-tests.el |  51 +++++++++++++--
 3 files changed, 140 insertions(+), 34 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b221f13624..b8e77bee68 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -375,6 +375,13 @@ emulators by using the new input-meta-mode with the special value
 This parameter, similar to 'drag-with-header-line', allows moving frames
 by dragging the tab lines of their topmost windows with the mouse.
 
+---
+** 'ansi-color' now supports "bright" color codes.
+"Bright" ANSI color codes are now displayed when applying ANSI color
+filters using the color values defined in 'ansi-bright-color-names-vector'.
+In addition, bold text with regular ANSI colors can be displayed as
+"bright" if 'ansi-color-bold-is-bright' is non-nil.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 79dc821ea1..23749304a0 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -150,6 +150,48 @@ ansi-color-names-vector
   :version "24.4" ; default colors copied from `xterm-standard-colors'
   :group 'ansi-colors)
 
+(defcustom ansi-bright-color-names-vector
+  ["gray30" "red2" "green2" "yellow2" "blue1" "magenta2" "cyan2" "white"]
+  "Colors used for SGR control sequences determining a \"bright\" color.
+This vector holds the colors used for SGR control sequences parameters
+90 to 97 (bright foreground colors) and 100 to 107 (brightbackground
+colors).
+
+Parameter   Color
+  90  100   bright black
+  91  101   bright red
+  92  102   bright green
+  93  103   bright yellow
+  94  104   bright blue
+  95  105   bright magenta
+  96  106   bright cyan
+  97  107   bright white
+
+This vector is used by `ansi-color-make-color-map' to create a color
+map.  This color map is stored in the variable `ansi-color-map'.
+
+Each element may also be a cons cell where the car and cdr specify the
+foreground and background colors, respectively."
+  :type '(vector (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color)))
+  :set 'ansi-color-map-update
+  :initialize 'custom-initialize-default
+  :version "28.1"
+  :group 'ansi-colors)
+
+(defcustom ansi-color-bold-is-bright nil
+  "If set to non-nil, combining ANSI bold and a color produces the bright
+version of that color."
+  :type 'boolean
+  :version "28.1"
+  :group 'ansi-colors)
+
 (defconst ansi-color-control-seq-regexp
   ;; See ECMA 48, section 5.4 "Control Sequences".
   "\e\\[[\x30-\x3F]*[\x20-\x2F]*[\x40-\x7E]"
@@ -304,9 +346,14 @@ ansi-color-filter-apply
 
 (defun ansi-color--find-face (codes)
   "Return the face corresponding to CODES."
-  (let (faces)
+  ;; Sort the codes in ascending order to can guarantee that "bold" comes
+  ;; before any of the colors.  This ensures that `ansi-color-bold-is-bright'
+  ;; is applied correctly.
+  (let (faces bright (codes (sort (copy-sequence codes) #'<)))
     (while codes
-      (let ((face (ansi-color-get-face-1 (pop codes))))
+      (let ((face (ansi-color-get-face-1 (pop codes) bright)))
+        (when (and ansi-color-bold-is-bright (eq face 'bold))
+          (setq bright t))
 	;; In the (default underline) face, say, the value of the
 	;; "underline" attribute of the `default' face wins.
 	(unless (eq face 'default)
@@ -570,11 +617,11 @@ ansi-color-apply-sequence
 
 For each new code, the following happens: if it is 1-7, add it to
 the list of codes; if it is 21-25 or 27, delete appropriate
-parameters from the list of codes; if it is 30-37 resp. 39, the
-foreground color code is replaced or added resp. deleted; if it
-is 40-47 resp. 49, the background color code is replaced or added
-resp. deleted; any other code is discarded together with the old
-codes.	Finally, the so changed list of codes is returned."
+parameters from the list of codes; if it is 30-37 (or 90-97) resp. 39,
+the foreground color code is replaced or added resp. deleted; if it
+is 40-47 (or 100-107) resp. 49, the background color code is replaced
+or added resp. deleted; any other code is discarded together with the
+old codes.  Finally, the so changed list of codes is returned."
   (let ((new-codes (ansi-color-parse-sequence escape-sequence)))
     (while new-codes
       (let* ((new (pop new-codes))
@@ -591,7 +638,7 @@ ansi-color-apply-sequence
 					(22 (remq 1 codes))
 					(25 (remq 6 codes))
 					(_ codes)))))
-		((or 3 4) (let ((r (mod new 10)))
+		((or 3 4 9 10) (let ((r (mod new 10)))
 			    (unless (= r 8)
 			      (let (beg)
 				(while (and codes (/= q (/ (car codes) 10)))
@@ -603,6 +650,19 @@ ansi-color-apply-sequence
 		(_ nil)))))
     codes))
 
+(defun ansi-color--fill-color-map (map map-index property vector get-color)
+  "Fill a range of color values from VECTOR and store in MAP.
+
+Start filling MAP from MAP-INDEX, and make faces for PROPERTY (`foreground'
+or `background'). GET-COLOR is a function taking an element of VECTOR and
+returning the color value to use."
+  (mapc
+   (lambda (e)
+     (aset map map-index
+           (ansi-color-make-face property (funcall get-color e)))
+     (setq map-index (1+ map-index)) )
+   vector))
+
 (defun ansi-color-make-color-map ()
   "Creates a vector of face definitions and returns it.
 
@@ -611,7 +671,7 @@ ansi-color-make-color-map
 
 The face definitions are based upon the variables
 `ansi-color-faces-vector' and `ansi-color-names-vector'."
-  (let ((map (make-vector 50 nil))
+  (let ((map (make-vector 110 nil))
         (index 0))
     ;; miscellaneous attributes
     (mapc
@@ -620,23 +680,21 @@ ansi-color-make-color-map
        (setq index (1+ index)) )
      ansi-color-faces-vector)
     ;; foreground attributes
-    (setq index 30)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'foreground
-                         (if (consp e) (car e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
+    (ansi-color--fill-color-map
+     map 30 'foreground ansi-color-names-vector
+     (lambda (e) (if (consp e) (car e) e)))
     ;; background attributes
-    (setq index 40)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'background
-                         (if (consp e) (cdr e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
+    (ansi-color--fill-color-map
+     map 40 'background ansi-color-names-vector
+     (lambda (e) (if (consp e) (cdr e) e)))
+    ;; bright foreground attributes
+    (ansi-color--fill-color-map
+     map 90 'foreground ansi-bright-color-names-vector
+     (lambda (e) (if (consp e) (car e) e)))
+    ;; bright background attributes
+    (ansi-color--fill-color-map
+     map 100 'background ansi-bright-color-names-vector
+     (lambda (e) (if (consp e) (cdr e) e)))
     map))
 
 (defvar ansi-color-map (ansi-color-make-color-map)
@@ -660,9 +718,13 @@ ansi-color-map-update
   (set-default symbol value)
   (setq ansi-color-map (ansi-color-make-color-map)))
 
-(defun ansi-color-get-face-1 (ansi-code)
+(defun ansi-color-get-face-1 (ansi-code &optional bright)
   "Get face definition from `ansi-color-map'.
-ANSI-CODE is used as an index into the vector."
+ANSI-CODE is used as an index into the vector.  BRIGHT, if non-nil,
+requests \"bright\" ANSI colors, even if ANSI-CODE is a normal-intensity
+color."
+  (when (and bright (<= 30 ansi-code 49))
+    (setq ansi-code (+ ansi-code 60)))
   (condition-case nil
       (aref ansi-color-map ansi-code)
     (args-out-of-range nil)))
diff --git a/test/lisp/ansi-color-tests.el b/test/lisp/ansi-color-tests.el
index 107dc8e400..c94561bda1 100644
--- a/test/lisp/ansi-color-tests.el
+++ b/test/lisp/ansi-color-tests.el
@@ -25,17 +25,54 @@
 ;;; Code:
 
 (require 'ansi-color)
+(eval-when-compile (require 'cl-lib))
 
-(defvar test-strings '(("\e[33mHello World\e[0m" . "Hello World")
-                       ("\e[1m\e[3m\e[5mbold italics blink\e[0m" . "bold italics blink")))
+(defvar yellow (aref ansi-color-names-vector 3))
+(defvar bright-yellow (aref ansi-bright-color-names-vector 3))
+
+(defvar test-strings
+  `(("\e[33mHello World\e[0m" "Hello World"
+     (foreground-color . ,yellow))
+    ("\e[43mHello World\e[0m" "Hello World"
+     (background-color . ,yellow))
+    ("\e[93mHello World\e[0m" "Hello World"
+     (foreground-color . ,bright-yellow))
+    ("\e[103mHello World\e[0m" "Hello World"
+     (background-color . ,bright-yellow))
+    ("\e[1;33mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[33;1mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[1m\e[33mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[33m\e[1mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[1m\e[3m\e[5mbold italics blink\e[0m" "bold italics blink"
+     (bold italic success))))
 
 (ert-deftest ansi-color-apply-on-region-test ()
-    (dolist (pair test-strings)
-      (with-temp-buffer
-        (insert (car pair))
+  (pcase-dolist (`(,input ,text ,face) test-strings)
+    (with-temp-buffer
+      (insert input)
+      (ansi-color-apply-on-region (point-min) (point-max))
+      (should (equal (buffer-string) text))
+      (should (equal (get-char-property (point-min) 'face) face))
+      (should (not (equal (overlays-at (point-min)) nil))))))
+
+(ert-deftest ansi-color-apply-on-region-bold-is-bright-test ()
+  (pcase-dolist (`(,input ,text ,face ,bright-face) test-strings)
+    (with-temp-buffer
+      (let ((ansi-color-bold-is-bright t))
+        (insert input)
         (ansi-color-apply-on-region (point-min) (point-max))
-        (should (equal (buffer-string) (cdr pair)))
-        (should (not (equal (overlays-at (point-min)) nil))))))
+        (should (equal (buffer-string) text))
+        (should (equal (get-char-property (point-min) 'face)
+                       (or bright-face face)))
+        (should (not (equal (overlays-at (point-min)) nil)))))))
 
 (ert-deftest ansi-color-apply-on-region-preserving-test ()
     (dolist (pair test-strings)
-- 
2.25.1


[-- Attachment #3: 0002-Add-support-for-bright-ANSI-colors-in-term-mode.patch --]
[-- Type: application/octet-stream, Size: 12430 bytes --]

From 68098c8f5ff27f30f835953214fb177cda0a7a5e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 23 Aug 2021 20:12:10 -0700
Subject: [PATCH 2/2] Add support for "bright" ANSI colors in term-mode

* list/term.el (ansi-term-color-vector): Add new faces.
(term-color-white): Tweak colors.
(term-color-bright-*): New faces.
(term-color-bold-is-bright): New defcustom.
(term--maybe-brighten-color): New function.
(term-handle-colors-array): Handle bright colors.
* test/lisp/term-tests.el (term-colors, term-colors-bold-is-bright):
New functions.
---
 etc/NEWS                |   7 ++
 lisp/term.el            | 140 +++++++++++++++++++++++++++++++---------
 test/lisp/term-tests.el |  59 ++++++++++++++++-
 3 files changed, 174 insertions(+), 32 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b8e77bee68..ddc73d01af 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2367,6 +2367,13 @@ based on the current window size.  In previous versions of Emacs, this
 was always done (and that could lead to odd displays when resizing the
 window after starting).  This variable defaults to nil.
 
+---
+*** 'term-mode' now supports "bright" color codes.
+"Bright" ANSI color codes are now displayed using the color values
+defined in 'term-color-bright-*'.  In addition, bold text with regular
+ANSI colors can be displayed as "bright" if 'term-color-bold-is-bright'
+is non-nil.
+
 ** Widget
 
 +++
diff --git a/lisp/term.el b/lisp/term.el
index b3870a814d..398b61b630 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -727,7 +727,15 @@ ansi-term-color-vector
    term-color-blue
    term-color-magenta
    term-color-cyan
-   term-color-white])
+   term-color-white
+   term-color-bright-black
+   term-color-bright-red
+   term-color-bright-green
+   term-color-bright-yellow
+   term-color-bright-blue
+   term-color-bright-magenta
+   term-color-bright-cyan
+   term-color-bright-white])
 
 (defcustom term-default-fg-color nil
   "If non-nil, default color for foreground in Term mode."
@@ -797,10 +805,57 @@ term-color-cyan
   :group 'term)
 
 (defface term-color-white
-  '((t :foreground "white" :background "white"))
+  '((t :foreground "grey90" :background "gray90"))
   "Face used to render white color code."
   :group 'term)
 
+(defface term-color-bright-black
+  '((t :foreground "gray30" :background "gray30"))
+  "Face used to render bright black color code."
+  :group 'term)
+
+(defface term-color-bright-red
+  '((t :foreground "red2" :background "red2"))
+  "Face used to render bright red color code."
+  :group 'term)
+
+(defface term-color-bright-green
+  '((t :foreground "green2" :background "green2"))
+  "Face used to render bright green color code."
+  :group 'term)
+
+(defface term-color-bright-yellow
+  '((t :foreground "yellow2" :background "yellow2"))
+  "Face used to render bright yellow color code."
+  :group 'term)
+
+(defface term-color-bright-blue
+  '((t :foreground "blue1" :background "blue1"))
+  "Face used to render bright blue color code."
+  :group 'term)
+
+(defface term-color-bright-magenta
+  '((t :foreground "magenta2" :background "magenta2"))
+  "Face used to render bright magenta color code."
+  :group 'term)
+
+(defface term-color-bright-cyan
+  '((t :foreground "cyan2" :background "cyan2"))
+  "Face used to render bright cyan color code."
+  :group 'term)
+
+(defface term-color-bright-white
+  '((t :foreground "white" :background "white"))
+  "Face used to render bright white color code."
+  :group 'term)
+
+(defcustom term-color-bold-is-bright nil
+  "If set to non-nil, combining ANSI bold and a color produces the bright
+version of that color."
+  :group 'term
+  :type 'boolean
+  :version "28.1")
+
 (defcustom term-buffer-maximum-size 8192
   "The maximum size in lines for term buffers.
 Term buffers are truncated from the top to be no greater than this number.
@@ -3225,6 +3280,15 @@ term-reset-terminal
   ;; FIXME: No idea why this is here, it looks wrong.  --Stef
   (setq term-ansi-face-already-done nil))
 
+(defun term--maybe-brighten-color (color bold)
+  "Possibly convert COLOR to its bright variant.
+COLOR is an index into `ansi-term-color-vector'.  If BOLD and
+`term-color-bold-is-bright' are non-nil and COLOR is a regular color,
+return the bright version of COLOR; otherwise, return COLOR."
+  (if (and term-color-bold-is-bright bold (<= 1 color 8))
+      (+ color 8)
+    color))
+
 ;; New function to deal with ansi colorized output, as you can see you can
 ;; have any bold/underline/fg/bg/reverse combination. -mm
 
@@ -3264,6 +3328,10 @@ term-handle-colors-array
    ((and (>= parameter 30) (<= parameter 37))
     (setq term-ansi-current-color (- parameter 29)))
 
+   ;; Bright foreground
+   ((and (>= parameter 90) (<= parameter 97))
+    (setq term-ansi-current-color (- parameter 81)))
+
    ;; Reset foreground
    ((eq parameter 39)
     (setq term-ansi-current-color 0))
@@ -3272,6 +3340,10 @@ term-handle-colors-array
    ((and (>= parameter 40) (<= parameter 47))
     (setq term-ansi-current-bg-color (- parameter 39)))
 
+   ;; Bright foreground
+   ((and (>= parameter 100) (<= parameter 107))
+    (setq term-ansi-current-bg-color (- parameter 91)))
+
    ;; Reset background
    ((eq parameter 49)
     (setq term-ansi-current-bg-color 0))
@@ -3290,37 +3362,43 @@ term-handle-colors-array
   ;;          term-ansi-current-bg-color)
 
   (unless term-ansi-face-already-done
-    (if term-ansi-current-invisible
-        (let ((color
-               (if term-ansi-current-reverse
-                   (face-foreground
-                    (elt ansi-term-color-vector term-ansi-current-color)
-                    nil 'default)
-                 (face-background
-                  (elt ansi-term-color-vector term-ansi-current-bg-color)
-                  nil 'default))))
-          (setq term-current-face
-                (list :background color
-                      :foreground color))
-          ) ;; No need to bother with anything else if it's invisible.
-      (setq term-current-face
-            (list :foreground
-                  (face-foreground
-                   (elt ansi-term-color-vector term-ansi-current-color)
-                   nil 'default)
-                  :background
-                  (face-background
-                   (elt ansi-term-color-vector term-ansi-current-bg-color)
-                   nil 'default)
-                  :inverse-video term-ansi-current-reverse))
-
-      (when term-ansi-current-bold
+    (let ((current-color (term--maybe-brighten-color
+                          term-ansi-current-color
+                          term-ansi-current-bold))
+          (current-bg-color (term--maybe-brighten-color
+                             term-ansi-current-bg-color
+                             term-ansi-current-bold)))
+      (if term-ansi-current-invisible
+          (let ((color
+                 (if term-ansi-current-reverse
+                     (face-foreground
+                      (elt ansi-term-color-vector current-color)
+                      nil 'default)
+                   (face-background
+                    (elt ansi-term-color-vector current-bg-color)
+                    nil 'default))))
+            (setq term-current-face
+                  (list :background color
+                        :foreground color))
+            ) ;; No need to bother with anything else if it's invisible.
         (setq term-current-face
-              `(,term-current-face :inherit term-bold)))
+              (list :foreground
+                    (face-foreground
+                     (elt ansi-term-color-vector current-color)
+                     nil 'default)
+                    :background
+                    (face-background
+                     (elt ansi-term-color-vector current-bg-color)
+                     nil 'default)
+                    :inverse-video term-ansi-current-reverse))
+
+        (when term-ansi-current-bold
+          (setq term-current-face
+                `(,term-current-face :inherit term-bold)))
 
-      (when term-ansi-current-underline
-        (setq term-current-face
-              `(,term-current-face :inherit term-underline)))))
+        (when term-ansi-current-underline
+          (setq term-current-face
+                `(,term-current-face :inherit term-underline))))))
 
   ;;	(message "Debug %S" term-current-face)
   ;; FIXME: shouldn't we set term-ansi-face-already-done to t here?  --Stef
diff --git a/test/lisp/term-tests.el b/test/lisp/term-tests.el
index 50ac370b5b..a61d0939ea 100644
--- a/test/lisp/term-tests.el
+++ b/test/lisp/term-tests.el
@@ -28,6 +28,45 @@
 (defvar term-height)                    ; Number of lines in window.
 (defvar term-width)                     ; Number of columns in window.
 
+(defvar yellow-fg-props
+  '(:foreground "yellow3" :background "unspecified-bg" :inverse-video nil))
+(defvar yellow-bg-props
+  '(:foreground "unspecified-fg" :background "yellow3" :inverse-video nil))
+(defvar bright-yellow-fg-props
+  '(:foreground "yellow2" :background "unspecified-bg" :inverse-video nil))
+(defvar bright-yellow-bg-props
+  '(:foreground "unspecified-fg" :background "yellow2" :inverse-video nil))
+
+(defvar ansi-test-strings
+  `(("\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-fg-props))
+    ("\e[43mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-bg-props))
+    ("\e[93mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-fg-props))
+    ("\e[103mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-bg-props))
+    ("\e[1;33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33;1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[1m\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33m\e[1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))))
+
 (defun term-test-screen-from-input (width height input &optional return-var)
   (with-temp-buffer
     (term-mode)
@@ -48,7 +87,7 @@ term-test-screen-from-input
                 (mapc (lambda (input) (term-emulate-terminal proc input)) input)
               (term-emulate-terminal proc input))
       (if return-var (buffer-local-value return-var (current-buffer))
-        (buffer-substring-no-properties (point-min) (point-max))))))
+        (buffer-substring (point-min) (point-max))))))
 
 (ert-deftest term-simple-lines ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
@@ -77,6 +116,24 @@ term-line-wrap
            (term-test-screen-from-input 40 12 (let ((str (make-string 30 ?a)))
                                                 (list str str))))))
 
+(ert-deftest term-colors ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (pcase-dolist (`(,str ,expected) ansi-test-strings)
+    (let ((result (term-test-screen-from-input 40 12 str)))
+      (should (equal result expected))
+      (should (equal (text-properties-at 0 result)
+                     (text-properties-at 0 expected))))))
+
+(ert-deftest term-colors-bold-is-bright ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (let ((term-color-bold-is-bright t))
+    (pcase-dolist (`(,str ,expected ,bright-expected) ansi-test-strings)
+      (let ((expected (or bright-expected expected))
+            (result (term-test-screen-from-input 40 12 str)))
+        (should (equal result expected))
+        (should (equal (text-properties-at 0 result)
+                       (text-properties-at 0 expected)))))))
+
 (ert-deftest term-cursor-movement ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
   ;; Absolute positioning.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-24  4:02 bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode Jim Porter
@ 2021-08-24 12:07 ` Eli Zaretskii
  2021-08-24 17:38   ` Jim Porter
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2021-08-24 12:07 UTC (permalink / raw)
  To: Jim Porter; +Cc: 50179

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Mon, 23 Aug 2021 21:02:46 -0700
> 
> With the administrative issues out of the way... these patches provide
> support for "bright" ANSI colors (SGR 90-97 and 100-107 for foreground
> and background, respectively)[1]. Most of the complexity here is due
> to the new defcustoms `*-bold-is-bright'. Enabling this results in
> ANSI "bold" text (SGR 1) to be rendered in the bright color palette
> (as well as being bold). This is a pretty common option in terminal
> emulators; all the ones I looked at[2] support it, and it's often the
> default behavior. For me, the main benefit of this option is so I can
> easily match the color palettes between Emacs and my terminal
> emulator.
> 
> I've split this into two patches, one for 'ansi-color' and one for
> 'term-mode'. Despite the similarity in functionality, the
> implementations are pretty different. It might be nice if they could
> be unified somehow, but that may be more trouble than it's worth...

Thanks, please see some comments below.

> +(defcustom ansi-bright-color-names-vector
> +  ["gray30" "red2" "green2" "yellow2" "blue1" "magenta2" "cyan2" "white"]
> +  "Colors used for SGR control sequences determining a \"bright\" color.
> +This vector holds the colors used for SGR control sequences parameters
> +90 to 97 (bright foreground colors) and 100 to 107 (brightbackground
> +colors).

I wouldn't offer a customizable list for this: users have no
particular reason to redefine standard colors.

>  (defun ansi-color--find-face (codes)
>    "Return the face corresponding to CODES."
> -  (let (faces)
> +  ;; Sort the codes in ascending order to can guarantee that "bold" comes
                                          ^^^^^^^^^^^^^^^^
Something wrong with the wording here.

> (term-color-bright-*): New faces.

Please name the new faces explicitly.

> +(defcustom term-color-bold-is-bright nil
> +  "If set to non-nil, combining ANSI bold and a color produces the bright
> +version of that color."
> +  :group 'term
> +  :type 'boolean
> +  :version "28.1")

Do we really need 2 separate knobs for these two features?  How
probable is it that the same user will want to have bright colors in
one package, but not in the other?





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-24 12:07 ` Eli Zaretskii
@ 2021-08-24 17:38   ` Jim Porter
  2021-08-24 17:59     ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-08-24 17:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 50179

On Tue, Aug 24, 2021 at 5:07 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > +(defcustom ansi-bright-color-names-vector
> > +  ["gray30" "red2" "green2" "yellow2" "blue1" "magenta2" "cyan2" "white"]
> > +  "Colors used for SGR control sequences determining a \"bright\" color.
> > +This vector holds the colors used for SGR control sequences parameters
> > +90 to 97 (bright foreground colors) and 100 to 107 (brightbackground
> > +colors).
>
> I wouldn't offer a customizable list for this: users have no
> particular reason to redefine standard colors.

I made this a defcustom because `ansi-color-faces-vector' and
`ansi-color-names-vector' are defcustoms too. More practically
speaking, I'd want to customize this new variable to make these colors
match the Emacs theme I use. I chose colors in this patch to
complement `ansi-color-names-vector', but they'd clash with my theme.

> > +(defcustom term-color-bold-is-bright nil
> > +  "If set to non-nil, combining ANSI bold and a color produces the bright
> > +version of that color."
> > +  :group 'term
> > +  :type 'boolean
> > +  :version "28.1")
>
> Do we really need 2 separate knobs for these two features?  How
> probable is it that the same user will want to have bright colors in
> one package, but not in the other?

I doubt anyone would want to control these independently. It'd be nice
to have a single defcustom for this, but I wasn't sure where it should
go in that case. Would it be ok to use `ansi-color-bold-is-bright'
(from patch 1 in `ansi-color.el') in `term.el'? I see that `term.el`
requires `comint.el', which requires `ansi-color.el', so `term.el'
should see it without changing what gets loaded.

(It might even be nice to use the same definitions for the colors in
both `ansi-color.el' and `term.el', but that would take a bit of work
to migrate users'/themes' customizations. Maybe that's something to do
for Emacs 29 so I have time to figure out a good migration path.)





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-24 17:38   ` Jim Porter
@ 2021-08-24 17:59     ` Eli Zaretskii
  2021-08-24 18:59       ` Jim Porter
  2021-08-25  7:06       ` bug#50179: [PATCH] " Kévin Le Gouguec
  0 siblings, 2 replies; 20+ messages in thread
From: Eli Zaretskii @ 2021-08-24 17:59 UTC (permalink / raw)
  To: Jim Porter; +Cc: 50179

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Tue, 24 Aug 2021 10:38:06 -0700
> Cc: 50179@debbugs.gnu.org
> 
> > I wouldn't offer a customizable list for this: users have no
> > particular reason to redefine standard colors.
> 
> I made this a defcustom because `ansi-color-faces-vector' and
> `ansi-color-names-vector' are defcustoms too. More practically
> speaking, I'd want to customize this new variable to make these colors
> match the Emacs theme I use. I chose colors in this patch to
> complement `ansi-color-names-vector', but they'd clash with my theme.

How can named colors change with the theme?  Faces can, but colors are
absolute.  Bright-yellow is the same color whatever the theme.  At
least IMO.  I wonder if others think otherwise.

> > > +(defcustom term-color-bold-is-bright nil
> > > +  "If set to non-nil, combining ANSI bold and a color produces the bright
> > > +version of that color."
> > > +  :group 'term
> > > +  :type 'boolean
> > > +  :version "28.1")
> >
> > Do we really need 2 separate knobs for these two features?  How
> > probable is it that the same user will want to have bright colors in
> > one package, but not in the other?
> 
> I doubt anyone would want to control these independently. It'd be nice
> to have a single defcustom for this, but I wasn't sure where it should
> go in that case.

If we agree to have a single defcustom, then where to put it is
secondary.

> Would it be ok to use `ansi-color-bold-is-bright'
> (from patch 1 in `ansi-color.el') in `term.el'?

Yes, why not?

Thanks.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-24 17:59     ` Eli Zaretskii
@ 2021-08-24 18:59       ` Jim Porter
  2021-08-24 22:53         ` Jim Porter
  2021-08-25  7:06       ` bug#50179: [PATCH] " Kévin Le Gouguec
  1 sibling, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-08-24 18:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 50179

On Tue, Aug 24, 2021 at 10:59 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Jim Porter <jporterbugs@gmail.com>
> > Date: Tue, 24 Aug 2021 10:38:06 -0700
> > Cc: 50179@debbugs.gnu.org
> >
> > > I wouldn't offer a customizable list for this: users have no
> > > particular reason to redefine standard colors.
> >
> > I made this a defcustom because `ansi-color-faces-vector' and
> > `ansi-color-names-vector' are defcustoms too. More practically
> > speaking, I'd want to customize this new variable to make these colors
> > match the Emacs theme I use. I chose colors in this patch to
> > complement `ansi-color-names-vector', but they'd clash with my theme.
>
> How can named colors change with the theme?  Faces can, but colors are
> absolute.  Bright-yellow is the same color whatever the theme.  At
> least IMO.  I wonder if others think otherwise.

The colors in `ansi-bright-color-names-vector' are just the color
values that get used to set face attributes on bits of text. They (and
the values in the other defcustoms mentioned above) get compiled into
`ansi-color-map', which contains entries like `(foreground-color .
"yellow2")'. Those then get applied as face attributes onto the
relevant text. While you'd always want bright-yellow to be some kind
of bright-looking yellow, the exact RGB value for that is just a
matter of preference. In particular, a theme might need to adjust
these color values so that there's appropriate contrast between
ANSI-colorized text and the default foreground/background color
specified by the theme.

> > Would it be ok to use `ansi-color-bold-is-bright'
> > (from patch 1 in `ansi-color.el') in `term.el'?
>
> Yes, why not?

Mostly just my relative unfamiliarity with the preferred way to do
things here. `ansi-color.el' and `term.el' seemed very independent of
each other, and I wasn't sure if there was a particular reason for
that. I'll just use `ansi-color-bold-is-bright' for both patches then.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-24 18:59       ` Jim Porter
@ 2021-08-24 22:53         ` Jim Porter
  2021-08-25 12:04           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-08-24 22:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 50179

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

Ok, here are updated patches addressing your comments. I fixed the
grammatical error in the comment, improved the commit message for the
second patch, and switched to using `ansi-color-bold-is-bright' in
both files. I left `ansi-bright-color-names-vector' as a defcustom
though, since I think that's the right thing to do there (see my
previous message), but I can change that later if we agree on a
different/better route there.

[-- Attachment #2: 0001-Add-support-for-bright-ANSI-colors-in-ansi-color.patch --]
[-- Type: application/octet-stream, Size: 11378 bytes --]

From b924a57e06d66d1c960ee87d03fd9cdacedec1a4 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 23 Aug 2021 17:51:05 -0700
Subject: [PATCH 1/2] Add support for "bright" ANSI colors in ansi-color

* lisp/ansi-color.el (ansi-bright-color-names-vector): New defcustom.
(ansi-color-bold-is-bright): New defcustom.
(ansi-color--find-face): Sort ANSI codes and check
'ansi-color-bold-is-bright'.
(ansi-color-apply-sequence): Support bright ANSI colors.
(ansi-color--fill-color-map): New function.
(ansi-color-make-color-map): Add bright ANSI colors.
(ansi-color-get-face-1): Add BRIGHT parameter.
* test/lisp/ansi-color-tests.el
(ansi-color-apply-on-region-bold-is-bright-test): New function.
---
 etc/NEWS                      |   7 ++
 lisp/ansi-color.el            | 116 ++++++++++++++++++++++++++--------
 test/lisp/ansi-color-tests.el |  51 +++++++++++++--
 3 files changed, 140 insertions(+), 34 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b221f13624..b8e77bee68 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -375,6 +375,13 @@ emulators by using the new input-meta-mode with the special value
 This parameter, similar to 'drag-with-header-line', allows moving frames
 by dragging the tab lines of their topmost windows with the mouse.
 
+---
+** 'ansi-color' now supports "bright" color codes.
+"Bright" ANSI color codes are now displayed when applying ANSI color
+filters using the color values defined in 'ansi-bright-color-names-vector'.
+In addition, bold text with regular ANSI colors can be displayed as
+"bright" if 'ansi-color-bold-is-bright' is non-nil.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 79dc821ea1..0dc9a52388 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -150,6 +150,48 @@ ansi-color-names-vector
   :version "24.4" ; default colors copied from `xterm-standard-colors'
   :group 'ansi-colors)
 
+(defcustom ansi-bright-color-names-vector
+  ["gray30" "red2" "green2" "yellow2" "blue1" "magenta2" "cyan2" "white"]
+  "Colors used for SGR control sequences determining a \"bright\" color.
+This vector holds the colors used for SGR control sequences parameters
+90 to 97 (bright foreground colors) and 100 to 107 (brightbackground
+colors).
+
+Parameter   Color
+  90  100   bright black
+  91  101   bright red
+  92  102   bright green
+  93  103   bright yellow
+  94  104   bright blue
+  95  105   bright magenta
+  96  106   bright cyan
+  97  107   bright white
+
+This vector is used by `ansi-color-make-color-map' to create a color
+map.  This color map is stored in the variable `ansi-color-map'.
+
+Each element may also be a cons cell where the car and cdr specify the
+foreground and background colors, respectively."
+  :type '(vector (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color)))
+  :set 'ansi-color-map-update
+  :initialize 'custom-initialize-default
+  :version "28.1"
+  :group 'ansi-colors)
+
+(defcustom ansi-color-bold-is-bright nil
+  "If set to non-nil, combining ANSI bold and a color produces the bright
+version of that color."
+  :type 'boolean
+  :version "28.1"
+  :group 'ansi-colors)
+
 (defconst ansi-color-control-seq-regexp
   ;; See ECMA 48, section 5.4 "Control Sequences".
   "\e\\[[\x30-\x3F]*[\x20-\x2F]*[\x40-\x7E]"
@@ -304,9 +346,14 @@ ansi-color-filter-apply
 
 (defun ansi-color--find-face (codes)
   "Return the face corresponding to CODES."
-  (let (faces)
+  ;; Sort the codes in ascending order to guarantee that "bold" comes before
+  ;; any of the colors.  This ensures that `ansi-color-bold-is-bright' is
+  ;; applied correctly.
+  (let (faces bright (codes (sort (copy-sequence codes) #'<)))
     (while codes
-      (let ((face (ansi-color-get-face-1 (pop codes))))
+      (let ((face (ansi-color-get-face-1 (pop codes) bright)))
+        (when (and ansi-color-bold-is-bright (eq face 'bold))
+          (setq bright t))
 	;; In the (default underline) face, say, the value of the
 	;; "underline" attribute of the `default' face wins.
 	(unless (eq face 'default)
@@ -570,11 +617,11 @@ ansi-color-apply-sequence
 
 For each new code, the following happens: if it is 1-7, add it to
 the list of codes; if it is 21-25 or 27, delete appropriate
-parameters from the list of codes; if it is 30-37 resp. 39, the
-foreground color code is replaced or added resp. deleted; if it
-is 40-47 resp. 49, the background color code is replaced or added
-resp. deleted; any other code is discarded together with the old
-codes.	Finally, the so changed list of codes is returned."
+parameters from the list of codes; if it is 30-37 (or 90-97) resp. 39,
+the foreground color code is replaced or added resp. deleted; if it
+is 40-47 (or 100-107) resp. 49, the background color code is replaced
+or added resp. deleted; any other code is discarded together with the
+old codes.  Finally, the so changed list of codes is returned."
   (let ((new-codes (ansi-color-parse-sequence escape-sequence)))
     (while new-codes
       (let* ((new (pop new-codes))
@@ -591,7 +638,7 @@ ansi-color-apply-sequence
 					(22 (remq 1 codes))
 					(25 (remq 6 codes))
 					(_ codes)))))
-		((or 3 4) (let ((r (mod new 10)))
+		((or 3 4 9 10) (let ((r (mod new 10)))
 			    (unless (= r 8)
 			      (let (beg)
 				(while (and codes (/= q (/ (car codes) 10)))
@@ -603,6 +650,19 @@ ansi-color-apply-sequence
 		(_ nil)))))
     codes))
 
+(defun ansi-color--fill-color-map (map map-index property vector get-color)
+  "Fill a range of color values from VECTOR and store in MAP.
+
+Start filling MAP from MAP-INDEX, and make faces for PROPERTY (`foreground'
+or `background'). GET-COLOR is a function taking an element of VECTOR and
+returning the color value to use."
+  (mapc
+   (lambda (e)
+     (aset map map-index
+           (ansi-color-make-face property (funcall get-color e)))
+     (setq map-index (1+ map-index)) )
+   vector))
+
 (defun ansi-color-make-color-map ()
   "Creates a vector of face definitions and returns it.
 
@@ -611,7 +671,7 @@ ansi-color-make-color-map
 
 The face definitions are based upon the variables
 `ansi-color-faces-vector' and `ansi-color-names-vector'."
-  (let ((map (make-vector 50 nil))
+  (let ((map (make-vector 110 nil))
         (index 0))
     ;; miscellaneous attributes
     (mapc
@@ -620,23 +680,21 @@ ansi-color-make-color-map
        (setq index (1+ index)) )
      ansi-color-faces-vector)
     ;; foreground attributes
-    (setq index 30)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'foreground
-                         (if (consp e) (car e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
+    (ansi-color--fill-color-map
+     map 30 'foreground ansi-color-names-vector
+     (lambda (e) (if (consp e) (car e) e)))
     ;; background attributes
-    (setq index 40)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'background
-                         (if (consp e) (cdr e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
+    (ansi-color--fill-color-map
+     map 40 'background ansi-color-names-vector
+     (lambda (e) (if (consp e) (cdr e) e)))
+    ;; bright foreground attributes
+    (ansi-color--fill-color-map
+     map 90 'foreground ansi-bright-color-names-vector
+     (lambda (e) (if (consp e) (car e) e)))
+    ;; bright background attributes
+    (ansi-color--fill-color-map
+     map 100 'background ansi-bright-color-names-vector
+     (lambda (e) (if (consp e) (cdr e) e)))
     map))
 
 (defvar ansi-color-map (ansi-color-make-color-map)
@@ -660,9 +718,13 @@ ansi-color-map-update
   (set-default symbol value)
   (setq ansi-color-map (ansi-color-make-color-map)))
 
-(defun ansi-color-get-face-1 (ansi-code)
+(defun ansi-color-get-face-1 (ansi-code &optional bright)
   "Get face definition from `ansi-color-map'.
-ANSI-CODE is used as an index into the vector."
+ANSI-CODE is used as an index into the vector.  BRIGHT, if non-nil,
+requests \"bright\" ANSI colors, even if ANSI-CODE is a normal-intensity
+color."
+  (when (and bright (<= 30 ansi-code 49))
+    (setq ansi-code (+ ansi-code 60)))
   (condition-case nil
       (aref ansi-color-map ansi-code)
     (args-out-of-range nil)))
diff --git a/test/lisp/ansi-color-tests.el b/test/lisp/ansi-color-tests.el
index 107dc8e400..c94561bda1 100644
--- a/test/lisp/ansi-color-tests.el
+++ b/test/lisp/ansi-color-tests.el
@@ -25,17 +25,54 @@
 ;;; Code:
 
 (require 'ansi-color)
+(eval-when-compile (require 'cl-lib))
 
-(defvar test-strings '(("\e[33mHello World\e[0m" . "Hello World")
-                       ("\e[1m\e[3m\e[5mbold italics blink\e[0m" . "bold italics blink")))
+(defvar yellow (aref ansi-color-names-vector 3))
+(defvar bright-yellow (aref ansi-bright-color-names-vector 3))
+
+(defvar test-strings
+  `(("\e[33mHello World\e[0m" "Hello World"
+     (foreground-color . ,yellow))
+    ("\e[43mHello World\e[0m" "Hello World"
+     (background-color . ,yellow))
+    ("\e[93mHello World\e[0m" "Hello World"
+     (foreground-color . ,bright-yellow))
+    ("\e[103mHello World\e[0m" "Hello World"
+     (background-color . ,bright-yellow))
+    ("\e[1;33mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[33;1mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[1m\e[33mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[33m\e[1mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[1m\e[3m\e[5mbold italics blink\e[0m" "bold italics blink"
+     (bold italic success))))
 
 (ert-deftest ansi-color-apply-on-region-test ()
-    (dolist (pair test-strings)
-      (with-temp-buffer
-        (insert (car pair))
+  (pcase-dolist (`(,input ,text ,face) test-strings)
+    (with-temp-buffer
+      (insert input)
+      (ansi-color-apply-on-region (point-min) (point-max))
+      (should (equal (buffer-string) text))
+      (should (equal (get-char-property (point-min) 'face) face))
+      (should (not (equal (overlays-at (point-min)) nil))))))
+
+(ert-deftest ansi-color-apply-on-region-bold-is-bright-test ()
+  (pcase-dolist (`(,input ,text ,face ,bright-face) test-strings)
+    (with-temp-buffer
+      (let ((ansi-color-bold-is-bright t))
+        (insert input)
         (ansi-color-apply-on-region (point-min) (point-max))
-        (should (equal (buffer-string) (cdr pair)))
-        (should (not (equal (overlays-at (point-min)) nil))))))
+        (should (equal (buffer-string) text))
+        (should (equal (get-char-property (point-min) 'face)
+                       (or bright-face face)))
+        (should (not (equal (overlays-at (point-min)) nil)))))))
 
 (ert-deftest ansi-color-apply-on-region-preserving-test ()
     (dolist (pair test-strings)
-- 
2.25.1


[-- Attachment #3: 0002-Add-support-for-bright-ANSI-colors-in-term-mode.patch --]
[-- Type: application/octet-stream, Size: 12371 bytes --]

From 3e1d91e34a563bb750a498d4a75433b3aac1f0e8 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 24 Aug 2021 15:46:06 -0700
Subject: [PATCH 2/2] Add support for "bright" ANSI colors in term-mode

* list/term.el (ansi-term-color-vector): Add new faces.
(term-color-white): Tweak colors.
(term-color-bright-black, term-color-bright-red, term-color-bright-green)
(term-color-bright-yellow, term-color-bright-blue)
(term-color-bright-magenta, term-color-bright-cyan)
(term-color-bright-white): New faces.
(term--maybe-brighten-color): New function.
(term-handle-colors-array): Handle bright colors.
* test/lisp/term-tests.el (term-colors, term-colors-bold-is-bright):
New functions.
---
 etc/NEWS                |   7 +++
 lisp/term.el            | 133 ++++++++++++++++++++++++++++++----------
 test/lisp/term-tests.el |  59 +++++++++++++++++-
 3 files changed, 167 insertions(+), 32 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b8e77bee68..e5aa055fb7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2367,6 +2367,13 @@ based on the current window size.  In previous versions of Emacs, this
 was always done (and that could lead to odd displays when resizing the
 window after starting).  This variable defaults to nil.
 
+---
+*** 'term-mode' now supports "bright" color codes.
+"Bright" ANSI color codes are now displayed using the color values
+defined in 'term-color-bright-*'.  In addition, bold text with regular
+ANSI colors can be displayed as "bright" if 'ansi-color-bold-is-bright'
+is non-nil.
+
 ** Widget
 
 +++
diff --git a/lisp/term.el b/lisp/term.el
index b3870a814d..ef2532182c 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -727,7 +727,15 @@ ansi-term-color-vector
    term-color-blue
    term-color-magenta
    term-color-cyan
-   term-color-white])
+   term-color-white
+   term-color-bright-black
+   term-color-bright-red
+   term-color-bright-green
+   term-color-bright-yellow
+   term-color-bright-blue
+   term-color-bright-magenta
+   term-color-bright-cyan
+   term-color-bright-white])
 
 (defcustom term-default-fg-color nil
   "If non-nil, default color for foreground in Term mode."
@@ -797,10 +805,50 @@ term-color-cyan
   :group 'term)
 
 (defface term-color-white
-  '((t :foreground "white" :background "white"))
+  '((t :foreground "grey90" :background "gray90"))
   "Face used to render white color code."
   :group 'term)
 
+(defface term-color-bright-black
+  '((t :foreground "gray30" :background "gray30"))
+  "Face used to render bright black color code."
+  :group 'term)
+
+(defface term-color-bright-red
+  '((t :foreground "red2" :background "red2"))
+  "Face used to render bright red color code."
+  :group 'term)
+
+(defface term-color-bright-green
+  '((t :foreground "green2" :background "green2"))
+  "Face used to render bright green color code."
+  :group 'term)
+
+(defface term-color-bright-yellow
+  '((t :foreground "yellow2" :background "yellow2"))
+  "Face used to render bright yellow color code."
+  :group 'term)
+
+(defface term-color-bright-blue
+  '((t :foreground "blue1" :background "blue1"))
+  "Face used to render bright blue color code."
+  :group 'term)
+
+(defface term-color-bright-magenta
+  '((t :foreground "magenta2" :background "magenta2"))
+  "Face used to render bright magenta color code."
+  :group 'term)
+
+(defface term-color-bright-cyan
+  '((t :foreground "cyan2" :background "cyan2"))
+  "Face used to render bright cyan color code."
+  :group 'term)
+
+(defface term-color-bright-white
+  '((t :foreground "white" :background "white"))
+  "Face used to render bright white color code."
+  :group 'term)
+
 (defcustom term-buffer-maximum-size 8192
   "The maximum size in lines for term buffers.
 Term buffers are truncated from the top to be no greater than this number.
@@ -3225,6 +3273,15 @@ term-reset-terminal
   ;; FIXME: No idea why this is here, it looks wrong.  --Stef
   (setq term-ansi-face-already-done nil))
 
+(defun term--maybe-brighten-color (color bold)
+  "Possibly convert COLOR to its bright variant.
+COLOR is an index into `ansi-term-color-vector'.  If BOLD and
+`ansi-color-bold-is-bright' are non-nil and COLOR is a regular color,
+return the bright version of COLOR; otherwise, return COLOR."
+  (if (and ansi-color-bold-is-bright bold (<= 1 color 8))
+      (+ color 8)
+    color))
+
 ;; New function to deal with ansi colorized output, as you can see you can
 ;; have any bold/underline/fg/bg/reverse combination. -mm
 
@@ -3264,6 +3321,10 @@ term-handle-colors-array
    ((and (>= parameter 30) (<= parameter 37))
     (setq term-ansi-current-color (- parameter 29)))
 
+   ;; Bright foreground
+   ((and (>= parameter 90) (<= parameter 97))
+    (setq term-ansi-current-color (- parameter 81)))
+
    ;; Reset foreground
    ((eq parameter 39)
     (setq term-ansi-current-color 0))
@@ -3272,6 +3333,10 @@ term-handle-colors-array
    ((and (>= parameter 40) (<= parameter 47))
     (setq term-ansi-current-bg-color (- parameter 39)))
 
+   ;; Bright foreground
+   ((and (>= parameter 100) (<= parameter 107))
+    (setq term-ansi-current-bg-color (- parameter 91)))
+
    ;; Reset background
    ((eq parameter 49)
     (setq term-ansi-current-bg-color 0))
@@ -3290,37 +3355,43 @@ term-handle-colors-array
   ;;          term-ansi-current-bg-color)
 
   (unless term-ansi-face-already-done
-    (if term-ansi-current-invisible
-        (let ((color
-               (if term-ansi-current-reverse
-                   (face-foreground
-                    (elt ansi-term-color-vector term-ansi-current-color)
-                    nil 'default)
-                 (face-background
-                  (elt ansi-term-color-vector term-ansi-current-bg-color)
-                  nil 'default))))
-          (setq term-current-face
-                (list :background color
-                      :foreground color))
-          ) ;; No need to bother with anything else if it's invisible.
-      (setq term-current-face
-            (list :foreground
-                  (face-foreground
-                   (elt ansi-term-color-vector term-ansi-current-color)
-                   nil 'default)
-                  :background
-                  (face-background
-                   (elt ansi-term-color-vector term-ansi-current-bg-color)
-                   nil 'default)
-                  :inverse-video term-ansi-current-reverse))
-
-      (when term-ansi-current-bold
+    (let ((current-color (term--maybe-brighten-color
+                          term-ansi-current-color
+                          term-ansi-current-bold))
+          (current-bg-color (term--maybe-brighten-color
+                             term-ansi-current-bg-color
+                             term-ansi-current-bold)))
+      (if term-ansi-current-invisible
+          (let ((color
+                 (if term-ansi-current-reverse
+                     (face-foreground
+                      (elt ansi-term-color-vector current-color)
+                      nil 'default)
+                   (face-background
+                    (elt ansi-term-color-vector current-bg-color)
+                    nil 'default))))
+            (setq term-current-face
+                  (list :background color
+                        :foreground color))
+            ) ;; No need to bother with anything else if it's invisible.
         (setq term-current-face
-              `(,term-current-face :inherit term-bold)))
+              (list :foreground
+                    (face-foreground
+                     (elt ansi-term-color-vector current-color)
+                     nil 'default)
+                    :background
+                    (face-background
+                     (elt ansi-term-color-vector current-bg-color)
+                     nil 'default)
+                    :inverse-video term-ansi-current-reverse))
+
+        (when term-ansi-current-bold
+          (setq term-current-face
+                `(,term-current-face :inherit term-bold)))
 
-      (when term-ansi-current-underline
-        (setq term-current-face
-              `(,term-current-face :inherit term-underline)))))
+        (when term-ansi-current-underline
+          (setq term-current-face
+                `(,term-current-face :inherit term-underline))))))
 
   ;;	(message "Debug %S" term-current-face)
   ;; FIXME: shouldn't we set term-ansi-face-already-done to t here?  --Stef
diff --git a/test/lisp/term-tests.el b/test/lisp/term-tests.el
index 50ac370b5b..a61d0939ea 100644
--- a/test/lisp/term-tests.el
+++ b/test/lisp/term-tests.el
@@ -28,6 +28,45 @@
 (defvar term-height)                    ; Number of lines in window.
 (defvar term-width)                     ; Number of columns in window.
 
+(defvar yellow-fg-props
+  '(:foreground "yellow3" :background "unspecified-bg" :inverse-video nil))
+(defvar yellow-bg-props
+  '(:foreground "unspecified-fg" :background "yellow3" :inverse-video nil))
+(defvar bright-yellow-fg-props
+  '(:foreground "yellow2" :background "unspecified-bg" :inverse-video nil))
+(defvar bright-yellow-bg-props
+  '(:foreground "unspecified-fg" :background "yellow2" :inverse-video nil))
+
+(defvar ansi-test-strings
+  `(("\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-fg-props))
+    ("\e[43mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-bg-props))
+    ("\e[93mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-fg-props))
+    ("\e[103mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-bg-props))
+    ("\e[1;33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33;1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[1m\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33m\e[1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))))
+
 (defun term-test-screen-from-input (width height input &optional return-var)
   (with-temp-buffer
     (term-mode)
@@ -48,7 +87,7 @@ term-test-screen-from-input
                 (mapc (lambda (input) (term-emulate-terminal proc input)) input)
               (term-emulate-terminal proc input))
       (if return-var (buffer-local-value return-var (current-buffer))
-        (buffer-substring-no-properties (point-min) (point-max))))))
+        (buffer-substring (point-min) (point-max))))))
 
 (ert-deftest term-simple-lines ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
@@ -77,6 +116,24 @@ term-line-wrap
            (term-test-screen-from-input 40 12 (let ((str (make-string 30 ?a)))
                                                 (list str str))))))
 
+(ert-deftest term-colors ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (pcase-dolist (`(,str ,expected) ansi-test-strings)
+    (let ((result (term-test-screen-from-input 40 12 str)))
+      (should (equal result expected))
+      (should (equal (text-properties-at 0 result)
+                     (text-properties-at 0 expected))))))
+
+(ert-deftest term-colors-bold-is-bright ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (let ((term-color-bold-is-bright t))
+    (pcase-dolist (`(,str ,expected ,bright-expected) ansi-test-strings)
+      (let ((expected (or bright-expected expected))
+            (result (term-test-screen-from-input 40 12 str)))
+        (should (equal result expected))
+        (should (equal (text-properties-at 0 result)
+                       (text-properties-at 0 expected)))))))
+
 (ert-deftest term-cursor-movement ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
   ;; Absolute positioning.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-24 17:59     ` Eli Zaretskii
  2021-08-24 18:59       ` Jim Porter
@ 2021-08-25  7:06       ` Kévin Le Gouguec
  2021-08-25 11:57         ` Eli Zaretskii
  1 sibling, 1 reply; 20+ messages in thread
From: Kévin Le Gouguec @ 2021-08-25  7:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jim Porter, 50179

Eli Zaretskii <eliz@gnu.org> writes:

>> > I wouldn't offer a customizable list for this: users have no
>> > particular reason to redefine standard colors.
>> 
>> I made this a defcustom because `ansi-color-faces-vector' and
>> `ansi-color-names-vector' are defcustoms too. More practically
>> speaking, I'd want to customize this new variable to make these colors
>> match the Emacs theme I use. I chose colors in this patch to
>> complement `ansi-color-names-vector', but they'd clash with my theme.
>
> How can named colors change with the theme?  Faces can, but colors are
> absolute.  Bright-yellow is the same color whatever the theme.  At
> least IMO.  I wonder if others think otherwise.

FWIW, a couple of Emacs's built-in themes set these ansi-color options.
See e.g. the Modus themes, which are designed to meet WCAG's highest
standards for colour contrast.

Maybe there's an analogy to make with terminal emulators?  Most of those
(e.g. Konsole, Terminator, Xfce's) allow the user to customize the
16-color palette.

Digging into the history of this 4-bit palette[1], it almost seems like
no two consoles ever used the exact same colors, so these color names
might not be as absolute as say, HTML color names[2].


[1] https://en.wikipedia.org/wiki/ANSI_escape_code#Colors
[2] https://en.wikipedia.org/wiki/Web_colors#HTML_color_names





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-25  7:06       ` bug#50179: [PATCH] " Kévin Le Gouguec
@ 2021-08-25 11:57         ` Eli Zaretskii
  0 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2021-08-25 11:57 UTC (permalink / raw)
  To: Kévin Le Gouguec; +Cc: jporterbugs, 50179

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: Jim Porter <jporterbugs@gmail.com>,  50179@debbugs.gnu.org
> Date: Wed, 25 Aug 2021 09:06:35 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I made this a defcustom because `ansi-color-faces-vector' and
> >> `ansi-color-names-vector' are defcustoms too. More practically
> >> speaking, I'd want to customize this new variable to make these colors
> >> match the Emacs theme I use. I chose colors in this patch to
> >> complement `ansi-color-names-vector', but they'd clash with my theme.
> >
> > How can named colors change with the theme?  Faces can, but colors are
> > absolute.  Bright-yellow is the same color whatever the theme.  At
> > least IMO.  I wonder if others think otherwise.
> 
> FWIW, a couple of Emacs's built-in themes set these ansi-color options.
> See e.g. the Modus themes, which are designed to meet WCAG's highest
> standards for colour contrast.

It's OK to set a variable.  I was asking why would we want to offer
users the opportunity to customize these translations.

> Maybe there's an analogy to make with terminal emulators?  Most of those
> (e.g. Konsole, Terminator, Xfce's) allow the user to customize the
> 16-color palette.

Once again, if someone _really_ wants that, they can set a variable
allright.  I'm asking why would _we_ want that.

> Digging into the history of this 4-bit palette[1], it almost seems like
> no two consoles ever used the exact same colors, so these color names
> might not be as absolute as say, HTML color names[2].

I don't see how this justifies a desfcustom, sorry.  If there's no
110% consensus, we can choose whatever similar color we want.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-24 22:53         ` Jim Porter
@ 2021-08-25 12:04           ` Lars Ingebrigtsen
  2021-08-25 16:41             ` Jim Porter
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-25 12:04 UTC (permalink / raw)
  To: Jim Porter; +Cc: 50179

Jim Porter <jporterbugs@gmail.com> writes:

> Ok, here are updated patches addressing your comments. I fixed the
> grammatical error in the comment, improved the commit message for the
> second patch, and switched to using `ansi-color-bold-is-bright' in
> both files. I left `ansi-bright-color-names-vector' as a defcustom
> though, since I think that's the right thing to do there (see my
> previous message), but I can change that later if we agree on a
> different/better route there.

Thanks; applied to Emacs 28.  But unfortunately, I didn't notice the
test failure until after pushing.  :-/

1 unexpected results:
   FAILED  term-colors-bold-is-bright

And the faces are:

(font-lock-face ((:foreground "yellow3" :background "unspecified-bg"
 :inverse-video nil) :inherit term-bold))
(font-lock-face ((:foreground "yellow2" :background "unspecified-bg"
 :inverse-video nil) :inherit term-bold))


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-25 12:04           ` Lars Ingebrigtsen
@ 2021-08-25 16:41             ` Jim Porter
  2021-08-25 16:46               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-08-25 16:41 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 50179

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

Oops, I ran the tests locally, but I think I forgot to rebuild
term.elc, so naturally the tests passed without needing any changes.
Here's a patch.

However, I just want to be sure it's ok for these patches to merge. As
mentioned in the original message, my copyright assignment is
currently out of date. I contacted assign@ to get it updated, and I
don't foresee any problems, but you never know with companies. If the
maintainers are ok with the patch being in-tree despite this, then I'm
ok with it too; I just wanted to be sure everyone was aware of the
situation.

On Wed, Aug 25, 2021 at 5:04 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> Jim Porter <jporterbugs@gmail.com> writes:
>
> > Ok, here are updated patches addressing your comments. I fixed the
> > grammatical error in the comment, improved the commit message for the
> > second patch, and switched to using `ansi-color-bold-is-bright' in
> > both files. I left `ansi-bright-color-names-vector' as a defcustom
> > though, since I think that's the right thing to do there (see my
> > previous message), but I can change that later if we agree on a
> > different/better route there.
>
> Thanks; applied to Emacs 28.  But unfortunately, I didn't notice the
> test failure until after pushing.  :-/
>
> 1 unexpected results:
>    FAILED  term-colors-bold-is-bright
>
> And the faces are:
>
> (font-lock-face ((:foreground "yellow3" :background "unspecified-bg"
>  :inverse-video nil) :inherit term-bold))
> (font-lock-face ((:foreground "yellow2" :background "unspecified-bg"
>  :inverse-video nil) :inherit term-bold))
>
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

[-- Attachment #2: 0001-Update-a-test-that-got-missed-for-bug-50179.patch --]
[-- Type: application/octet-stream, Size: 968 bytes --]

From 809c420cbc42adf1b5f0d7d8d0f529f6ad7b08f4 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 25 Aug 2021 09:35:49 -0700
Subject: [PATCH] Update a test that got missed for bug#50179

* test/lisp/term-tests.el (term-colors-bold-is-bright): Set
'ansi-color-bold-is-bright'.
---
 test/lisp/term-tests.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/lisp/term-tests.el b/test/lisp/term-tests.el
index a61d0939ea..b6a5e9e814 100644
--- a/test/lisp/term-tests.el
+++ b/test/lisp/term-tests.el
@@ -126,7 +126,7 @@ term-colors
 
 (ert-deftest term-colors-bold-is-bright ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
-  (let ((term-color-bold-is-bright t))
+  (let ((ansi-color-bold-is-bright t))
     (pcase-dolist (`(,str ,expected ,bright-expected) ansi-test-strings)
       (let ((expected (or bright-expected expected))
             (result (term-test-screen-from-input 40 12 str)))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-25 16:41             ` Jim Porter
@ 2021-08-25 16:46               ` Lars Ingebrigtsen
  2021-08-25 16:54                 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-25 16:46 UTC (permalink / raw)
  To: Jim Porter; +Cc: 50179

Jim Porter <jporterbugs@gmail.com> writes:

> Oops, I ran the tests locally, but I think I forgot to rebuild
> term.elc, so naturally the tests passed without needing any changes.
> Here's a patch.

Thanks.

> However, I just want to be sure it's ok for these patches to merge. As
> mentioned in the original message, my copyright assignment is
> currently out of date.

Sorry; I did read that yesterday, but today I'd completely forgotten it
again -- I did mean to wait until the paperwork was sorted.

> I contacted assign@ to get it updated, and I don't foresee any
> problems, but you never know with companies. If the maintainers are ok
> with the patch being in-tree despite this, then I'm ok with it too; I
> just wanted to be sure everyone was aware of the situation.

Hm...  perhaps the best thing to do would be to revert the two patches
while waiting for the paperwork to be finalised.  On the other hand,
that's more churn in the VC.  Uhm...  I'm not sure whether we have a
policy here.  Eli?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-25 16:46               ` Lars Ingebrigtsen
@ 2021-08-25 16:54                 ` Eli Zaretskii
  2021-08-26 13:23                   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2021-08-25 16:54 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: jporterbugs, 50179

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  50179@debbugs.gnu.org
> Date: Wed, 25 Aug 2021 18:46:51 +0200
> 
> > I contacted assign@ to get it updated, and I don't foresee any
> > problems, but you never know with companies. If the maintainers are ok
> > with the patch being in-tree despite this, then I'm ok with it too; I
> > just wanted to be sure everyone was aware of the situation.
> 
> Hm...  perhaps the best thing to do would be to revert the two patches
> while waiting for the paperwork to be finalised.  On the other hand,
> that's more churn in the VC.  Uhm...  I'm not sure whether we have a
> policy here.  Eli?

I think we have to revert and wait for the paperwork to run to
completion.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-25 16:54                 ` Eli Zaretskii
@ 2021-08-26 13:23                   ` Lars Ingebrigtsen
  2021-09-18 18:58                     ` bug#50179: [UPDATED PATCH] " Jim Porter
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-26 13:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jporterbugs, 50179

Eli Zaretskii <eliz@gnu.org> writes:

> I think we have to revert and wait for the paperwork to run to
> completion.

OK; now done.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [UPDATED PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-08-26 13:23                   ` Lars Ingebrigtsen
@ 2021-09-18 18:58                     ` Jim Porter
  2021-09-19 14:45                       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-09-18 18:58 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Eli Zaretskii; +Cc: 50179

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

On 8/26/2021 6:23 AM, Lars Ingebrigtsen wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>> I think we have to revert and wait for the paperwork to run to
>> completion.
> 
> OK; now done.

Ok, my paperwork is complete. I've attached updated patches that should 
apply cleanly again (I had to fix the merge to account for changes to 
NEWS). I also rolled my fix to the term.el tests and Eli's fix to 
include version info in the new term faces into the second patch.

There's one further enhancement that might make sense here: currently, 
color values are set via a vector of strings for `ansi-color', whereas 
they're set via faces for `term-mode'. Would it be better to use faces 
for `ansi-color' too? Perhaps they should even be the *same* faces; is 
there any reason an Emacs user (or package) would want ANSI colors to be 
different between `ansi-color' and `term-mode'? If so, maybe each 
`term-color-FOO' face should inherit from the (hypothetical) 
corresponding `ansi-color-FOO' face?

However, maybe there's a particular reason why `ansi-color' works this 
way that I'm not aware of. If the current way is best, that's fine by me 
too. Otherwise, just let me know and I can update the patches to include 
`ansi-color-FOO' faces.

[-- Attachment #2: 0001-Add-support-for-bright-ANSI-colors-in-ansi-color.patch --]
[-- Type: text/plain, Size: 11308 bytes --]

From 1e830c631b4ecd696cca5a6c37534d7eb2b3fec6 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 23 Aug 2021 17:51:05 -0700
Subject: [PATCH 1/2] Add support for "bright" ANSI colors in ansi-color

* lisp/ansi-color.el (ansi-bright-color-names-vector): New defcustom.
(ansi-color-bold-is-bright): New defcustom.
(ansi-color--find-face): Sort ANSI codes and check
'ansi-color-bold-is-bright'.
(ansi-color-apply-sequence): Support bright ANSI colors.
(ansi-color--fill-color-map): New function.
(ansi-color-make-color-map): Add bright ANSI colors.
(ansi-color-get-face-1): Add BRIGHT parameter.
* test/lisp/ansi-color-tests.el
(ansi-color-apply-on-region-bold-is-bright-test): New function.
---
 etc/NEWS                      |   9 +++
 lisp/ansi-color.el            | 116 ++++++++++++++++++++++++++--------
 test/lisp/ansi-color-tests.el |  51 +++++++++++++--
 3 files changed, 142 insertions(+), 34 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 515f8bac56..94e837705d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2659,6 +2659,15 @@ sequences.
 *** 'comint-delete-output' can now save deleted text in the kill-ring.
 Interactively, 'C-u C-c C-o' triggers this new optional behavior.
 
+** ansi-color.el
+
+---
+*** Supports for "bright" color codes.
+"Bright" ANSI color codes are now displayed when applying ANSI color
+filters using the color values defined in 'ansi-bright-color-names-vector'.
+In addition, bold text with regular ANSI colors can be displayed as
+"bright" if 'ansi-color-bold-is-bright' is non-nil.
+
 ** ERC
 
 ---
diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 79b1c9912f..c54d89c293 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -150,6 +150,48 @@ ansi-color-names-vector
   :version "24.4" ; default colors copied from `xterm-standard-colors'
   :group 'ansi-colors)
 
+(defcustom ansi-bright-color-names-vector
+  ["gray30" "red2" "green2" "yellow2" "blue1" "magenta2" "cyan2" "white"]
+  "Colors used for SGR control sequences determining a \"bright\" color.
+This vector holds the colors used for SGR control sequences parameters
+90 to 97 (bright foreground colors) and 100 to 107 (brightbackground
+colors).
+
+Parameter   Color
+  90  100   bright black
+  91  101   bright red
+  92  102   bright green
+  93  103   bright yellow
+  94  104   bright blue
+  95  105   bright magenta
+  96  106   bright cyan
+  97  107   bright white
+
+This vector is used by `ansi-color-make-color-map' to create a color
+map.  This color map is stored in the variable `ansi-color-map'.
+
+Each element may also be a cons cell where the car and cdr specify the
+foreground and background colors, respectively."
+  :type '(vector (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color))
+                 (choice color (cons color color)))
+  :set 'ansi-color-map-update
+  :initialize 'custom-initialize-default
+  :version "28.1"
+  :group 'ansi-colors)
+
+(defcustom ansi-color-bold-is-bright nil
+  "If set to non-nil, combining ANSI bold and a color produces the bright
+version of that color."
+  :type 'boolean
+  :version "28.1"
+  :group 'ansi-colors)
+
 (defconst ansi-color-control-seq-regexp
   ;; See ECMA 48, section 5.4 "Control Sequences".
   "\e\\[[\x30-\x3F]*[\x20-\x2F]*[\x40-\x7E]"
@@ -304,9 +346,14 @@ ansi-color-filter-apply
 
 (defun ansi-color--find-face (codes)
   "Return the face corresponding to CODES."
-  (let (faces)
+  ;; Sort the codes in ascending order to guarantee that "bold" comes before
+  ;; any of the colors.  This ensures that `ansi-color-bold-is-bright' is
+  ;; applied correctly.
+  (let (faces bright (codes (sort (copy-sequence codes) #'<)))
     (while codes
-      (let ((face (ansi-color-get-face-1 (pop codes))))
+      (let ((face (ansi-color-get-face-1 (pop codes) bright)))
+        (when (and ansi-color-bold-is-bright (eq face 'bold))
+          (setq bright t))
 	;; In the (default underline) face, say, the value of the
 	;; "underline" attribute of the `default' face wins.
 	(unless (eq face 'default)
@@ -570,11 +617,11 @@ ansi-color-apply-sequence
 
 For each new code, the following happens: if it is 1-7, add it to
 the list of codes; if it is 21-25 or 27, delete appropriate
-parameters from the list of codes; if it is 30-37 resp. 39, the
-foreground color code is replaced or added resp. deleted; if it
-is 40-47 resp. 49, the background color code is replaced or added
-resp. deleted; any other code is discarded together with the old
-codes.	Finally, the so changed list of codes is returned."
+parameters from the list of codes; if it is 30-37 (or 90-97) resp. 39,
+the foreground color code is replaced or added resp. deleted; if it
+is 40-47 (or 100-107) resp. 49, the background color code is replaced
+or added resp. deleted; any other code is discarded together with the
+old codes.  Finally, the so changed list of codes is returned."
   (let ((new-codes (ansi-color-parse-sequence escape-sequence)))
     (while new-codes
       (let* ((new (pop new-codes))
@@ -591,7 +638,7 @@ ansi-color-apply-sequence
 					(22 (remq 1 codes))
 					(25 (remq 6 codes))
 					(_ codes)))))
-		((or 3 4) (let ((r (mod new 10)))
+		((or 3 4 9 10) (let ((r (mod new 10)))
 			    (unless (= r 8)
 			      (let (beg)
 				(while (and codes (/= q (/ (car codes) 10)))
@@ -603,6 +650,19 @@ ansi-color-apply-sequence
 		(_ nil)))))
     codes))
 
+(defun ansi-color--fill-color-map (map map-index property vector get-color)
+  "Fill a range of color values from VECTOR and store in MAP.
+
+Start filling MAP from MAP-INDEX, and make faces for PROPERTY (`foreground'
+or `background'). GET-COLOR is a function taking an element of VECTOR and
+returning the color value to use."
+  (mapc
+   (lambda (e)
+     (aset map map-index
+           (ansi-color-make-face property (funcall get-color e)))
+     (setq map-index (1+ map-index)) )
+   vector))
+
 (defun ansi-color-make-color-map ()
   "Creates a vector of face definitions and returns it.
 
@@ -611,7 +671,7 @@ ansi-color-make-color-map
 
 The face definitions are based upon the variables
 `ansi-color-faces-vector' and `ansi-color-names-vector'."
-  (let ((map (make-vector 50 nil))
+  (let ((map (make-vector 110 nil))
         (index 0))
     ;; miscellaneous attributes
     (mapc
@@ -620,23 +680,21 @@ ansi-color-make-color-map
        (setq index (1+ index)) )
      ansi-color-faces-vector)
     ;; foreground attributes
-    (setq index 30)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'foreground
-                         (if (consp e) (car e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
+    (ansi-color--fill-color-map
+     map 30 'foreground ansi-color-names-vector
+     (lambda (e) (if (consp e) (car e) e)))
     ;; background attributes
-    (setq index 40)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'background
-                         (if (consp e) (cdr e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
+    (ansi-color--fill-color-map
+     map 40 'background ansi-color-names-vector
+     (lambda (e) (if (consp e) (cdr e) e)))
+    ;; bright foreground attributes
+    (ansi-color--fill-color-map
+     map 90 'foreground ansi-bright-color-names-vector
+     (lambda (e) (if (consp e) (car e) e)))
+    ;; bright background attributes
+    (ansi-color--fill-color-map
+     map 100 'background ansi-bright-color-names-vector
+     (lambda (e) (if (consp e) (cdr e) e)))
     map))
 
 (defvar ansi-color-map (ansi-color-make-color-map)
@@ -660,9 +718,13 @@ ansi-color-map-update
   (set-default symbol value)
   (setq ansi-color-map (ansi-color-make-color-map)))
 
-(defun ansi-color-get-face-1 (ansi-code)
+(defun ansi-color-get-face-1 (ansi-code &optional bright)
   "Get face definition from `ansi-color-map'.
-ANSI-CODE is used as an index into the vector."
+ANSI-CODE is used as an index into the vector.  BRIGHT, if non-nil,
+requests \"bright\" ANSI colors, even if ANSI-CODE is a normal-intensity
+color."
+  (when (and bright (<= 30 ansi-code 49))
+    (setq ansi-code (+ ansi-code 60)))
   (condition-case nil
       (aref ansi-color-map ansi-code)
     (args-out-of-range nil)))
diff --git a/test/lisp/ansi-color-tests.el b/test/lisp/ansi-color-tests.el
index 107dc8e400..c94561bda1 100644
--- a/test/lisp/ansi-color-tests.el
+++ b/test/lisp/ansi-color-tests.el
@@ -25,17 +25,54 @@
 ;;; Code:
 
 (require 'ansi-color)
+(eval-when-compile (require 'cl-lib))
 
-(defvar test-strings '(("\e[33mHello World\e[0m" . "Hello World")
-                       ("\e[1m\e[3m\e[5mbold italics blink\e[0m" . "bold italics blink")))
+(defvar yellow (aref ansi-color-names-vector 3))
+(defvar bright-yellow (aref ansi-bright-color-names-vector 3))
+
+(defvar test-strings
+  `(("\e[33mHello World\e[0m" "Hello World"
+     (foreground-color . ,yellow))
+    ("\e[43mHello World\e[0m" "Hello World"
+     (background-color . ,yellow))
+    ("\e[93mHello World\e[0m" "Hello World"
+     (foreground-color . ,bright-yellow))
+    ("\e[103mHello World\e[0m" "Hello World"
+     (background-color . ,bright-yellow))
+    ("\e[1;33mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[33;1mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[1m\e[33mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[33m\e[1mHello World\e[0m" "Hello World"
+     (bold (foreground-color . ,yellow))
+     (bold (foreground-color . ,bright-yellow)))
+    ("\e[1m\e[3m\e[5mbold italics blink\e[0m" "bold italics blink"
+     (bold italic success))))
 
 (ert-deftest ansi-color-apply-on-region-test ()
-    (dolist (pair test-strings)
-      (with-temp-buffer
-        (insert (car pair))
+  (pcase-dolist (`(,input ,text ,face) test-strings)
+    (with-temp-buffer
+      (insert input)
+      (ansi-color-apply-on-region (point-min) (point-max))
+      (should (equal (buffer-string) text))
+      (should (equal (get-char-property (point-min) 'face) face))
+      (should (not (equal (overlays-at (point-min)) nil))))))
+
+(ert-deftest ansi-color-apply-on-region-bold-is-bright-test ()
+  (pcase-dolist (`(,input ,text ,face ,bright-face) test-strings)
+    (with-temp-buffer
+      (let ((ansi-color-bold-is-bright t))
+        (insert input)
         (ansi-color-apply-on-region (point-min) (point-max))
-        (should (equal (buffer-string) (cdr pair)))
-        (should (not (equal (overlays-at (point-min)) nil))))))
+        (should (equal (buffer-string) text))
+        (should (equal (get-char-property (point-min) 'face)
+                       (or bright-face face)))
+        (should (not (equal (overlays-at (point-min)) nil)))))))
 
 (ert-deftest ansi-color-apply-on-region-preserving-test ()
     (dolist (pair test-strings)
-- 
2.25.1


[-- Attachment #3: 0002-Add-support-for-bright-ANSI-colors-in-term-mode.patch --]
[-- Type: text/plain, Size: 12521 bytes --]

From 0deddcd4047469075bf7f37b989fe284196d5d85 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 29 Aug 2021 10:55:58 -0700
Subject: [PATCH 2/2] Add support for "bright" ANSI colors in term-mode

* list/term.el (ansi-term-color-vector): Add new faces.
(term-color-white): Tweak colors.
(term-color-bright-black, term-color-bright-red, term-color-bright-green)
(term-color-bright-yellow, term-color-bright-blue)
(term-color-bright-magenta, term-color-bright-cyan)
(term-color-bright-white): New faces.
(term--maybe-brighten-color): New function.
(term-handle-colors-array): Handle bright colors.
* test/lisp/term-tests.el (term-colors, term-colors-bold-is-bright):
New functions.
---
 etc/NEWS                |   7 ++
 lisp/term.el            | 141 +++++++++++++++++++++++++++++++---------
 test/lisp/term-tests.el |  59 ++++++++++++++++-
 3 files changed, 175 insertions(+), 32 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 94e837705d..4802f4e569 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1471,6 +1471,13 @@ based on the current window size.  In previous versions of Emacs, this
 was always done (and that could lead to odd displays when resizing the
 window after starting).  This variable defaults to nil.
 
+---
+*** 'term-mode' now supports "bright" color codes.
+"Bright" ANSI color codes are now displayed using the color values
+defined in 'term-color-bright-*'.  In addition, bold text with regular
+ANSI colors can be displayed as "bright" if 'ansi-color-bold-is-bright'
+is non-nil.
+
 ** Eshell
 
 ---
diff --git a/lisp/term.el b/lisp/term.el
index 42b2e5a248..c1a3db3651 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -727,7 +727,15 @@ ansi-term-color-vector
    term-color-blue
    term-color-magenta
    term-color-cyan
-   term-color-white])
+   term-color-white
+   term-color-bright-black
+   term-color-bright-red
+   term-color-bright-green
+   term-color-bright-yellow
+   term-color-bright-blue
+   term-color-bright-magenta
+   term-color-bright-cyan
+   term-color-bright-white])
 
 (defcustom term-default-fg-color nil
   "If non-nil, default color for foreground in Term mode."
@@ -797,10 +805,58 @@ term-color-cyan
   :group 'term)
 
 (defface term-color-white
-  '((t :foreground "white" :background "white"))
+  '((t :foreground "grey90" :background "gray90"))
   "Face used to render white color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-black
+  '((t :foreground "gray30" :background "gray30"))
+  "Face used to render bright black color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-red
+  '((t :foreground "red2" :background "red2"))
+  "Face used to render bright red color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-green
+  '((t :foreground "green2" :background "green2"))
+  "Face used to render bright green color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-yellow
+  '((t :foreground "yellow2" :background "yellow2"))
+  "Face used to render bright yellow color code."
   :group 'term)
 
+(defface term-color-bright-blue
+  '((t :foreground "blue1" :background "blue1"))
+  "Face used to render bright blue color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-magenta
+  '((t :foreground "magenta2" :background "magenta2"))
+  "Face used to render bright magenta color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-cyan
+  '((t :foreground "cyan2" :background "cyan2"))
+  "Face used to render bright cyan color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-white
+  '((t :foreground "white" :background "white"))
+  "Face used to render bright white color code."
+  :group 'term
+  :version "28.1")
+
 (defcustom term-buffer-maximum-size 8192
   "The maximum size in lines for term buffers.
 Term buffers are truncated from the top to be no greater than this number.
@@ -3225,6 +3281,15 @@ term-reset-terminal
   ;; FIXME: No idea why this is here, it looks wrong.  --Stef
   (setq term-ansi-face-already-done nil))
 
+(defun term--maybe-brighten-color (color bold)
+  "Possibly convert COLOR to its bright variant.
+COLOR is an index into `ansi-term-color-vector'.  If BOLD and
+`ansi-color-bold-is-bright' are non-nil and COLOR is a regular color,
+return the bright version of COLOR; otherwise, return COLOR."
+  (if (and ansi-color-bold-is-bright bold (<= 1 color 8))
+      (+ color 8)
+    color))
+
 ;; New function to deal with ansi colorized output, as you can see you can
 ;; have any bold/underline/fg/bg/reverse combination. -mm
 
@@ -3264,6 +3329,10 @@ term-handle-colors-array
    ((and (>= parameter 30) (<= parameter 37))
     (setq term-ansi-current-color (- parameter 29)))
 
+   ;; Bright foreground
+   ((and (>= parameter 90) (<= parameter 97))
+    (setq term-ansi-current-color (- parameter 81)))
+
    ;; Reset foreground
    ((eq parameter 39)
     (setq term-ansi-current-color 0))
@@ -3272,6 +3341,10 @@ term-handle-colors-array
    ((and (>= parameter 40) (<= parameter 47))
     (setq term-ansi-current-bg-color (- parameter 39)))
 
+   ;; Bright foreground
+   ((and (>= parameter 100) (<= parameter 107))
+    (setq term-ansi-current-bg-color (- parameter 91)))
+
    ;; Reset background
    ((eq parameter 49)
     (setq term-ansi-current-bg-color 0))
@@ -3290,37 +3363,43 @@ term-handle-colors-array
   ;;          term-ansi-current-bg-color)
 
   (unless term-ansi-face-already-done
-    (if term-ansi-current-invisible
-        (let ((color
-               (if term-ansi-current-reverse
-                   (face-foreground
-                    (elt ansi-term-color-vector term-ansi-current-color)
-                    nil 'default)
-                 (face-background
-                  (elt ansi-term-color-vector term-ansi-current-bg-color)
-                  nil 'default))))
-          (setq term-current-face
-                (list :background color
-                      :foreground color))
-          ) ;; No need to bother with anything else if it's invisible.
-      (setq term-current-face
-            (list :foreground
-                  (face-foreground
-                   (elt ansi-term-color-vector term-ansi-current-color)
-                   nil 'default)
-                  :background
-                  (face-background
-                   (elt ansi-term-color-vector term-ansi-current-bg-color)
-                   nil 'default)
-                  :inverse-video term-ansi-current-reverse))
-
-      (when term-ansi-current-bold
+    (let ((current-color (term--maybe-brighten-color
+                          term-ansi-current-color
+                          term-ansi-current-bold))
+          (current-bg-color (term--maybe-brighten-color
+                             term-ansi-current-bg-color
+                             term-ansi-current-bold)))
+      (if term-ansi-current-invisible
+          (let ((color
+                 (if term-ansi-current-reverse
+                     (face-foreground
+                      (elt ansi-term-color-vector current-color)
+                      nil 'default)
+                   (face-background
+                    (elt ansi-term-color-vector current-bg-color)
+                    nil 'default))))
+            (setq term-current-face
+                  (list :background color
+                        :foreground color))
+            ) ;; No need to bother with anything else if it's invisible.
         (setq term-current-face
-              `(,term-current-face :inherit term-bold)))
+              (list :foreground
+                    (face-foreground
+                     (elt ansi-term-color-vector current-color)
+                     nil 'default)
+                    :background
+                    (face-background
+                     (elt ansi-term-color-vector current-bg-color)
+                     nil 'default)
+                    :inverse-video term-ansi-current-reverse))
+
+        (when term-ansi-current-bold
+          (setq term-current-face
+                `(,term-current-face :inherit term-bold)))
 
-      (when term-ansi-current-underline
-        (setq term-current-face
-              `(,term-current-face :inherit term-underline)))))
+        (when term-ansi-current-underline
+          (setq term-current-face
+                `(,term-current-face :inherit term-underline))))))
 
   ;;	(message "Debug %S" term-current-face)
   ;; FIXME: shouldn't we set term-ansi-face-already-done to t here?  --Stef
diff --git a/test/lisp/term-tests.el b/test/lisp/term-tests.el
index 50ac370b5b..b6a5e9e814 100644
--- a/test/lisp/term-tests.el
+++ b/test/lisp/term-tests.el
@@ -28,6 +28,45 @@
 (defvar term-height)                    ; Number of lines in window.
 (defvar term-width)                     ; Number of columns in window.
 
+(defvar yellow-fg-props
+  '(:foreground "yellow3" :background "unspecified-bg" :inverse-video nil))
+(defvar yellow-bg-props
+  '(:foreground "unspecified-fg" :background "yellow3" :inverse-video nil))
+(defvar bright-yellow-fg-props
+  '(:foreground "yellow2" :background "unspecified-bg" :inverse-video nil))
+(defvar bright-yellow-bg-props
+  '(:foreground "unspecified-fg" :background "yellow2" :inverse-video nil))
+
+(defvar ansi-test-strings
+  `(("\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-fg-props))
+    ("\e[43mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-bg-props))
+    ("\e[93mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-fg-props))
+    ("\e[103mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-bg-props))
+    ("\e[1;33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33;1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[1m\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33m\e[1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))))
+
 (defun term-test-screen-from-input (width height input &optional return-var)
   (with-temp-buffer
     (term-mode)
@@ -48,7 +87,7 @@ term-test-screen-from-input
                 (mapc (lambda (input) (term-emulate-terminal proc input)) input)
               (term-emulate-terminal proc input))
       (if return-var (buffer-local-value return-var (current-buffer))
-        (buffer-substring-no-properties (point-min) (point-max))))))
+        (buffer-substring (point-min) (point-max))))))
 
 (ert-deftest term-simple-lines ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
@@ -77,6 +116,24 @@ term-line-wrap
            (term-test-screen-from-input 40 12 (let ((str (make-string 30 ?a)))
                                                 (list str str))))))
 
+(ert-deftest term-colors ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (pcase-dolist (`(,str ,expected) ansi-test-strings)
+    (let ((result (term-test-screen-from-input 40 12 str)))
+      (should (equal result expected))
+      (should (equal (text-properties-at 0 result)
+                     (text-properties-at 0 expected))))))
+
+(ert-deftest term-colors-bold-is-bright ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (let ((ansi-color-bold-is-bright t))
+    (pcase-dolist (`(,str ,expected ,bright-expected) ansi-test-strings)
+      (let ((expected (or bright-expected expected))
+            (result (term-test-screen-from-input 40 12 str)))
+        (should (equal result expected))
+        (should (equal (text-properties-at 0 result)
+                       (text-properties-at 0 expected)))))))
+
 (ert-deftest term-cursor-movement ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
   ;; Absolute positioning.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#50179: [UPDATED PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-09-18 18:58                     ` bug#50179: [UPDATED PATCH] " Jim Porter
@ 2021-09-19 14:45                       ` Lars Ingebrigtsen
  2021-09-22 19:39                         ` bug#50179: [WIP PATCH v3] " Jim Porter
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-19 14:45 UTC (permalink / raw)
  To: Jim Porter; +Cc: 50179

Jim Porter <jporterbugs@gmail.com> writes:

> There's one further enhancement that might make sense here: currently,
> color values are set via a vector of strings for `ansi-color', whereas
> they're set via faces for `term-mode'. Would it be better to use faces
> for `ansi-color' too? Perhaps they should even be the *same* faces; is
> there any reason an Emacs user (or package) would want ANSI colors to
> be different between `ansi-color' and `term-mode'? If so, maybe each
> `term-color-FOO' face should inherit from the (hypothetical)
> corresponding `ansi-color-FOO' face?

It seems like term.el was reworked in

commit ae4969c2d69a74c896eb49c9a34aeb645ffed082
Author:     Julien Danjou <julien@danjou.info>
AuthorDate: Thu Jun 28 12:40:24 2012 +0200

to use faces instead of colour names, but ansi-color wasn't.  Looking at
the code in both ansi-color and term, I think it would indeed make sense
to rework ansi-color to use faces, too, and inheriting like you suggest
makes sense to me.  (We generally prefer using faces instead of colour
names these days.)  But:

> However, maybe there's a particular reason why `ansi-color' works this
> way that I'm not aware of. If the current way is best, that's fine by
> me too. Otherwise, just let me know and I can update the patches to
> include `ansi-color-FOO' faces.

I'm not really very familiar with ansi-color.el.  Does anybody else have
an opinion here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [WIP PATCH v3] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-09-19 14:45                       ` Lars Ingebrigtsen
@ 2021-09-22 19:39                         ` Jim Porter
  2021-09-22 19:49                           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-09-22 19:39 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 50179

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

On 9/19/2021 7:45 AM, Lars Ingebrigtsen wrote:
> It seems like term.el was reworked in
> 
> commit ae4969c2d69a74c896eb49c9a34aeb645ffed082
> Author:     Julien Danjou <julien@danjou.info>
> AuthorDate: Thu Jun 28 12:40:24 2012 +0200
> 
> to use faces instead of colour names, but ansi-color wasn't.  Looking at
> the code in both ansi-color and term, I think it would indeed make sense
> to rework ansi-color to use faces, too, and inheriting like you suggest
> makes sense to me.  (We generally prefer using faces instead of colour
> names these days.)

Since no one chimed in opposing this change, here's a WIP patch that 
adds faces to ansi-color. This removes the use for `ansi-color-map', 
since `ansi-color-get-face-1' does all the work now. I benchmarked this 
a bit and the performance seems on par with the old implementation, 
except when we fontify *more* stuff thanks to the support for additional 
ANSI escapes; in that case, it takes longer, scaling linearly with the 
amount that gets fontified.

I also took the opportunity to redefine a couple of the faces. For 
example, ANSI SGR 7 (negative image) was defined to use the `error' 
face. Given that `:inverse-video' exists, that seems pretty suboptimal.

I don't quite know what to do about man.el though. It has overrides for 
a few of the faces used by ansi-color. I could maintain that behavior 
fairly easily, but maybe it makes sense to have it use the defaults from 
ansi-color. More generally, I wonder what we should do to accommodate 
users of ansi-color who want to apply non-default faces for a specific 
case. I guess they could redefine `ansi-color-basic-faces-vector' and 
friends? That's basically how man.el used to work, and it wouldn't be 
hard to continue supporting that if we wanted.

[-- Attachment #2: 0001-Add-support-for-bright-ANSI-colors-in-ansi-color.patch --]
[-- Type: text/plain, Size: 21246 bytes --]

From 34cd7672413ec8cf43918090bcea03c8675be74e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 22 Sep 2021 12:02:28 -0700
Subject: [PATCH 1/2] Add support for "bright" ANSI colors in ansi-color

* lisp/ansi-color.el (ansi-color-bold, ansi-color-faint, ansi-color-italic)
(ansi-color-underline, ansi-color-slow-blink, ansi-color-fast-blink)
(ansi-color-inverse, ansi-color-red, ansi-color-green, ansi-color-yellow)
(ansi-color-blue, ansi-color-magenta, ansi-color-cyan, ansi-color-white)
(ansi-color-bright-red, ansi-color-bright-green, ansi-color-bright-yellow)
(ansi-color-bright-blue, ansi-color-bright-magenta, ansi-color-bright-cyan)
(ansi-color-bright-white): New faces.
(ansi-color-basic-faces-vector, ansi-color-normal-colors-vector)
(ansi-color-bright-colors-vector): New constants.
(ansi-color-faces-vector, ansi-color-names-vector): Removed.
(ansi-color-bold-is-bright): New defcustom.
(ansi-color--find-face): Sort ANSI codes and check
'ansi-color-bold-is-bright'.
(ansi-color-apply-sequence): Support bright ANSI colors.
(ansi-color--fill-color-map): New function.
(ansi-color-make-color-map, ansi-color-map, ansi-color-map-update): Removed.
(ansi-color-get-face-1): Add BRIGHT parameter.
* lisp/man.el (Man-ansi-color-map): Removed.
* test/lisp/ansi-color-tests.el
(ansi-color-apply-on-region-bold-is-bright-test): New function.
---
 etc/NEWS                      |  15 ++
 lisp/ansi-color.el            | 367 ++++++++++++++++++++++------------
 lisp/man.el                   |   9 +-
 test/lisp/ansi-color-tests.el |  55 ++++-
 4 files changed, 307 insertions(+), 139 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f273b8e82a..cf6823f51b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2703,6 +2703,21 @@ sequences.
 *** 'comint-delete-output' can now save deleted text in the kill-ring.
 Interactively, 'C-u C-c C-o' triggers this new optional behavior.
 
+** ansi-color.el
+
+---
+*** Colors are now defined by faces.
+ANSI SGR codes now have corresponding faces to describe their
+appearance, e.g. 'ansi-color-bold'.
+
+---
+*** Support for "bright" color codes.
+"Bright" ANSI color codes are now displayed when applying ANSI color
+filters using the color values defined by the faces
+'ansi-color-bright-COLOR'.  In addition, bold text with regular ANSI
+colors can be displayed as "bright" if 'ansi-color-bold-is-bright' is
+non-nil.
+
 ** ERC
 
 ---
diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 4315a7f3ce..aabda71068 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -90,36 +90,177 @@ ansi-colors
   :version "21.1"
   :group 'processes)
 
-(defcustom ansi-color-faces-vector
-  [default bold default italic underline success warning error]
+(defface ansi-color-bold
+  '((t :inherit 'bold))
+  "Face used to render bold text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-faint
+  '((t :weight light))
+  "Face used to render faint text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-italic
+  '((t :inherit 'italic))
+  "Face used to render italic text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-underline
+  '((t :inherit 'underline))
+  "Face used to render underlined text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-slow-blink
+  '((t :box (:line-width -1)))
+  "Face used to render slowly blinking text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-fast-blink
+  '((t :box (:line-width -1)))
+  "Face used to render rapidly blinking text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-inverse
+  '((t :inverse-video t))
+  "Face used to render inverted video text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-black
+  '((t :foreground "black" :background "black"))
+  "Face used to render black color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-red
+  '((t :foreground "red3" :background "red3"))
+  "Face used to render red color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-green
+  '((t :foreground "green3" :background "green3"))
+  "Face used to render green color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-yellow
+  '((t :foreground "yellow3" :background "yellow3"))
+  "Face used to render yellow color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-blue
+  '((t :foreground "blue2" :background "blue2"))
+  "Face used to render blue color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-magenta
+  '((t :foreground "magenta3" :background "magenta3"))
+  "Face used to render magenta color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-cyan
+  '((t :foreground "cyan3" :background "cyan3"))
+  "Face used to render cyan color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-white
+  '((t :foreground "grey90" :background "gray90"))
+  "Face used to render white color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-black
+  '((t :foreground "gray30" :background "gray30"))
+  "Face used to render bright black color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-red
+  '((t :foreground "red2" :background "red2"))
+  "Face used to render bright red color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-green
+  '((t :foreground "green2" :background "green2"))
+  "Face used to render bright green color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-yellow
+  '((t :foreground "yellow2" :background "yellow2"))
+  "Face used to render bright yellow color code."
+  :group 'ansi-colors)
+
+(defface ansi-color-bright-blue
+  '((t :foreground "blue1" :background "blue1"))
+  "Face used to render bright blue color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-magenta
+  '((t :foreground "magenta2" :background "magenta2"))
+  "Face used to render bright magenta color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-cyan
+  '((t :foreground "cyan2" :background "cyan2"))
+  "Face used to render bright cyan color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-white
+  '((t :foreground "white" :background "white"))
+  "Face used to render bright white color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defconst ansi-color-basic-faces-vector
+  [nil
+   ansi-color-bold
+   ansi-color-faint
+   ansi-color-italic
+   ansi-color-underline
+   ansi-color-slow-blink
+   ansi-color-fast-blink
+   ansi-color-inverse]
   "Faces used for SGR control sequences determining a face.
 This vector holds the faces used for SGR control sequence parameters 0
 to 7.
 
-Parameter  Description        Face used by default
-  0        default            default
-  1        bold               bold
-  2        faint              default
-  3        italic             italic
-  4        underlined         underline
-  5        slowly blinking    success
-  6        rapidly blinking   warning
-  7        negative image     error
-
-Note that the symbol `default' is special: It will not be combined
-with the current face.
-
-This vector is used by `ansi-color-make-color-map' to create a color
-map.  This color map is stored in the variable `ansi-color-map'."
-  :type '(vector face face face face face face face face)
-  :set 'ansi-color-map-update
-  :initialize 'custom-initialize-default
-  :group 'ansi-colors)
-
-(defcustom ansi-color-names-vector
-  ["black" "red3" "green3" "yellow3" "blue2" "magenta3" "cyan3" "gray90"]
-  "Colors used for SGR control sequences determining a color.
-This vector holds the colors used for SGR control sequences parameters
+Parameter  Description
+  0        default
+  1        bold
+  2        faint
+  3        italic
+  4        underlined
+  5        slowly blinking
+  6        rapidly blinking
+  7        negative image")
+
+(defconst ansi-color-normal-colors-vector
+  [ansi-color-black
+   ansi-color-red
+   ansi-color-green
+   ansi-color-yellow
+   ansi-color-blue
+   ansi-color-magenta
+   ansi-color-cyan
+   ansi-color-white]
+  "Faces used for SGR control sequences determining a color.
+This vector holds the faces used for SGR control sequences parameters
 30 to 37 (foreground colors) and 40 to 47 (background colors).
 
 Parameter  Color
@@ -130,24 +271,37 @@ ansi-color-names-vector
   34  44   blue
   35  45   magenta
   36  46   cyan
-  37  47   white
-
-This vector is used by `ansi-color-make-color-map' to create a color
-map.  This color map is stored in the variable `ansi-color-map'.
-
-Each element may also be a cons cell where the car and cdr specify the
-foreground and background colors, respectively."
-  :type '(vector (choice color (cons color color))
-                 (choice color (cons color color))
-                 (choice color (cons color color))
-                 (choice color (cons color color))
-                 (choice color (cons color color))
-                 (choice color (cons color color))
-                 (choice color (cons color color))
-                 (choice color (cons color color)))
-  :set 'ansi-color-map-update
-  :initialize 'custom-initialize-default
-  :version "24.4" ; default colors copied from `xterm-standard-colors'
+  37  47   white")
+
+(defconst ansi-color-bright-colors-vector
+  [ansi-color-bright-black
+   ansi-color-bright-red
+   ansi-color-bright-green
+   ansi-color-bright-yellow
+   ansi-color-bright-blue
+   ansi-color-bright-magenta
+   ansi-color-bright-cyan
+   ansi-color-bright-white]
+  "Faces used for SGR control sequences determining a \"bright\" color.
+This vector holds the faces used for SGR control sequences parameters
+90 to 97 (bright foreground colors) and 100 to 107 (bright background
+colors).
+
+Parameter   Color
+  90  100   bright black
+  91  101   bright red
+  92  102   bright green
+  93  103   bright yellow
+  94  104   bright blue
+  95  105   bright magenta
+  96  106   bright cyan
+  97  107   bright white")
+
+(defcustom ansi-color-bold-is-bright nil
+  "If set to non-nil, combining ANSI bold and a color produces the bright
+version of that color."
+  :type 'boolean
+  :version "28.1"
   :group 'ansi-colors)
 
 (defconst ansi-color-control-seq-regexp
@@ -304,13 +458,15 @@ ansi-color-filter-apply
 
 (defun ansi-color--find-face (codes)
   "Return the face corresponding to CODES."
-  (let (faces)
+  ;; Sort the codes in ascending order to guarantee that "bold" comes before
+  ;; any of the colors.  This ensures that `ansi-color-bold-is-bright' is
+  ;; applied correctly.
+  (let (faces bright (codes (sort (copy-sequence codes) #'<)))
     (while codes
-      (let ((face (ansi-color-get-face-1 (pop codes))))
-	;; In the (default underline) face, say, the value of the
-	;; "underline" attribute of the `default' face wins.
-	(unless (eq face 'default)
-	  (push face faces))))
+      (when-let ((face (ansi-color-get-face-1 (pop codes) bright)))
+        (when (and ansi-color-bold-is-bright (eq face 'ansi-color-bold))
+          (setq bright t))
+        (push face faces)))
     ;; Avoid some long-lived conses in the common case.
     (if (cdr faces)
 	(nreverse faces)
@@ -321,9 +477,8 @@ ansi-color-apply
 Delete all other control sequences without processing them.
 
 Applies SGR control sequences setting foreground and background colors
-to STRING using text properties and returns the result.  The colors used
-are given in `ansi-color-faces-vector' and `ansi-color-names-vector'.
-See function `ansi-color-apply-sequence' for details.
+to STRING using text properties and returns the result.  See function
+`ansi-color-apply-sequence' for details.
 
 Every call to this function will set and use the buffer-local variable
 `ansi-color-context' to save partial escape sequences and current ansi codes.
@@ -402,8 +557,7 @@ ansi-color-apply-on-region
 SGR control sequences are applied by calling the function
 specified by `ansi-color-apply-face-function'.  The default
 function sets foreground and background colors to the text
-between BEGIN and END, using overlays.  The colors used are given
-in `ansi-color-faces-vector' and `ansi-color-names-vector'.  See
+between BEGIN and END, using overlays.  See function
 `ansi-color-apply-sequence' for details.
 
 Every call to this function will set and use the buffer-local
@@ -570,11 +724,11 @@ ansi-color-apply-sequence
 
 For each new code, the following happens: if it is 1-7, add it to
 the list of codes; if it is 21-25 or 27, delete appropriate
-parameters from the list of codes; if it is 30-37 resp. 39, the
-foreground color code is replaced or added resp. deleted; if it
-is 40-47 resp. 49, the background color code is replaced or added
-resp. deleted; any other code is discarded together with the old
-codes.	Finally, the so changed list of codes is returned."
+parameters from the list of codes; if it is 30-37 (or 90-97) resp. 39,
+the foreground color code is replaced or added resp. deleted; if it
+is 40-47 (or 100-107) resp. 49, the background color code is replaced
+or added resp. deleted; any other code is discarded together with the
+old codes.  Finally, the so changed list of codes is returned."
   (let ((new-codes (ansi-color-parse-sequence escape-sequence)))
     (while new-codes
       (let* ((new (pop new-codes))
@@ -591,7 +745,7 @@ ansi-color-apply-sequence
 					(22 (remq 1 codes))
 					(25 (remq 6 codes))
 					(_ codes)))))
-		((or 3 4) (let ((r (mod new 10)))
+		((or 3 4 9 10) (let ((r (mod new 10)))
 			    (unless (= r 8)
 			      (let (beg)
 				(while (and codes (/= q (/ (car codes) 10)))
@@ -603,69 +757,34 @@ ansi-color-apply-sequence
 		(_ nil)))))
     codes))
 
-(defun ansi-color-make-color-map ()
-  "Create a vector of face definitions and return it.
-
-The index into the vector is an ANSI code.  See the documentation of
-`ansi-color-map' for an example.
-
-The face definitions are based upon the variables
-`ansi-color-faces-vector' and `ansi-color-names-vector'."
-  (let ((map (make-vector 50 nil))
-        (index 0))
-    ;; miscellaneous attributes
-    (mapc
-     (lambda (e)
-       (aset map index e)
-       (setq index (1+ index)) )
-     ansi-color-faces-vector)
-    ;; foreground attributes
-    (setq index 30)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'foreground
-                         (if (consp e) (car e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
-    ;; background attributes
-    (setq index 40)
-    (mapc
-     (lambda (e)
-       (aset map index
-             (ansi-color-make-face 'background
-                         (if (consp e) (cdr e) e)))
-       (setq index (1+ index)) )
-     ansi-color-names-vector)
-    map))
-
-(defvar ansi-color-map (ansi-color-make-color-map)
-  "A brand new color map suitable for `ansi-color-get-face'.
-
-The value of this variable is usually constructed by
-`ansi-color-make-color-map'.  The values in the array are such that the
-numbers included in an SGR control sequences point to the correct
-foreground or background colors.
-
-Example: The sequence \\033[34m specifies a blue foreground.  Therefore:
-     (aref ansi-color-map 34)
-          => (foreground-color . \"blue\")")
-
-(defun ansi-color-map-update (symbol value)
-  "Update `ansi-color-map'.
-
-Whenever the vectors used to construct `ansi-color-map' are changed,
-this function is called.  Therefore this function is listed as the :set
-property of `ansi-color-faces-vector' and `ansi-color-names-vector'."
-  (set-default symbol value)
-  (setq ansi-color-map (ansi-color-make-color-map)))
-
-(defun ansi-color-get-face-1 (ansi-code)
-  "Get face definition from `ansi-color-map'.
-ANSI-CODE is used as an index into the vector."
-  (condition-case nil
-      (aref ansi-color-map ansi-code)
-    (args-out-of-range nil)))
+(defun ansi-color-get-face-1 (ansi-code &optional bright)
+  "Get face definition for ANSI-CODE.
+BRIGHT, if non-nil, requests \"bright\" ANSI colors, even if ANSI-CODE
+is a normal-intensity color."
+  (when (and bright (<= 30 ansi-code 49))
+    (setq ansi-code (+ ansi-code 60)))
+  (cond ((<= 0 ansi-code 7)
+         (aref ansi-color-basic-faces-vector ansi-code))
+        ((<= 30 ansi-code 38)
+         (list :foreground
+               (face-foreground
+                (aref ansi-color-normal-colors-vector (- ansi-code 30))
+                nil 'default)))
+        ((<= 40 ansi-code 48)
+         (list :background
+               (face-background
+                (aref ansi-color-normal-colors-vector (- ansi-code 40))
+                nil 'default)))
+        ((<= 90 ansi-code 98)
+         (list :foreground
+               (face-foreground
+                (aref ansi-color-bright-colors-vector (- ansi-code 90))
+                nil 'default)))
+        ((<= 100 ansi-code 108)
+         (list :background
+               (face-background
+                (aref ansi-color-bright-colors-vector (- ansi-code 100))
+                nil 'default)))))
 
 (provide 'ansi-color)
 
diff --git a/lisp/man.el b/lisp/man.el
index 6009a31919..9aa27c3fd6 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -141,12 +141,6 @@ Man-reverse
   :group 'man
   :version "24.3")
 
-(defvar Man-ansi-color-map (let ((ansi-color-faces-vector
-				  [ default Man-overstrike default Man-underline
-				    Man-underline default default Man-reverse ]))
-			     (ansi-color-make-color-map))
-  "The value used here for `ansi-color-map'.")
-
 (defcustom Man-notify-method 'friendly
   "Selects the behavior when manpage is ready.
 This variable may have one of the following values, where (sf) means
@@ -1242,8 +1236,7 @@ Man-fontify-manpage
   (interactive nil man-common)
   (goto-char (point-min))
   ;; Fontify ANSI escapes.
-  (let ((ansi-color-apply-face-function #'ansi-color-apply-text-property-face)
-	(ansi-color-map Man-ansi-color-map))
+  (let ((ansi-color-apply-face-function #'ansi-color-apply-text-property-face))
     (ansi-color-apply-on-region (point-min) (point-max)))
   ;; Other highlighting.
   (let ((buffer-undo-list t))
diff --git a/test/lisp/ansi-color-tests.el b/test/lisp/ansi-color-tests.el
index 107dc8e400..df674dfc7f 100644
--- a/test/lisp/ansi-color-tests.el
+++ b/test/lisp/ansi-color-tests.el
@@ -25,17 +25,58 @@
 ;;; Code:
 
 (require 'ansi-color)
+(eval-when-compile (require 'cl-lib))
 
-(defvar test-strings '(("\e[33mHello World\e[0m" . "Hello World")
-                       ("\e[1m\e[3m\e[5mbold italics blink\e[0m" . "bold italics blink")))
+(defvar yellow (face-foreground 'ansi-color-yellow nil 'default))
+(defvar bright-yellow (face-foreground 'ansi-color-bright-yellow nil 'default))
+
+(defvar test-strings
+  `(("Hello World" "Hello World")
+    ("\e[33mHello World\e[0m" "Hello World"
+     (:foreground ,yellow))
+    ("\e[43mHello World\e[0m" "Hello World"
+     (:background ,yellow))
+    ("\e[93mHello World\e[0m" "Hello World"
+     (:foreground ,bright-yellow))
+    ("\e[103mHello World\e[0m" "Hello World"
+     (:background ,bright-yellow))
+    ("\e[1;33mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[33;1mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[1m\e[33mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[33m\e[1mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[1m\e[3m\e[5mbold italics blink\e[0m" "bold italics blink"
+     (ansi-color-bold ansi-color-italic ansi-color-slow-blink))
+    ("\e[10munrecognized\e[0m" "unrecognized")))
 
 (ert-deftest ansi-color-apply-on-region-test ()
-    (dolist (pair test-strings)
-      (with-temp-buffer
-        (insert (car pair))
+  (pcase-dolist (`(,input ,text ,face) test-strings)
+    (with-temp-buffer
+      (insert input)
+      (ansi-color-apply-on-region (point-min) (point-max))
+      (should (equal (buffer-string) text))
+      (should (equal (get-char-property (point-min) 'face) face))
+      (when face
+        (should (not (equal (overlays-at (point-min)) nil)))))))
+
+(ert-deftest ansi-color-apply-on-region-bold-is-bright-test ()
+  (pcase-dolist (`(,input ,text ,normal-face ,bright-face) test-strings)
+    (with-temp-buffer
+      (let ((ansi-color-bold-is-bright t)
+            (face (or bright-face normal-face)))
+        (insert input)
         (ansi-color-apply-on-region (point-min) (point-max))
-        (should (equal (buffer-string) (cdr pair)))
-        (should (not (equal (overlays-at (point-min)) nil))))))
+        (should (equal (buffer-string) text))
+        (should (equal (get-char-property (point-min) 'face) face))
+        (when face
+          (should (not (equal (overlays-at (point-min)) nil))))))))
 
 (ert-deftest ansi-color-apply-on-region-preserving-test ()
     (dolist (pair test-strings)
-- 
2.25.1


[-- Attachment #3: 0002-Add-support-for-bright-ANSI-colors-in-term-mode.patch --]
[-- Type: text/plain, Size: 14627 bytes --]

From 113d9f8d2df7b58655850ca9e0a058a55bd36619 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 22 Sep 2021 12:09:03 -0700
Subject: [PATCH 2/2] Add support for "bright" ANSI colors in term-mode

* list/term.el (ansi-term-color-vector): Add new faces.
(term-color-black, term-color-red, term-color-green, term-color-yellow)
(term-color-blue, term-color-magenta, term-color-cyan, term-color-white):
Inherit from 'ansi-color-COLOR'.
(term-color-bright-black, term-color-bright-red, term-color-bright-green)
(term-color-bright-yellow, term-color-bright-blue)
(term-color-bright-magenta, term-color-bright-cyan)
(term-color-bright-white): New faces.
(term--maybe-brighten-color): New function.
(term-handle-colors-array): Handle bright colors.
* test/lisp/term-tests.el (term-colors, term-colors-bold-is-bright):
New functions.
---
 etc/NEWS                |   7 ++
 lisp/term.el            | 186 +++++++++++++++++++++++++++++-----------
 test/lisp/term-tests.el |  65 +++++++++++++-
 3 files changed, 208 insertions(+), 50 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index cf6823f51b..27c6996bb3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1508,6 +1508,13 @@ based on the current window size.  In previous versions of Emacs, this
 was always done (and that could lead to odd displays when resizing the
 window after starting).  This variable defaults to nil.
 
+---
+*** 'term-mode' now supports "bright" color codes.
+"Bright" ANSI color codes are now displayed using the color values
+defined in 'term-color-bright-*'.  In addition, bold text with regular
+ANSI colors can be displayed as "bright" if 'ansi-color-bold-is-bright'
+is non-nil.
+
 ** Eshell
 
 ---
diff --git a/lisp/term.el b/lisp/term.el
index af93089104..f57fcc8cd5 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -727,7 +727,15 @@ ansi-term-color-vector
    term-color-blue
    term-color-magenta
    term-color-cyan
-   term-color-white])
+   term-color-white
+   term-color-bright-black
+   term-color-bright-red
+   term-color-bright-green
+   term-color-bright-yellow
+   term-color-bright-blue
+   term-color-bright-magenta
+   term-color-bright-cyan
+   term-color-bright-white])
 
 (defcustom term-default-fg-color nil
   "If non-nil, default color for foreground in Term mode."
@@ -752,55 +760,112 @@ term
   :group 'term)
 
 (defface term-bold
-  '((t :bold t))
+  '((t :inherit ansi-color-bold))
   "Default face to use for bold text."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-underline
-  '((t :underline t))
+  '((t :inherit ansi-color-underline))
   "Default face to use for underlined text."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-black
-  '((t :foreground "black" :background "black"))
+  '((t :inherit ansi-color-black))
   "Face used to render black color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-red
-  '((t :foreground "red3" :background "red3"))
+  '((t :inherit ansi-color-red))
   "Face used to render red color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-green
-  '((t :foreground "green3" :background "green3"))
+  '((t :inherit ansi-color-green))
   "Face used to render green color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-yellow
-  '((t :foreground "yellow3" :background "yellow3"))
+  '((t :inherit ansi-color-yellow))
   "Face used to render yellow color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-blue
-  '((t :foreground "blue2" :background "blue2"))
+  '((t :inherit ansi-color-blue))
   "Face used to render blue color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-magenta
-  '((t :foreground "magenta3" :background "magenta3"))
+  '((t :inherit ansi-color-magenta))
   "Face used to render magenta color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-cyan
-  '((t :foreground "cyan3" :background "cyan3"))
+  '((t :inherit ansi-color-cyan))
   "Face used to render cyan color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-white
-  '((t :foreground "white" :background "white"))
+  '((t :inherit ansi-color-white))
   "Face used to render white color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-black
+  '((t :inherit ansi-color-bright-black))
+  "Face used to render bright black color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-red
+  '((t :inherit ansi-color-bright-red))
+  "Face used to render bright red color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-green
+  '((t :inherit ansi-color-bright-green))
+  "Face used to render bright green color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-yellow
+  '((t :inherit ansi-color-bright-yellow))
+  "Face used to render bright yellow color code."
   :group 'term)
 
+(defface term-color-bright-blue
+  '((t :inherit ansi-color-bright-blue))
+  "Face used to render bright blue color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-magenta
+  '((t :inherit ansi-color-bright-magenta))
+  "Face used to render bright magenta color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-cyan
+  '((t :inherit ansi-color-bright-cyan))
+  "Face used to render bright cyan color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-white
+  '((t :inherit ansi-color-bright-white))
+  "Face used to render bright white color code."
+  :group 'term
+  :version "28.1")
+
 (defcustom term-buffer-maximum-size 8192
   "The maximum size in lines for term buffers.
 Term buffers are truncated from the top to be no greater than this number.
@@ -3223,6 +3288,15 @@ term-reset-terminal
   ;; FIXME: No idea why this is here, it looks wrong.  --Stef
   (setq term-ansi-face-already-done nil))
 
+(defun term--maybe-brighten-color (color bold)
+  "Possibly convert COLOR to its bright variant.
+COLOR is an index into `ansi-term-color-vector'.  If BOLD and
+`ansi-color-bold-is-bright' are non-nil and COLOR is a regular color,
+return the bright version of COLOR; otherwise, return COLOR."
+  (if (and ansi-color-bold-is-bright bold (<= 1 color 8))
+      (+ color 8)
+    color))
+
 ;; New function to deal with ansi colorized output, as you can see you can
 ;; have any bold/underline/fg/bg/reverse combination. -mm
 
@@ -3262,6 +3336,10 @@ term-handle-colors-array
    ((and (>= parameter 30) (<= parameter 37))
     (setq term-ansi-current-color (- parameter 29)))
 
+   ;; Bright foreground
+   ((and (>= parameter 90) (<= parameter 97))
+    (setq term-ansi-current-color (- parameter 81)))
+
    ;; Reset foreground
    ((eq parameter 39)
     (setq term-ansi-current-color 0))
@@ -3270,6 +3348,10 @@ term-handle-colors-array
    ((and (>= parameter 40) (<= parameter 47))
     (setq term-ansi-current-bg-color (- parameter 39)))
 
+   ;; Bright foreground
+   ((and (>= parameter 100) (<= parameter 107))
+    (setq term-ansi-current-bg-color (- parameter 91)))
+
    ;; Reset background
    ((eq parameter 49)
     (setq term-ansi-current-bg-color 0))
@@ -3288,37 +3370,43 @@ term-handle-colors-array
   ;;          term-ansi-current-bg-color)
 
   (unless term-ansi-face-already-done
-    (if term-ansi-current-invisible
-        (let ((color
-               (if term-ansi-current-reverse
-                   (face-foreground
-                    (elt ansi-term-color-vector term-ansi-current-color)
-                    nil 'default)
-                 (face-background
-                  (elt ansi-term-color-vector term-ansi-current-bg-color)
-                  nil 'default))))
-          (setq term-current-face
-                (list :background color
-                      :foreground color))
-          ) ;; No need to bother with anything else if it's invisible.
-      (setq term-current-face
-            (list :foreground
-                  (face-foreground
-                   (elt ansi-term-color-vector term-ansi-current-color)
-                   nil 'default)
-                  :background
-                  (face-background
-                   (elt ansi-term-color-vector term-ansi-current-bg-color)
-                   nil 'default)
-                  :inverse-video term-ansi-current-reverse))
-
-      (when term-ansi-current-bold
+    (let ((current-color (term--maybe-brighten-color
+                          term-ansi-current-color
+                          term-ansi-current-bold))
+          (current-bg-color (term--maybe-brighten-color
+                             term-ansi-current-bg-color
+                             term-ansi-current-bold)))
+      (if term-ansi-current-invisible
+          (let ((color
+                 (if term-ansi-current-reverse
+                     (face-foreground
+                      (elt ansi-term-color-vector current-color)
+                      nil 'default)
+                   (face-background
+                    (elt ansi-term-color-vector current-bg-color)
+                    nil 'default))))
+            (setq term-current-face
+                  (list :background color
+                        :foreground color))
+            ) ;; No need to bother with anything else if it's invisible.
         (setq term-current-face
-              `(,term-current-face :inherit term-bold)))
+              (list :foreground
+                    (face-foreground
+                     (elt ansi-term-color-vector current-color)
+                     nil 'default)
+                    :background
+                    (face-background
+                     (elt ansi-term-color-vector current-bg-color)
+                     nil 'default)
+                    :inverse-video term-ansi-current-reverse))
+
+        (when term-ansi-current-bold
+          (setq term-current-face
+                `(,term-current-face :inherit term-bold)))
 
-      (when term-ansi-current-underline
-        (setq term-current-face
-              `(,term-current-face :inherit term-underline)))))
+        (when term-ansi-current-underline
+          (setq term-current-face
+                `(,term-current-face :inherit term-underline))))))
 
   ;;	(message "Debug %S" term-current-face)
   ;; FIXME: shouldn't we set term-ansi-face-already-done to t here?  --Stef
diff --git a/test/lisp/term-tests.el b/test/lisp/term-tests.el
index 50ac370b5b..96b6d73488 100644
--- a/test/lisp/term-tests.el
+++ b/test/lisp/term-tests.el
@@ -28,6 +28,51 @@
 (defvar term-height)                    ; Number of lines in window.
 (defvar term-width)                     ; Number of columns in window.
 
+(defvar yellow-fg-props
+  `( :foreground ,(face-foreground 'term-color-yellow nil 'default)
+     :background "unspecified-bg" :inverse-video nil))
+(defvar yellow-bg-props
+  `( :foreground "unspecified-fg"
+     :background ,(face-background 'term-color-yellow nil 'default)
+     :inverse-video nil))
+(defvar bright-yellow-fg-props
+  `( :foreground ,(face-foreground 'term-color-bright-yellow nil 'default)
+     :background "unspecified-bg" :inverse-video nil))
+(defvar bright-yellow-bg-props
+  `( :foreground "unspecified-fg"
+     :background ,(face-background 'term-color-bright-yellow nil 'default)
+     :inverse-video nil))
+
+(defvar ansi-test-strings
+  `(("\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-fg-props))
+    ("\e[43mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-bg-props))
+    ("\e[93mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-fg-props))
+    ("\e[103mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-bg-props))
+    ("\e[1;33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33;1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[1m\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33m\e[1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))))
+
 (defun term-test-screen-from-input (width height input &optional return-var)
   (with-temp-buffer
     (term-mode)
@@ -48,7 +93,7 @@ term-test-screen-from-input
                 (mapc (lambda (input) (term-emulate-terminal proc input)) input)
               (term-emulate-terminal proc input))
       (if return-var (buffer-local-value return-var (current-buffer))
-        (buffer-substring-no-properties (point-min) (point-max))))))
+        (buffer-substring (point-min) (point-max))))))
 
 (ert-deftest term-simple-lines ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
@@ -77,6 +122,24 @@ term-line-wrap
            (term-test-screen-from-input 40 12 (let ((str (make-string 30 ?a)))
                                                 (list str str))))))
 
+(ert-deftest term-colors ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (pcase-dolist (`(,str ,expected) ansi-test-strings)
+    (let ((result (term-test-screen-from-input 40 12 str)))
+      (should (equal result expected))
+      (should (equal (text-properties-at 0 result)
+                     (text-properties-at 0 expected))))))
+
+(ert-deftest term-colors-bold-is-bright ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (let ((ansi-color-bold-is-bright t))
+    (pcase-dolist (`(,str ,expected ,bright-expected) ansi-test-strings)
+      (let ((expected (or bright-expected expected))
+            (result (term-test-screen-from-input 40 12 str)))
+        (should (equal result expected))
+        (should (equal (text-properties-at 0 result)
+                       (text-properties-at 0 expected)))))))
+
 (ert-deftest term-cursor-movement ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
   ;; Absolute positioning.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#50179: [WIP PATCH v3] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-09-22 19:39                         ` bug#50179: [WIP PATCH v3] " Jim Porter
@ 2021-09-22 19:49                           ` Lars Ingebrigtsen
  2021-09-23  1:47                             ` bug#50179: [PATCH v4] " Jim Porter
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-22 19:49 UTC (permalink / raw)
  To: Jim Porter; +Cc: 50179

Jim Porter <jporterbugs@gmail.com> writes:

> I don't quite know what to do about man.el though. It has overrides
> for a few of the faces used by ansi-color. I could maintain that
> behavior fairly easily, but maybe it makes sense to have it use the
> defaults from ansi-color.

Hm...  I wonder why man.el is overriding the colours?

Skimming the patch, it looks good to me, but some trivial comments:

> -(defcustom ansi-color-faces-vector
> -  [default bold default italic underline success warning error]

Instead of removing the variable, mark it as obsolete instead.  (Users
may have code that references it in their .emacs files, and that
shouldn't bug out.)

> +(defconst ansi-color-basic-faces-vector
> +  [nil

[...]

> -  :type '(vector face face face face face face face face)
> -  :set 'ansi-color-map-update
> -  :initialize 'custom-initialize-default
> -  :group 'ansi-colors)

Needs a :version.

> -(defcustom ansi-color-names-vector
> -  ["black" "red3" "green3" "yellow3" "blue2" "magenta3" "cyan3" "gray90"]

Same thing with obsoletion here.

> -(defvar Man-ansi-color-map (let ((ansi-color-faces-vector
> -				  [ default Man-overstrike default Man-underline
> -				    Man-underline default default Man-reverse ]))
> -			     (ansi-color-make-color-map))
> -  "The value used here for `ansi-color-map'.")

And same obsoletion thing here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH v4] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-09-22 19:49                           ` Lars Ingebrigtsen
@ 2021-09-23  1:47                             ` Jim Porter
  2021-09-23 20:58                               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 20+ messages in thread
From: Jim Porter @ 2021-09-23  1:47 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 50179

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

On 9/22/2021 12:49 PM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
> 
>> I don't quite know what to do about man.el though. It has overrides
>> for a few of the faces used by ansi-color. I could maintain that
>> behavior fairly easily, but maybe it makes sense to have it use the
>> defaults from ansi-color.
> 
> Hm...  I wonder why man.el is overriding the colours?

Since I'm not totally sure, I maintained that behavior. It's not really 
any extra effort, so someone can make that decision later.

> Skimming the patch, it looks good to me, but some trivial comments:
> 
>> -(defcustom ansi-color-faces-vector
>> -  [default bold default italic underline success warning error]
> 
> Instead of removing the variable, mark it as obsolete instead.  (Users
> may have code that references it in their .emacs files, and that
> shouldn't bug out.)

Ok, I've done this here, and all the other places as well. I also 
obsoleted the old `ansi-color-map' and friends instead of deleting them 
for the same reason. Hopefully all that's right; I tried to make sure 
that they warn the user if they use them, but that warnings don't show 
up when building Emacs.

[-- Attachment #2: 0001-Add-support-for-bright-ANSI-colors-in-ansi-color.patch --]
[-- Type: text/plain, Size: 23175 bytes --]

From 7083f0be951f3100c3a3249313ede7391df09281 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 22 Sep 2021 18:37:52 -0700
Subject: [PATCH 1/2] Add support for "bright" ANSI colors in ansi-color

* lisp/ansi-color.el (ansi-color-bold, ansi-color-faint, ansi-color-italic)
(ansi-color-underline, ansi-color-slow-blink, ansi-color-fast-blink)
(ansi-color-inverse, ansi-color-red, ansi-color-green, ansi-color-yellow)
(ansi-color-blue, ansi-color-magenta, ansi-color-cyan, ansi-color-white)
(ansi-color-bright-red, ansi-color-bright-green, ansi-color-bright-yellow)
(ansi-color-bright-blue, ansi-color-bright-magenta, ansi-color-bright-cyan)
(ansi-color-bright-white): New faces.
(ansi-color-basic-faces-vector, ansi-color-normal-colors-vector)
(ansi-color-bright-colors-vector): New constants.
(ansi-color-faces-vector, ansi-color-names-vector): Make obsolete.
(ansi-color-bold-is-bright): New defcustom.
(ansi-color--find-face): Sort ANSI codes and check
'ansi-color-bold-is-bright'.
(ansi-color-apply-sequence): Support bright ANSI colors.
(ansi-color-make-color-map, ansi-color-map, ansi-color-map-update):
Make obsolete.
(ansi-color-get-face-1): Add BRIGHT parameter.
* lisp/man.el (Man-ansi-color-basic-faces-vector): New variable.
(Man-ansi-color-map): Make obsolete.
(Man-fontify-manpage): Use 'Man-ansi-color-basic-faces-vector' here.
* test/lisp/ansi-color-tests.el
(ansi-color-apply-on-region-bold-is-bright-test): New function.
---
 etc/NEWS                      |  15 ++
 lisp/ansi-color.el            | 355 +++++++++++++++++++++++++++-------
 lisp/man.el                   |  22 ++-
 test/lisp/ansi-color-tests.el |  55 +++++-
 4 files changed, 365 insertions(+), 82 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f273b8e82a..cf6823f51b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2703,6 +2703,21 @@ sequences.
 *** 'comint-delete-output' can now save deleted text in the kill-ring.
 Interactively, 'C-u C-c C-o' triggers this new optional behavior.
 
+** ansi-color.el
+
+---
+*** Colors are now defined by faces.
+ANSI SGR codes now have corresponding faces to describe their
+appearance, e.g. 'ansi-color-bold'.
+
+---
+*** Support for "bright" color codes.
+"Bright" ANSI color codes are now displayed when applying ANSI color
+filters using the color values defined by the faces
+'ansi-color-bright-COLOR'.  In addition, bold text with regular ANSI
+colors can be displayed as "bright" if 'ansi-color-bold-is-bright' is
+non-nil.
+
 ** ERC
 
 ---
diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 4315a7f3ce..b1c9cdaeca 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -90,53 +90,168 @@ ansi-colors
   :version "21.1"
   :group 'processes)
 
+(defface ansi-color-bold
+  '((t :inherit 'bold))
+  "Face used to render bold text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-faint
+  '((t :weight light))
+  "Face used to render faint text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-italic
+  '((t :inherit 'italic))
+  "Face used to render italic text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-underline
+  '((t :inherit 'underline))
+  "Face used to render underlined text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-slow-blink
+  '((t :box (:line-width -1)))
+  "Face used to render slowly blinking text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-fast-blink
+  '((t :box (:line-width -1)))
+  "Face used to render rapidly blinking text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-inverse
+  '((t :inverse-video t))
+  "Face used to render inverted video text."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-black
+  '((t :foreground "black" :background "black"))
+  "Face used to render black color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-red
+  '((t :foreground "red3" :background "red3"))
+  "Face used to render red color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-green
+  '((t :foreground "green3" :background "green3"))
+  "Face used to render green color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-yellow
+  '((t :foreground "yellow3" :background "yellow3"))
+  "Face used to render yellow color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-blue
+  '((t :foreground "blue2" :background "blue2"))
+  "Face used to render blue color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-magenta
+  '((t :foreground "magenta3" :background "magenta3"))
+  "Face used to render magenta color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-cyan
+  '((t :foreground "cyan3" :background "cyan3"))
+  "Face used to render cyan color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-white
+  '((t :foreground "grey90" :background "gray90"))
+  "Face used to render white color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-black
+  '((t :foreground "gray30" :background "gray30"))
+  "Face used to render bright black color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-red
+  '((t :foreground "red2" :background "red2"))
+  "Face used to render bright red color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-green
+  '((t :foreground "green2" :background "green2"))
+  "Face used to render bright green color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-yellow
+  '((t :foreground "yellow2" :background "yellow2"))
+  "Face used to render bright yellow color code."
+  :group 'ansi-colors)
+
+(defface ansi-color-bright-blue
+  '((t :foreground "blue1" :background "blue1"))
+  "Face used to render bright blue color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-magenta
+  '((t :foreground "magenta2" :background "magenta2"))
+  "Face used to render bright magenta color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-cyan
+  '((t :foreground "cyan2" :background "cyan2"))
+  "Face used to render bright cyan color code."
+  :group 'ansi-colors
+  :version "28.1")
+
+(defface ansi-color-bright-white
+  '((t :foreground "white" :background "white"))
+  "Face used to render bright white color code."
+  :group 'ansi-colors
+  :version "28.1")
+
 (defcustom ansi-color-faces-vector
   [default bold default italic underline success warning error]
   "Faces used for SGR control sequences determining a face.
 This vector holds the faces used for SGR control sequence parameters 0
 to 7.
 
-Parameter  Description        Face used by default
-  0        default            default
-  1        bold               bold
-  2        faint              default
-  3        italic             italic
-  4        underlined         underline
-  5        slowly blinking    success
-  6        rapidly blinking   warning
-  7        negative image     error
-
-Note that the symbol `default' is special: It will not be combined
-with the current face.
-
-This vector is used by `ansi-color-make-color-map' to create a color
-map.  This color map is stored in the variable `ansi-color-map'."
+This variable is obsolete.  To customize the display of faces used by
+ansi-color, change 'ansi-color-FACE', e.g. `ansi-color-bold'.  To
+customize the actual faces used (e.g. to temporarily display SGR
+control sequences differently), use `ansi-color-basic-faces-vector'."
   :type '(vector face face face face face face face face)
-  :set 'ansi-color-map-update
-  :initialize 'custom-initialize-default
   :group 'ansi-colors)
+(make-obsolete-variable 'ansi-color-faces-vector 'ansi-color-basic-faces-vector
+                        "28.1")
 
 (defcustom ansi-color-names-vector
   ["black" "red3" "green3" "yellow3" "blue2" "magenta3" "cyan3" "gray90"]
   "Colors used for SGR control sequences determining a color.
-This vector holds the colors used for SGR control sequences parameters
+This vector holds the colors used for SGR control sequence parameters
 30 to 37 (foreground colors) and 40 to 47 (background colors).
 
-Parameter  Color
-  30  40   black
-  31  41   red
-  32  42   green
-  33  43   yellow
-  34  44   blue
-  35  45   magenta
-  36  46   cyan
-  37  47   white
-
-This vector is used by `ansi-color-make-color-map' to create a color
-map.  This color map is stored in the variable `ansi-color-map'.
-
-Each element may also be a cons cell where the car and cdr specify the
-foreground and background colors, respectively."
+This variable is obsolete.  To customize the display of colors used by
+ansi-color, change 'ansi-color-COLOR', e.g. `ansi-color-red'.  To
+customize the actual faces used (e.g. to temporarily display SGR
+control sequences differently), use `ansi-color-normal-colors-vector'."
   :type '(vector (choice color (cons color color))
                  (choice color (cons color color))
                  (choice color (cons color color))
@@ -145,10 +260,87 @@ ansi-color-names-vector
                  (choice color (cons color color))
                  (choice color (cons color color))
                  (choice color (cons color color)))
-  :set 'ansi-color-map-update
-  :initialize 'custom-initialize-default
   :version "24.4" ; default colors copied from `xterm-standard-colors'
   :group 'ansi-colors)
+(make-obsolete-variable 'ansi-color-faces-vector
+                        'ansi-color-normal-colors-vector "28.1")
+
+(defvar ansi-color-basic-faces-vector
+  [nil
+   ansi-color-bold
+   ansi-color-faint
+   ansi-color-italic
+   ansi-color-underline
+   ansi-color-slow-blink
+   ansi-color-fast-blink
+   ansi-color-inverse]
+  "Faces used for SGR control sequences determining a face.
+This vector holds the faces used for SGR control sequence parameters 0
+to 7.
+
+Parameter  Description
+  0        default
+  1        bold
+  2        faint
+  3        italic
+  4        underlined
+  5        slowly blinking
+  6        rapidly blinking
+  7        negative image")
+
+(defvar ansi-color-normal-colors-vector
+  [ansi-color-black
+   ansi-color-red
+   ansi-color-green
+   ansi-color-yellow
+   ansi-color-blue
+   ansi-color-magenta
+   ansi-color-cyan
+   ansi-color-white]
+  "Faces used for SGR control sequences determining a color.
+This vector holds the faces used for SGR control sequence parameters
+30 to 37 (foreground colors) and 40 to 47 (background colors).
+
+Parameter  Color
+  30  40   black
+  31  41   red
+  32  42   green
+  33  43   yellow
+  34  44   blue
+  35  45   magenta
+  36  46   cyan
+  37  47   white")
+
+(defvar ansi-color-bright-colors-vector
+  [ansi-color-bright-black
+   ansi-color-bright-red
+   ansi-color-bright-green
+   ansi-color-bright-yellow
+   ansi-color-bright-blue
+   ansi-color-bright-magenta
+   ansi-color-bright-cyan
+   ansi-color-bright-white]
+  "Faces used for SGR control sequences determining a \"bright\" color.
+This vector holds the faces used for SGR control sequence parameters
+90 to 97 (bright foreground colors) and 100 to 107 (bright background
+colors).
+
+Parameter   Color
+  90  100   bright black
+  91  101   bright red
+  92  102   bright green
+  93  103   bright yellow
+  94  104   bright blue
+  95  105   bright magenta
+  96  106   bright cyan
+  97  107   bright white")
+
+(defcustom ansi-color-bold-is-bright nil
+  "If set to non-nil, combining ANSI bold and a color produces the bright
+version of that color."
+  :type 'boolean
+  :version "28.1"
+  :group 'ansi-colors)
 
 (defconst ansi-color-control-seq-regexp
   ;; See ECMA 48, section 5.4 "Control Sequences".
@@ -304,13 +496,15 @@ ansi-color-filter-apply
 
 (defun ansi-color--find-face (codes)
   "Return the face corresponding to CODES."
-  (let (faces)
+  ;; Sort the codes in ascending order to guarantee that "bold" comes before
+  ;; any of the colors.  This ensures that `ansi-color-bold-is-bright' is
+  ;; applied correctly.
+  (let (faces bright (codes (sort (copy-sequence codes) #'<)))
     (while codes
-      (let ((face (ansi-color-get-face-1 (pop codes))))
-	;; In the (default underline) face, say, the value of the
-	;; "underline" attribute of the `default' face wins.
-	(unless (eq face 'default)
-	  (push face faces))))
+      (when-let ((face (ansi-color-get-face-1 (pop codes) bright)))
+        (when (and ansi-color-bold-is-bright (eq face 'ansi-color-bold))
+          (setq bright t))
+        (push face faces)))
     ;; Avoid some long-lived conses in the common case.
     (if (cdr faces)
 	(nreverse faces)
@@ -321,9 +515,8 @@ ansi-color-apply
 Delete all other control sequences without processing them.
 
 Applies SGR control sequences setting foreground and background colors
-to STRING using text properties and returns the result.  The colors used
-are given in `ansi-color-faces-vector' and `ansi-color-names-vector'.
-See function `ansi-color-apply-sequence' for details.
+to STRING using text properties and returns the result.  See function
+`ansi-color-apply-sequence' for details.
 
 Every call to this function will set and use the buffer-local variable
 `ansi-color-context' to save partial escape sequences and current ansi codes.
@@ -402,8 +595,7 @@ ansi-color-apply-on-region
 SGR control sequences are applied by calling the function
 specified by `ansi-color-apply-face-function'.  The default
 function sets foreground and background colors to the text
-between BEGIN and END, using overlays.  The colors used are given
-in `ansi-color-faces-vector' and `ansi-color-names-vector'.  See
+between BEGIN and END, using overlays.  See function
 `ansi-color-apply-sequence' for details.
 
 Every call to this function will set and use the buffer-local
@@ -570,11 +762,11 @@ ansi-color-apply-sequence
 
 For each new code, the following happens: if it is 1-7, add it to
 the list of codes; if it is 21-25 or 27, delete appropriate
-parameters from the list of codes; if it is 30-37 resp. 39, the
-foreground color code is replaced or added resp. deleted; if it
-is 40-47 resp. 49, the background color code is replaced or added
-resp. deleted; any other code is discarded together with the old
-codes.	Finally, the so changed list of codes is returned."
+parameters from the list of codes; if it is 30-37 (or 90-97) resp. 39,
+the foreground color code is replaced or added resp. deleted; if it
+is 40-47 (or 100-107) resp. 49, the background color code is replaced
+or added resp. deleted; any other code is discarded together with the
+old codes.  Finally, the so changed list of codes is returned."
   (let ((new-codes (ansi-color-parse-sequence escape-sequence)))
     (while new-codes
       (let* ((new (pop new-codes))
@@ -591,7 +783,7 @@ ansi-color-apply-sequence
 					(22 (remq 1 codes))
 					(25 (remq 6 codes))
 					(_ codes)))))
-		((or 3 4) (let ((r (mod new 10)))
+		((or 3 4 9 10) (let ((r (mod new 10)))
 			    (unless (= r 8)
 			      (let (beg)
 				(while (and codes (/= q (/ (car codes) 10)))
@@ -610,7 +802,9 @@ ansi-color-make-color-map
 `ansi-color-map' for an example.
 
 The face definitions are based upon the variables
-`ansi-color-faces-vector' and `ansi-color-names-vector'."
+`ansi-color-faces-vector' and `ansi-color-names-vector'.
+
+This function is obsolete, and no longer needed to use ansi-color."
   (let ((map (make-vector 50 nil))
         (index 0))
     ;; miscellaneous attributes
@@ -638,34 +832,57 @@ ansi-color-make-color-map
        (setq index (1+ index)) )
      ansi-color-names-vector)
     map))
+(make-obsolete 'ansi-color-make-color-map "you can remove it." "28.1")
 
-(defvar ansi-color-map (ansi-color-make-color-map)
-  "A brand new color map suitable for `ansi-color-get-face'.
+(defvar ansi-color-map
+  (with-no-warnings (ansi-color-make-color-map))
+  "A brand new color map, formerly suitable for `ansi-color-get-face'.
 
 The value of this variable is usually constructed by
 `ansi-color-make-color-map'.  The values in the array are such that the
 numbers included in an SGR control sequences point to the correct
 foreground or background colors.
 
-Example: The sequence \\033[34m specifies a blue foreground.  Therefore:
-     (aref ansi-color-map 34)
-          => (foreground-color . \"blue\")")
+This variable is obsolete, and no longer needed to use ansi-color.")
+(make-obsolete-variable 'ansi-color-map "you can remove it." "28.1")
 
 (defun ansi-color-map-update (symbol value)
   "Update `ansi-color-map'.
 
-Whenever the vectors used to construct `ansi-color-map' are changed,
-this function is called.  Therefore this function is listed as the :set
-property of `ansi-color-faces-vector' and `ansi-color-names-vector'."
+This function is obsolete, and no longer needed to use ansi-color."
   (set-default symbol value)
-  (setq ansi-color-map (ansi-color-make-color-map)))
-
-(defun ansi-color-get-face-1 (ansi-code)
-  "Get face definition from `ansi-color-map'.
-ANSI-CODE is used as an index into the vector."
-  (condition-case nil
-      (aref ansi-color-map ansi-code)
-    (args-out-of-range nil)))
+  (with-no-warnings
+    (setq ansi-color-map (ansi-color-make-color-map))))
+(make-obsolete 'ansi-color-map-update "you can remove it." "28.1")
+
+(defun ansi-color-get-face-1 (ansi-code &optional bright)
+  "Get face definition for ANSI-CODE.
+BRIGHT, if non-nil, requests \"bright\" ANSI colors, even if ANSI-CODE
+is a normal-intensity color."
+  (when (and bright (<= 30 ansi-code 49))
+    (setq ansi-code (+ ansi-code 60)))
+  (cond ((<= 0 ansi-code 7)
+         (aref ansi-color-basic-faces-vector ansi-code))
+        ((<= 30 ansi-code 38)
+         (list :foreground
+               (face-foreground
+                (aref ansi-color-normal-colors-vector (- ansi-code 30))
+                nil 'default)))
+        ((<= 40 ansi-code 48)
+         (list :background
+               (face-background
+                (aref ansi-color-normal-colors-vector (- ansi-code 40))
+                nil 'default)))
+        ((<= 90 ansi-code 98)
+         (list :foreground
+               (face-foreground
+                (aref ansi-color-bright-colors-vector (- ansi-code 90))
+                nil 'default)))
+        ((<= 100 ansi-code 108)
+         (list :background
+               (face-background
+                (aref ansi-color-bright-colors-vector (- ansi-code 100))
+                nil 'default)))))
 
 (provide 'ansi-color)
 
diff --git a/lisp/man.el b/lisp/man.el
index 6009a31919..84287c9f9d 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -141,11 +141,21 @@ Man-reverse
   :group 'man
   :version "24.3")
 
-(defvar Man-ansi-color-map (let ((ansi-color-faces-vector
-				  [ default Man-overstrike default Man-underline
-				    Man-underline default default Man-reverse ]))
-			     (ansi-color-make-color-map))
-  "The value used here for `ansi-color-map'.")
+(defvar Man-ansi-color-basic-faces-vector
+  [nil Man-overstrike nil Man-underline Man-underline nil nil Man-reverse]
+  "The value used here for `ansi-color-basic-faces-vector'.")
+
+(defvar Man-ansi-color-map
+  (with-no-warnings
+    (let ((ansi-color-faces-vector Man-ansi-color-basic-faces-vector))
+           [ default Man-overstrike default Man-underline
+             Man-underline default default Man-reverse ]))
+      (ansi-color-make-color-map)))
+  "The value formerly used here for `ansi-color-map'.
+This variable is obsolete.  To customize the faces used by ansi-color,
+set `Man-ansi-color-basic-faces-vector'.")
+(make-obsolete-variable 'Man-ansi-color-map
+                        'Man-ansi-color-basic-faces-vector "28.1")
 
 (defcustom Man-notify-method 'friendly
   "Selects the behavior when manpage is ready.
@@ -1243,7 +1253,7 @@ Man-fontify-manpage
   (goto-char (point-min))
   ;; Fontify ANSI escapes.
   (let ((ansi-color-apply-face-function #'ansi-color-apply-text-property-face)
-	(ansi-color-map Man-ansi-color-map))
+	(ansi-color-basic-faces-vector Man-ansi-color-basic-faces-vector))
     (ansi-color-apply-on-region (point-min) (point-max)))
   ;; Other highlighting.
   (let ((buffer-undo-list t))
diff --git a/test/lisp/ansi-color-tests.el b/test/lisp/ansi-color-tests.el
index 107dc8e400..df674dfc7f 100644
--- a/test/lisp/ansi-color-tests.el
+++ b/test/lisp/ansi-color-tests.el
@@ -25,17 +25,58 @@
 ;;; Code:
 
 (require 'ansi-color)
+(eval-when-compile (require 'cl-lib))
 
-(defvar test-strings '(("\e[33mHello World\e[0m" . "Hello World")
-                       ("\e[1m\e[3m\e[5mbold italics blink\e[0m" . "bold italics blink")))
+(defvar yellow (face-foreground 'ansi-color-yellow nil 'default))
+(defvar bright-yellow (face-foreground 'ansi-color-bright-yellow nil 'default))
+
+(defvar test-strings
+  `(("Hello World" "Hello World")
+    ("\e[33mHello World\e[0m" "Hello World"
+     (:foreground ,yellow))
+    ("\e[43mHello World\e[0m" "Hello World"
+     (:background ,yellow))
+    ("\e[93mHello World\e[0m" "Hello World"
+     (:foreground ,bright-yellow))
+    ("\e[103mHello World\e[0m" "Hello World"
+     (:background ,bright-yellow))
+    ("\e[1;33mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[33;1mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[1m\e[33mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[33m\e[1mHello World\e[0m" "Hello World"
+     (ansi-color-bold (:foreground ,yellow))
+     (ansi-color-bold (:foreground ,bright-yellow)))
+    ("\e[1m\e[3m\e[5mbold italics blink\e[0m" "bold italics blink"
+     (ansi-color-bold ansi-color-italic ansi-color-slow-blink))
+    ("\e[10munrecognized\e[0m" "unrecognized")))
 
 (ert-deftest ansi-color-apply-on-region-test ()
-    (dolist (pair test-strings)
-      (with-temp-buffer
-        (insert (car pair))
+  (pcase-dolist (`(,input ,text ,face) test-strings)
+    (with-temp-buffer
+      (insert input)
+      (ansi-color-apply-on-region (point-min) (point-max))
+      (should (equal (buffer-string) text))
+      (should (equal (get-char-property (point-min) 'face) face))
+      (when face
+        (should (not (equal (overlays-at (point-min)) nil)))))))
+
+(ert-deftest ansi-color-apply-on-region-bold-is-bright-test ()
+  (pcase-dolist (`(,input ,text ,normal-face ,bright-face) test-strings)
+    (with-temp-buffer
+      (let ((ansi-color-bold-is-bright t)
+            (face (or bright-face normal-face)))
+        (insert input)
         (ansi-color-apply-on-region (point-min) (point-max))
-        (should (equal (buffer-string) (cdr pair)))
-        (should (not (equal (overlays-at (point-min)) nil))))))
+        (should (equal (buffer-string) text))
+        (should (equal (get-char-property (point-min) 'face) face))
+        (when face
+          (should (not (equal (overlays-at (point-min)) nil))))))))
 
 (ert-deftest ansi-color-apply-on-region-preserving-test ()
     (dolist (pair test-strings)
-- 
2.25.1


[-- Attachment #3: 0002-Add-support-for-bright-ANSI-colors-in-term-mode.patch --]
[-- Type: text/plain, Size: 14587 bytes --]

From 1669822acaa36201a2b8dd81d93a3c737c8c72d9 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 22 Sep 2021 18:39:52 -0700
Subject: [PATCH 2/2] Add support for "bright" ANSI colors in term-mode

* list/term.el (ansi-term-color-vector): Add new faces.
(term-color-black, term-color-red, term-color-green, term-color-yellow)
(term-color-blue, term-color-magenta, term-color-cyan, term-color-white):
Inherit from 'ansi-color-COLOR'.
(term-color-bright-black, term-color-bright-red, term-color-bright-green)
(term-color-bright-yellow, term-color-bright-blue)
(term-color-bright-magenta, term-color-bright-cyan)
(term-color-bright-white): New faces.
(term--maybe-brighten-color): New function.
(term-handle-colors-array): Handle bright colors.
* test/lisp/term-tests.el (term-colors, term-colors-bold-is-bright):
New functions.
---
 etc/NEWS                |   7 ++
 lisp/term.el            | 189 +++++++++++++++++++++++++++++-----------
 test/lisp/term-tests.el |  65 +++++++++++++-
 3 files changed, 210 insertions(+), 51 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index cf6823f51b..27c6996bb3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1508,6 +1508,13 @@ based on the current window size.  In previous versions of Emacs, this
 was always done (and that could lead to odd displays when resizing the
 window after starting).  This variable defaults to nil.
 
+---
+*** 'term-mode' now supports "bright" color codes.
+"Bright" ANSI color codes are now displayed using the color values
+defined in 'term-color-bright-*'.  In addition, bold text with regular
+ANSI colors can be displayed as "bright" if 'ansi-color-bold-is-bright'
+is non-nil.
+
 ** Eshell
 
 ---
diff --git a/lisp/term.el b/lisp/term.el
index af93089104..e76eb77647 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -727,7 +727,15 @@ ansi-term-color-vector
    term-color-blue
    term-color-magenta
    term-color-cyan
-   term-color-white])
+   term-color-white
+   term-color-bright-black
+   term-color-bright-red
+   term-color-bright-green
+   term-color-bright-yellow
+   term-color-bright-blue
+   term-color-bright-magenta
+   term-color-bright-cyan
+   term-color-bright-white])
 
 (defcustom term-default-fg-color nil
   "If non-nil, default color for foreground in Term mode."
@@ -752,54 +760,112 @@ term
   :group 'term)
 
 (defface term-bold
-  '((t :bold t))
+  '((t :inherit ansi-color-bold))
   "Default face to use for bold text."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-underline
-  '((t :underline t))
+  '((t :inherit ansi-color-underline))
   "Default face to use for underlined text."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-black
-  '((t :foreground "black" :background "black"))
+  '((t :inherit ansi-color-black))
   "Face used to render black color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-red
-  '((t :foreground "red3" :background "red3"))
+  '((t :inherit ansi-color-red))
   "Face used to render red color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-green
-  '((t :foreground "green3" :background "green3"))
+  '((t :inherit ansi-color-green))
   "Face used to render green color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-yellow
-  '((t :foreground "yellow3" :background "yellow3"))
+  '((t :inherit ansi-color-yellow))
   "Face used to render yellow color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-blue
-  '((t :foreground "blue2" :background "blue2"))
+  '((t :inherit ansi-color-blue))
   "Face used to render blue color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-magenta
-  '((t :foreground "magenta3" :background "magenta3"))
+  '((t :inherit ansi-color-magenta))
   "Face used to render magenta color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-cyan
-  '((t :foreground "cyan3" :background "cyan3"))
+  '((t :inherit ansi-color-cyan))
   "Face used to render cyan color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
 
 (defface term-color-white
-  '((t :foreground "white" :background "white"))
+  '((t :inherit ansi-color-white))
   "Face used to render white color code."
-  :group 'term)
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-black
+  '((t :inherit ansi-color-bright-black))
+  "Face used to render bright black color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-red
+  '((t :inherit ansi-color-bright-red))
+  "Face used to render bright red color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-green
+  '((t :inherit ansi-color-bright-green))
+  "Face used to render bright green color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-yellow
+  '((t :inherit ansi-color-bright-yellow))
+  "Face used to render bright yellow color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-blue
+  '((t :inherit ansi-color-bright-blue))
+  "Face used to render bright blue color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-magenta
+  '((t :inherit ansi-color-bright-magenta))
+  "Face used to render bright magenta color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-cyan
+  '((t :inherit ansi-color-bright-cyan))
+  "Face used to render bright cyan color code."
+  :group 'term
+  :version "28.1")
+
+(defface term-color-bright-white
+  '((t :inherit ansi-color-bright-white))
+  "Face used to render bright white color code."
+  :group 'term
+  :version "28.1")
 
 (defcustom term-buffer-maximum-size 8192
   "The maximum size in lines for term buffers.
@@ -3223,6 +3289,15 @@ term-reset-terminal
   ;; FIXME: No idea why this is here, it looks wrong.  --Stef
   (setq term-ansi-face-already-done nil))
 
+(defun term--maybe-brighten-color (color bold)
+  "Possibly convert COLOR to its bright variant.
+COLOR is an index into `ansi-term-color-vector'.  If BOLD and
+`ansi-color-bold-is-bright' are non-nil and COLOR is a regular color,
+return the bright version of COLOR; otherwise, return COLOR."
+  (if (and ansi-color-bold-is-bright bold (<= 1 color 8))
+      (+ color 8)
+    color))
+
 ;; New function to deal with ansi colorized output, as you can see you can
 ;; have any bold/underline/fg/bg/reverse combination. -mm
 
@@ -3262,6 +3337,10 @@ term-handle-colors-array
    ((and (>= parameter 30) (<= parameter 37))
     (setq term-ansi-current-color (- parameter 29)))
 
+   ;; Bright foreground
+   ((and (>= parameter 90) (<= parameter 97))
+    (setq term-ansi-current-color (- parameter 81)))
+
    ;; Reset foreground
    ((eq parameter 39)
     (setq term-ansi-current-color 0))
@@ -3270,6 +3349,10 @@ term-handle-colors-array
    ((and (>= parameter 40) (<= parameter 47))
     (setq term-ansi-current-bg-color (- parameter 39)))
 
+   ;; Bright foreground
+   ((and (>= parameter 100) (<= parameter 107))
+    (setq term-ansi-current-bg-color (- parameter 91)))
+
    ;; Reset background
    ((eq parameter 49)
     (setq term-ansi-current-bg-color 0))
@@ -3288,37 +3371,43 @@ term-handle-colors-array
   ;;          term-ansi-current-bg-color)
 
   (unless term-ansi-face-already-done
-    (if term-ansi-current-invisible
-        (let ((color
-               (if term-ansi-current-reverse
-                   (face-foreground
-                    (elt ansi-term-color-vector term-ansi-current-color)
-                    nil 'default)
-                 (face-background
-                  (elt ansi-term-color-vector term-ansi-current-bg-color)
-                  nil 'default))))
-          (setq term-current-face
-                (list :background color
-                      :foreground color))
-          ) ;; No need to bother with anything else if it's invisible.
-      (setq term-current-face
-            (list :foreground
-                  (face-foreground
-                   (elt ansi-term-color-vector term-ansi-current-color)
-                   nil 'default)
-                  :background
-                  (face-background
-                   (elt ansi-term-color-vector term-ansi-current-bg-color)
-                   nil 'default)
-                  :inverse-video term-ansi-current-reverse))
-
-      (when term-ansi-current-bold
+    (let ((current-color (term--maybe-brighten-color
+                          term-ansi-current-color
+                          term-ansi-current-bold))
+          (current-bg-color (term--maybe-brighten-color
+                             term-ansi-current-bg-color
+                             term-ansi-current-bold)))
+      (if term-ansi-current-invisible
+          (let ((color
+                 (if term-ansi-current-reverse
+                     (face-foreground
+                      (elt ansi-term-color-vector current-color)
+                      nil 'default)
+                   (face-background
+                    (elt ansi-term-color-vector current-bg-color)
+                    nil 'default))))
+            (setq term-current-face
+                  (list :background color
+                        :foreground color))
+            ) ;; No need to bother with anything else if it's invisible.
         (setq term-current-face
-              `(,term-current-face :inherit term-bold)))
+              (list :foreground
+                    (face-foreground
+                     (elt ansi-term-color-vector current-color)
+                     nil 'default)
+                    :background
+                    (face-background
+                     (elt ansi-term-color-vector current-bg-color)
+                     nil 'default)
+                    :inverse-video term-ansi-current-reverse))
+
+        (when term-ansi-current-bold
+          (setq term-current-face
+                `(,term-current-face :inherit term-bold)))
 
-      (when term-ansi-current-underline
-        (setq term-current-face
-              `(,term-current-face :inherit term-underline)))))
+        (when term-ansi-current-underline
+          (setq term-current-face
+                `(,term-current-face :inherit term-underline))))))
 
   ;;	(message "Debug %S" term-current-face)
   ;; FIXME: shouldn't we set term-ansi-face-already-done to t here?  --Stef
diff --git a/test/lisp/term-tests.el b/test/lisp/term-tests.el
index 50ac370b5b..96b6d73488 100644
--- a/test/lisp/term-tests.el
+++ b/test/lisp/term-tests.el
@@ -28,6 +28,51 @@
 (defvar term-height)                    ; Number of lines in window.
 (defvar term-width)                     ; Number of columns in window.
 
+(defvar yellow-fg-props
+  `( :foreground ,(face-foreground 'term-color-yellow nil 'default)
+     :background "unspecified-bg" :inverse-video nil))
+(defvar yellow-bg-props
+  `( :foreground "unspecified-fg"
+     :background ,(face-background 'term-color-yellow nil 'default)
+     :inverse-video nil))
+(defvar bright-yellow-fg-props
+  `( :foreground ,(face-foreground 'term-color-bright-yellow nil 'default)
+     :background "unspecified-bg" :inverse-video nil))
+(defvar bright-yellow-bg-props
+  `( :foreground "unspecified-fg"
+     :background ,(face-background 'term-color-bright-yellow nil 'default)
+     :inverse-video nil))
+
+(defvar ansi-test-strings
+  `(("\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-fg-props))
+    ("\e[43mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face yellow-bg-props))
+    ("\e[93mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-fg-props))
+    ("\e[103mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face bright-yellow-bg-props))
+    ("\e[1;33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33;1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[1m\e[33mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))
+    ("\e[33m\e[1mHello World\e[0m"
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,yellow-fg-props :inherit term-bold))
+     ,(propertize "Hello World" 'font-lock-face
+                  `(,bright-yellow-fg-props :inherit term-bold)))))
+
 (defun term-test-screen-from-input (width height input &optional return-var)
   (with-temp-buffer
     (term-mode)
@@ -48,7 +93,7 @@ term-test-screen-from-input
                 (mapc (lambda (input) (term-emulate-terminal proc input)) input)
               (term-emulate-terminal proc input))
       (if return-var (buffer-local-value return-var (current-buffer))
-        (buffer-substring-no-properties (point-min) (point-max))))))
+        (buffer-substring (point-min) (point-max))))))
 
 (ert-deftest term-simple-lines ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
@@ -77,6 +122,24 @@ term-line-wrap
            (term-test-screen-from-input 40 12 (let ((str (make-string 30 ?a)))
                                                 (list str str))))))
 
+(ert-deftest term-colors ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (pcase-dolist (`(,str ,expected) ansi-test-strings)
+    (let ((result (term-test-screen-from-input 40 12 str)))
+      (should (equal result expected))
+      (should (equal (text-properties-at 0 result)
+                     (text-properties-at 0 expected))))))
+
+(ert-deftest term-colors-bold-is-bright ()
+  (skip-unless (not (memq system-type '(windows-nt ms-dos))))
+  (let ((ansi-color-bold-is-bright t))
+    (pcase-dolist (`(,str ,expected ,bright-expected) ansi-test-strings)
+      (let ((expected (or bright-expected expected))
+            (result (term-test-screen-from-input 40 12 str)))
+        (should (equal result expected))
+        (should (equal (text-properties-at 0 result)
+                       (text-properties-at 0 expected)))))))
+
 (ert-deftest term-cursor-movement ()
   (skip-unless (not (memq system-type '(windows-nt ms-dos))))
   ;; Absolute positioning.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH v4] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-09-23  1:47                             ` bug#50179: [PATCH v4] " Jim Porter
@ 2021-09-23 20:58                               ` Lars Ingebrigtsen
  2021-09-23 21:21                                 ` Jim Porter
  0 siblings, 1 reply; 20+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-23 20:58 UTC (permalink / raw)
  To: Jim Porter; +Cc: 50179

Jim Porter <jporterbugs@gmail.com> writes:

> Ok, I've done this here, and all the other places as well. I also
> obsoleted the old `ansi-color-map' and friends instead of deleting
> them for the same reason. Hopefully all that's right; I tried to make
> sure that they warn the user if they use them, but that warnings don't
> show up when building Emacs.

Thanks; looks good to me.  There was a typo in the man.el part of the
patch, but I think I fixed it -- can you have a look and see whether I
did the correct thing?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#50179: [PATCH v4] Add support for "bright" ANSI colors to ansi-color and term-mode
  2021-09-23 20:58                               ` Lars Ingebrigtsen
@ 2021-09-23 21:21                                 ` Jim Porter
  0 siblings, 0 replies; 20+ messages in thread
From: Jim Porter @ 2021-09-23 21:21 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 50179

On 9/23/2021 1:58 PM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
> 
>> Ok, I've done this here, and all the other places as well. I also
>> obsoleted the old `ansi-color-map' and friends instead of deleting
>> them for the same reason. Hopefully all that's right; I tried to make
>> sure that they warn the user if they use them, but that warnings don't
>> show up when building Emacs.
> 
> Thanks; looks good to me.  There was a typo in the man.el part of the
> patch, but I think I fixed it -- can you have a look and see whether I
> did the correct thing?

That looks correct to me. Thanks for the fix! (I switched back and forth 
between implementations there a few times, and I guess when I made the 
patch I was midway between changing it back.)





^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-09-23 21:21 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24  4:02 bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode Jim Porter
2021-08-24 12:07 ` Eli Zaretskii
2021-08-24 17:38   ` Jim Porter
2021-08-24 17:59     ` Eli Zaretskii
2021-08-24 18:59       ` Jim Porter
2021-08-24 22:53         ` Jim Porter
2021-08-25 12:04           ` Lars Ingebrigtsen
2021-08-25 16:41             ` Jim Porter
2021-08-25 16:46               ` Lars Ingebrigtsen
2021-08-25 16:54                 ` Eli Zaretskii
2021-08-26 13:23                   ` Lars Ingebrigtsen
2021-09-18 18:58                     ` bug#50179: [UPDATED PATCH] " Jim Porter
2021-09-19 14:45                       ` Lars Ingebrigtsen
2021-09-22 19:39                         ` bug#50179: [WIP PATCH v3] " Jim Porter
2021-09-22 19:49                           ` Lars Ingebrigtsen
2021-09-23  1:47                             ` bug#50179: [PATCH v4] " Jim Porter
2021-09-23 20:58                               ` Lars Ingebrigtsen
2021-09-23 21:21                                 ` Jim Porter
2021-08-25  7:06       ` bug#50179: [PATCH] " Kévin Le Gouguec
2021-08-25 11:57         ` Eli Zaretskii

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