Re: Inclusion of naquadah-theme

[Chong Yidong]

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

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?