[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Inclusion of naquadah-theme

From: Chong Yidong
Subject: Re: Inclusion of naquadah-theme
Date: Thu, 28 Jun 2012 15:36:03 +0800
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1.50 (gnu/linux)

Julien Danjou <address@hidden> writes:

> What do you think of the following patch?

> +     * term.el (term-handle-colors-array): Use a set of new faces to
> +     color the terminal. Also uses :inverse-video property.

For consistency with the rest of Emacs, please use two spaces after each

Please mark term-default-fg-color and term-default-bg-color as
deprecated, instead of deleting them outright.  Something like this:

(defcustom term-default-fg-color
  (face-foreground term-current-face)
  "If non-nil, default color for foreground in Term mode."
  :group 'term
  :type 'string)

(defface term-face
  (if term-default-fg-color
      `((t :foreground ,term-default-fg-color :inherit default))
    '((t :inherit default)))
  "Default face to use in Term mode."
  :group 'term)

and similarly for term-default-bg-color.  It is not necessary to handle
term-bold-attribute this way since it is just a defvar.

Please add a ChangeLog entry for each changed variable,
e.g. term-bold-attribute should say "Variable deleted".  Also, write a
NEWS item noting the deprecation of term-default-*-color and the
introduction of the new faces.

The rest of the patch looks fine.  Go ahead and commit once you have
made the above changes, and thanks.

>> Having defuns in a theme file is obnoxious.  Could you explain what
>> these functions are needed for?  If there is missing functionality, we
>> should incorporate that directly into Emacs; they should not be
>> naquadah-specific.
> It's a system we built to define faces with symbols and then expand
> into a real color value, depending on the terminal capability. This
> allows to define a face once with generic color and expand it:

I'm not sure I understand.  If you specify a face with a color like
"#252A2B", doesn't Emacs display the closest matching color on terminals
with limited colors?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]