unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Jim Porter <jporterbugs@gmail.com>
Cc: 50179@debbugs.gnu.org
Subject: bug#50179: [PATCH] Add support for "bright" ANSI colors to ansi-color and term-mode
Date: Tue, 24 Aug 2021 15:07:01 +0300	[thread overview]
Message-ID: <838s0rvyfu.fsf@gnu.org> (raw)
In-Reply-To: <CANh=_JGtOM=1W7fg2EANrdMdGYdfAGjb21kyCb6avJ+gDbz1Dg@mail.gmail.com> (message from Jim Porter on Mon, 23 Aug 2021 21:02:46 -0700)

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





  reply	other threads:[~2021-08-24 12:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=838s0rvyfu.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=50179@debbugs.gnu.org \
    --cc=jporterbugs@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).