* Inclusion of naquadah-theme @ 2012-06-25 10:13 Julien Danjou 2012-06-25 13:49 ` Chong Yidong 0 siblings, 1 reply; 7+ messages in thread From: Julien Danjou @ 2012-06-25 10:13 UTC (permalink / raw) To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 387 bytes --] Hi, I'd like to distribute my color theme more widely and therefore include it in Emacs distribution. I don't know what the inclusion constraints are, so I'm sending this here for review. http://git.naquadah.org/?p=naquadah-theme.git;a=blob;f=naquadah-theme.el;h=ea9e1b1aba1bac6631e7577e5fc4217ad699d779;hb=HEAD If that's ok I'll merge in trunk. -- Julien [-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Inclusion of naquadah-theme 2012-06-25 10:13 Inclusion of naquadah-theme Julien Danjou @ 2012-06-25 13:49 ` Chong Yidong 2012-06-26 10:48 ` Julien Danjou 0 siblings, 1 reply; 7+ messages in thread From: Chong Yidong @ 2012-06-25 13:49 UTC (permalink / raw) To: emacs-devel Julien Danjou <julien@danjou.info> writes: > I'd like to distribute my color theme more widely and therefore include > it in Emacs distribution. I don't know what the inclusion constraints > are, so I'm sending this here for review. > > http://git.naquadah.org/?p=naquadah-theme.git;a=blob;f=naquadah-theme.el;h=ea9e1b1aba1bac6631e7577e5fc4217ad699d779;hb=HEAD Please have a better description in the first line than "A color theme", since this serves as the accompanying text in the M-x customize-themes buffer. Likewise the docstring to deftheme, which is shown with the describe-theme command. Do not set ansi-term-color-vector directly with setq. If you find the need to customize it, please fix it right: submit a patch to term.el that converts this to a defcustom, then use custom-theme-set-variables. 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Inclusion of naquadah-theme 2012-06-25 13:49 ` Chong Yidong @ 2012-06-26 10:48 ` Julien Danjou 2012-06-28 7:36 ` Chong Yidong 0 siblings, 1 reply; 7+ messages in thread From: Julien Danjou @ 2012-06-26 10:48 UTC (permalink / raw) To: Chong Yidong; +Cc: emacs-devel [-- Attachment #1.1: Type: text/plain, Size: 628 bytes --] On Mon, Jun 25 2012, Chong Yidong wrote: > Please have a better description in the first line than "A color theme", > since this serves as the accompanying text in the M-x customize-themes > buffer. Likewise the docstring to deftheme, which is shown with the > describe-theme command. Sure, I fixed this two. But it's kinda hard to describe a theme actually. :) > Do not set ansi-term-color-vector directly with setq. If you find the > need to customize it, please fix it right: submit a patch to term.el > that converts this to a defcustom, then use custom-theme-set-variables. What do you think of the following patch? [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1.2: 0001-term-use-faces-for-color-handling.patch --] [-- Type: text/x-diff, Size: 8319 bytes --] From 15acb6ecb90cca322109344521379d843ce53c15 Mon Sep 17 00:00:00 2001 From: Julien Danjou <julien@danjou.info> Date: Tue, 26 Jun 2012 12:39:41 +0200 Subject: [PATCH] term: use faces for color handling Signed-off-by: Julien Danjou <julien@danjou.info> --- lisp/ChangeLog | 5 +++ lisp/term.el | 135 +++++++++++++++++++++++++++++++------------------------- 2 files changed, 81 insertions(+), 59 deletions(-) diff --git a/lisp/ChangeLog b/lisp/ChangeLog index 69fee94..9680fb3 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,3 +1,8 @@ +2012-06-26 Julien Danjou <julien@danjou.info> + + * term.el (term-handle-colors-array): Use a set of new faces to + color the terminal. Also uses :inverse-video property. + 2012-06-26 Martin Rudalics <rudalics@gmx.at> * calendar/calendar.el (calendar-exit): Don't try to delete or diff --git a/lisp/term.el b/lisp/term.el index 7461d74..d11fd57 100644 --- a/lisp/term.el +++ b/lisp/term.el @@ -108,11 +108,6 @@ ;; ;; Blink, is not supported. Currently it's mapped as bold. ;; -;; Important caveat: -;; ----------------- -;; if you want custom colors in term.el redefine term-default-fg-color -;; and term-default-bg-color BEFORE loading it. -;; ;; ---------------------------------------- ;; ;; If you'd like to check out my complete configuration, you can download @@ -459,7 +454,7 @@ state 4: term-terminal-parameter contains pending output.") "A queue of strings whose echo we want suppressed.") (defvar term-terminal-parameter) (defvar term-terminal-previous-parameter) -(defvar term-current-face 'default) +(defvar term-current-face 'term-face) (defvar term-scroll-start 0 "Top-most line (inclusive) of scrolling region.") (defvar term-scroll-end) ; Number of line (zero-based) after scrolling region. (defvar term-pager-count nil @@ -796,27 +791,71 @@ Buffer local variable.") (defvar term-terminal-previous-parameter-4 -1) ;;; faces -mm +(defvar ansi-term-color-vector + [term-face + term-color-black + term-color-red + term-color-green + term-color-yellow + term-color-blue + term-color-magenta + term-color-cyan + term-color-white]) + +(defface term-face + '((t :inherit default)) + "Default face to use in term." + :group 'term) -(defcustom term-default-fg-color - ;; FIXME: This depends on the current frame, so depending on when - ;; it's loaded, the result may be different. - (face-foreground term-current-face) - "Default color for foreground in `term'." - :group 'term - :type 'string) +(defface term-bold + '((t :bold t)) + "Default face to use for bold text." + :group 'term) -(defcustom term-default-bg-color - ;; FIXME: This depends on the current frame, so depending on when - ;; it's loaded, the result may be different. - (face-background term-current-face) - "Default color for background in `term'." - :group 'term - :type 'string) +(defface term-underline + '((t :underline t)) + "Default face to use for underlined text." + :group 'term) -;; Use the same colors that xterm uses, see `xterm-standard-colors'. -(defvar ansi-term-color-vector - [unspecified "black" "red3" "green3" "yellow3" "blue2" - "magenta3" "cyan3" "white"]) +(defface term-color-black + '((t :foreground "black" :background "black")) + "Face used to render black color code." + :group 'term) + +(defface term-color-red + '((t :foreground "red3" :background "red3")) + "Face used to render red color code." + :group 'term) + +(defface term-color-green + '((t :foreground "green3" :background "green3")) + "Face used to render green color code." + :group 'term) + +(defface term-color-yellow + '((t :foreground "yellow3" :background "yellow3")) + "Face used to render yellow color code." + :group 'term) + +(defface term-color-blue + '((t :foreground "blue2" :background "blue2")) + "Face used to render blue color code." + :group 'term) + +(defface term-color-magenta + '((t :foreground "magenta3" :background "magenta3")) + "Face used to render magenta color code." + :group 'term) + +(defface term-color-cyan + '((t :foreground "cyan3" :background "cyan3")) + "Face used to render cyan color code." + :group 'term) + +(defface term-color-white + '((t :foreground "white" :background "white")) + "Face used to render white color code." + :group 'term) ;; Inspiration came from comint.el -mm (defcustom term-buffer-maximum-size 2048 @@ -951,11 +990,7 @@ is buffer-local." dt)) (defun term-ansi-reset () - (setq term-current-face (nconc - (if term-default-bg-color - (list :background term-default-bg-color)) - (if term-default-fg-color - (list :foreground term-default-fg-color)))) + (setq term-current-face 'term-face) (setq term-ansi-current-underline nil) (setq term-ansi-current-bold nil) (setq term-ansi-current-reverse nil) @@ -3088,10 +3123,6 @@ See `term-prompt-regexp'." ;; New function to deal with ansi colorized output, as you can see you can ;; have any bold/underline/fg/bg/reverse combination. -mm -(defvar term-bold-attribute '(:weight bold) - "Attribute to use for the bold terminal attribute. -Set it to nil to disable bold.") - (defun term-handle-colors-array (parameter) (cond @@ -3153,46 +3184,32 @@ Set it to nil to disable bold.") ;; term-ansi-current-color ;; term-ansi-current-bg-color) - (unless term-ansi-face-already-done (if term-ansi-current-invisible (let ((color (if term-ansi-current-reverse - (if (= term-ansi-current-color 0) - term-default-fg-color - (elt ansi-term-color-vector term-ansi-current-color)) - (if (= term-ansi-current-bg-color 0) - term-default-bg-color - (elt ansi-term-color-vector term-ansi-current-bg-color))))) + (face-foreground + (elt ansi-term-color-vector term-ansi-current-color)) + (face-background + (elt ansi-term-color-vector term-ansi-current-bg-color))))) (setq term-current-face (list :background color :foreground color)) ) ;; No need to bother with anything else if it's invisible. - (setq term-current-face - (if term-ansi-current-reverse - (if (= term-ansi-current-color 0) - (list :background term-default-fg-color - :foreground term-default-bg-color) - (list :background - (elt ansi-term-color-vector term-ansi-current-color) - :foreground - (elt ansi-term-color-vector term-ansi-current-bg-color))) - - (if (= term-ansi-current-color 0) - (list :foreground term-default-fg-color - :background term-default-bg-color) - (list :foreground - (elt ansi-term-color-vector term-ansi-current-color) - :background - (elt ansi-term-color-vector term-ansi-current-bg-color))))) + (list :foreground + (face-foreground (elt ansi-term-color-vector term-ansi-current-color)) + :background + (face-background (elt ansi-term-color-vector term-ansi-current-bg-color)) + :inverse-video term-ansi-current-reverse)) (when term-ansi-current-bold (setq term-current-face - (append term-bold-attribute term-current-face))) + (list* term-current-face :inherit 'term-bold))) + (when term-ansi-current-underline (setq term-current-face - (list* :underline t term-current-face))))) + (list* 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 -- 1.7.10 [-- Attachment #1.3: Type: text/plain, Size: 1150 bytes --] > 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: (naquadah-simple-face-to-multiple '(default (:background background :foreground aluminium-1)))) => (default ((((class color) (min-colors 65535)) (:background "#252A2B" :foreground "#eeeeec")) (((class color) (min-colors 256)) (:background "color-234" :foreground "color-255")) (((class color) (min-colors 88)) (:background "color-80" :foreground "color-87")) (t (:background "black" :foreground "white")))) This way we just have to define the color "aluminium-1" to be a different color depending on the numbre of color available, and then define the font. The rest is expanded by this function. -- Julien [-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Inclusion of naquadah-theme 2012-06-26 10:48 ` Julien Danjou @ 2012-06-28 7:36 ` Chong Yidong 2012-06-28 10:42 ` Julien Danjou 0 siblings, 1 reply; 7+ messages in thread From: Chong Yidong @ 2012-06-28 7:36 UTC (permalink / raw) To: emacs-devel Julien Danjou <julien@danjou.info> 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? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Inclusion of naquadah-theme 2012-06-28 7:36 ` Chong Yidong @ 2012-06-28 10:42 ` Julien Danjou 2012-06-30 2:15 ` Chong Yidong 0 siblings, 1 reply; 7+ messages in thread From: Julien Danjou @ 2012-06-28 10:42 UTC (permalink / raw) To: Chong Yidong; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 677 bytes --] On Thu, Jun 28 2012, Chong Yidong wrote: […] > The rest of the patch looks fine. Go ahead and commit once you have > made the above changes, and thanks. Thanks for your review! I've commited with the requested changes. If anything bad comes up with this, don't hesitate to beat me, I'll fix. :) > 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? It does try, but the result is not always good. Some colors are correctly picked up as closest/best choice, others aren't fine. We're doing a better job manually. -- Julien [-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Inclusion of naquadah-theme 2012-06-28 10:42 ` Julien Danjou @ 2012-06-30 2:15 ` Chong Yidong 2012-07-02 21:02 ` Julien Danjou 0 siblings, 1 reply; 7+ messages in thread From: Chong Yidong @ 2012-06-30 2:15 UTC (permalink / raw) To: emacs-devel Julien Danjou <julien@danjou.info> writes: >> 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? > > It does try, but the result is not always good. Some colors are > correctly picked up as closest/best choice, others aren't fine. We're > doing a better job manually. Well, first of all there are really only two cases that need to be handled: terminals that can handle #rrggbb, and 16-color terminals. In my experience, 256-color xterms give a rather good approximation of the desired colors automatically, without manual color placement. As for matching color names to faces, the approach that we've used in tango-theme.el and others is to do something like this: (let ((class '((class color) (min-colors 89))) ;; Tango palette colors. (butter-1 "#fce94f") ...) (custom-theme-set-faces 'tango-dark ... `(cursor ((,class (:background ,butter-1)))) Apart from handling multiple terminals, naquadah-simple-face-to-multiple aims to do the same thing, right? I'm guessing the reason is that you feel that having explicit functions is more structured/less ad-hoc than the above let-form. If that's the case, maybe the solution is to add a new macro to custom.el, which does something like this: (custom-theme-set-faces-with-color-names THEME TERMINALS COLORS FACES...) (custom-theme-set-faces-with-color-names 'tango-dark ((term-1 ((class color) (min-colors 4096))) (term-2 ((class color) (min-colors 16)))) ((butter (term-1 "#fce94f") (term-2 "yellow"))) (cursor :background butter) ...) WDYT? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Inclusion of naquadah-theme 2012-06-30 2:15 ` Chong Yidong @ 2012-07-02 21:02 ` Julien Danjou 0 siblings, 0 replies; 7+ messages in thread From: Julien Danjou @ 2012-07-02 21:02 UTC (permalink / raw) To: Chong Yidong; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 1830 bytes --] On Sat, Jun 30 2012, Chong Yidong wrote: > Well, first of all there are really only two cases that need to be > handled: terminals that can handle #rrggbb, and 16-color terminals. In > my experience, 256-color xterms give a rather good approximation of the > desired colors automatically, without manual color placement. Clearly that fails with naquadah-theme. I wish I had more time to try to fix this. > As for matching color names to faces, the approach that we've used in > tango-theme.el and others is to do something like this: > > (let ((class '((class color) (min-colors 89))) > ;; Tango palette colors. > (butter-1 "#fce94f") > ...) > > (custom-theme-set-faces > 'tango-dark > ... > `(cursor ((,class (:background ,butter-1)))) > > Apart from handling multiple terminals, naquadah-simple-face-to-multiple > aims to do the same thing, right? Yes, that's exactly what we're doing. > I'm guessing the reason is that you feel that having explicit > functions is more structured/less ad-hoc than the above let-form. Not, that's because I ignored the syntax you used in tango actually. > If that's the case, maybe the solution is to add a new macro to > custom.el, which does something like this: > > (custom-theme-set-faces-with-color-names THEME TERMINALS COLORS FACES...) > > (custom-theme-set-faces-with-color-names > 'tango-dark > ((term-1 ((class color) (min-colors 4096))) > (term-2 ((class color) (min-colors 16)))) > ((butter (term-1 "#fce94f") (term-2 "yellow"))) > (cursor :background butter) > ...) > > WDYT? That would be really handy, indeed. Do you propose to do the patch yourself, or should it? -- /* Julien Danjou ╭ Free Software hacker & freelance ╰ http://julien.danjou.info */ [-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-07-02 21:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-06-25 10:13 Inclusion of naquadah-theme Julien Danjou 2012-06-25 13:49 ` Chong Yidong 2012-06-26 10:48 ` Julien Danjou 2012-06-28 7:36 ` Chong Yidong 2012-06-28 10:42 ` Julien Danjou 2012-06-30 2:15 ` Chong Yidong 2012-07-02 21:02 ` Julien Danjou
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.