Subject: bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode

From: Eli Zaretskii <[email-masked]>
Date: Tue, 24 Aug 2021 15:07:01 +0300

> From: Jim Porter <[email-masked]>
> 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?