[Emacs-diffs] /srv/bzr/emacs/trunk r104918: Fix how custom themes handle faces, so the multi-tty/multi-frame case works.

[Chong Yidong]

------------------------------------------------------------
revno: 104918
committer: Chong Yidong <address@hidden>
branch nick: trunk
timestamp: Sun 2011-07-03 18:16:07 -0400
message:
  Fix how custom themes handle faces, so the multi-tty/multi-frame case works.
  
  * lisp/custom.el (custom-push-theme): Don't record faces in `changed'
  theme; this doesn't work correctly for per-frame face settings.
  (disable-theme): Use face-set-after-frame-default to reset faces.
  (custom--frame-color-default): New function.
  
  * lisp/frame.el (frame-background-mode, frame-set-background-mode):
  Moved from faces.el.
  (frame-default-terminal-background): New function.
  
  * src/xfaces.c (Finternal_merge_in_global_face): Modify the foreground
  and background color parameters if they have been changed.
modified:
  lisp/ChangeLog
  lisp/custom.el
  lisp/faces.el
  lisp/frame.el
  src/ChangeLog
  src/xfaces.c

=== modified file 'lisp/ChangeLog'
--- a/lisp/ChangeLog    2011-07-03 22:14:08 +0000
+++ b/lisp/ChangeLog    2011-07-03 22:16:07 +0000
@@ -1,3 +1,14 @@
+2011-07-03  Chong Yidong  <address@hidden>
+
+       * frame.el (frame-background-mode, frame-set-background-mode):
+       Moved from faces.el.
+       (frame-default-terminal-background): New function.
+
+       * custom.el (custom-push-theme): Don't record faces in `changed'
+       theme; this doesn't work correctly for per-frame face settings.
+       (disable-theme): Use face-set-after-frame-default to reset faces.
+       (custom--frame-color-default): New function.
+
 2011-07-03  Lars Magne Ingebrigtsen  <address@hidden>
 
        * dired.el (dired-flagging-regexp): Removed unused variable

=== modified file 'lisp/custom.el'
--- a/lisp/custom.el    2011-07-02 23:52:35 +0000
+++ b/lisp/custom.el    2011-07-03 22:16:07 +0000
@@ -855,25 +855,18 @@
      ;; Add a new setting:
      (t
       (unless old
-       ;; If the user changed the value outside of Customize, we
-       ;; first save the current value to a fake theme, `changed'.
-       ;; This ensures that the user-set value comes back if the
-       ;; theme is later disabled.
-       (cond ((and (eq prop 'theme-value)
-                   (boundp symbol))
-              (let ((sv  (get symbol 'standard-value))
-                    (val (symbol-value symbol)))
-                (unless (and sv (equal (eval (car sv)) val))
-                  (setq old `((changed ,(custom-quote val)))))))
-             ((and (facep symbol)
-                   (not (face-attr-match-p
-                         symbol
-                         (custom-fix-face-spec
-                          (face-spec-choose
-                           (get symbol 'face-defface-spec))))))
-              (setq old `((changed
-                           (,(append '(t) (custom-face-attributes-get
-                                           symbol nil)))))))))
+       ;; If the user changed a variable outside of Customize, save
+       ;; the value to a fake theme, `changed'.  If the theme is
+       ;; later disabled, we use this to bring back the old value.
+       ;;
+       ;; For faces, we just use `face-new-frame-defaults' to
+       ;; recompute when the theme is disabled.
+       (when (and (eq prop 'theme-value)
+                  (boundp symbol))
+         (let ((sv  (get symbol 'standard-value))
+               (val (symbol-value symbol)))
+           (unless (and sv (equal (eval (car sv)) val))
+             (setq old `((changed ,(custom-quote val))))))))
       (put symbol prop (cons (list theme value) old))
       (put theme 'theme-settings
           (cons (list prop symbol theme value) theme-settings))))))
@@ -1356,11 +1349,33 @@
            ;; If the face spec specified by this theme is in the
            ;; saved-face property, reset that property.
            (when (equal (nth 3 s) (get symbol 'saved-face))
-             (put symbol 'saved-face (and val (cadr (car val)))))
-           (custom-theme-recalc-face symbol)))))
+             (put symbol 'saved-face (and val (cadr (car val)))))))))
+      ;; Recompute faces on all frames.
+      (dolist (frame (frame-list))
+       ;; We must reset the fg and bg color frame parameters, or
+       ;; `face-set-after-frame-default' will use the existing
+       ;; parameters, which could be from the disabled theme.
+       (set-frame-parameter frame 'background-color
+                            (custom--frame-color-default
+                             frame :background "background" "Background"
+                             "unspecified-bg" "white"))
+       (set-frame-parameter frame 'foreground-color
+                            (custom--frame-color-default
+                             frame :foreground "foreground" "Foreground"
+                             "unspecified-fg" "black"))
+       (face-set-after-frame-default frame))
       (setq custom-enabled-themes
            (delq theme custom-enabled-themes)))))
 
+(defun custom--frame-color-default (frame attribute resource-attr 
resource-class
+                                         tty-default x-default)
+  (let ((col (face-attribute 'default attribute t)))
+    (cond
+     ((and col (not (eq col 'unspecified))) col)
+     ((null (window-system frame)) tty-default)
+     ((setq col (x-get-resource resource-attr resource-class)) col)
+     (t x-default))))
+
 (defun custom-variable-theme-value (variable)
   "Return (list VALUE) indicating the custom theme value of VARIABLE.
 That is to say, it specifies what the value should be according to

=== modified file 'lisp/faces.el'
--- a/lisp/faces.el     2011-06-27 00:29:55 +0000
+++ b/lisp/faces.el     2011-07-03 22:16:07 +0000
@@ -1821,109 +1821,6 @@
           (cond ((memq 'background-color face) (cdr (memq 'background-color 
face)))
                 ((memq ':background face) (cadr (memq ':background face)))))
          (t nil))))                    ; Invalid face value.
-
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;;; Background mode.
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-
-(defcustom frame-background-mode nil
-  "The brightness of the background.
-Set this to the symbol `dark' if your background color is dark,
-`light' if your background is light, or nil (automatic by default)
-if you want Emacs to examine the brightness for you.  Don't set this
-variable with `setq'; this won't have the expected effect."
-  :group 'faces
-  :set #'(lambda (var value)
-          (set-default var value)
-          (mapc 'frame-set-background-mode (frame-list)))
-  :initialize 'custom-initialize-changed
-  :type '(choice (const dark)
-                (const light)
-                (const :tag "automatic" nil)))
-
-
-(declare-function x-get-resource "frame.c"
-                 (attribute class &optional component subclass))
-
-(defvar inhibit-frame-set-background-mode nil)
-
-(defun frame-set-background-mode (frame &optional keep-face-specs)
-  "Set up display-dependent faces on FRAME.
-Display-dependent faces are those which have different definitions
-according to the `background-mode' and `display-type' frame parameters.
-
-If optional arg KEEP-FACE-SPECS is non-nil, don't recalculate
-face specs for the new background mode."
-  (unless inhibit-frame-set-background-mode
-    (let* ((bg-resource
-           (and (window-system frame)
-                (x-get-resource "backgroundMode" "BackgroundMode")))
-          (bg-color (frame-parameter frame 'background-color))
-          (terminal-bg-mode (terminal-parameter frame 'background-mode))
-          (tty-type (tty-type frame))
-          (default-bg-mode
-            (if (or (window-system frame)
-                    (and tty-type
-                         (string-match "^\\(xterm\\|\\rxvt\\|dtterm\\|eterm\\)"
-                                       tty-type)))
-                'light
-              'dark))
-          (non-default-bg-mode (if (eq default-bg-mode 'light) 'dark 'light))
-          (bg-mode
-           (cond (frame-background-mode)
-                 (bg-resource (intern (downcase bg-resource)))
-                 (terminal-bg-mode)
-                 ((equal bg-color "unspecified-fg") ; inverted colors
-                  non-default-bg-mode)
-                 ((not (color-values bg-color frame))
-                  default-bg-mode)
-                 ((>= (apply '+ (color-values bg-color frame))
-                      ;; Just looking at the screen, colors whose
-                      ;; values add up to .6 of the white total
-                      ;; still look dark to me.
-                      (* (apply '+ (color-values "white" frame)) .6))
-                  'light)
-                 (t 'dark)))
-          (display-type
-           (cond ((null (window-system frame))
-                  (if (tty-display-color-p frame) 'color 'mono))
-                 ((display-color-p frame)
-                  'color)
-                 ((x-display-grayscale-p frame)
-                  'grayscale)
-                 (t 'mono)))
-          (old-bg-mode
-           (frame-parameter frame 'background-mode))
-          (old-display-type
-           (frame-parameter frame 'display-type)))
-
-      (unless (and (eq bg-mode old-bg-mode) (eq display-type old-display-type))
-       (let ((locally-modified-faces nil)
-             ;; Prevent face-spec-recalc from calling this function
-             ;; again, resulting in a loop (bug#911).
-             (inhibit-frame-set-background-mode t)
-             (params (list (cons 'background-mode bg-mode)
-                           (cons 'display-type display-type))))
-         (if keep-face-specs
-             (modify-frame-parameters frame params)
-           ;; If we are recomputing face specs, first collect a list
-           ;; of faces that don't match their face-specs.  These are
-           ;; the faces modified on FRAME, and we avoid changing them
-           ;; below.  Use a negative list to avoid consing (we assume
-           ;; most faces are unmodified).
-           (dolist (face (face-list))
-             (and (not (get face 'face-override-spec))
-                  (not (face-spec-match-p face
-                                          (face-user-default-spec face)
-                                          (selected-frame)))
-                  (push face locally-modified-faces)))
-           ;; Now change to the new frame parameters
-           (modify-frame-parameters frame params)
-           ;; For all unmodified named faces, choose face specs
-           ;; matching the new frame parameters.
-           (dolist (face (face-list))
-             (unless (memq face locally-modified-faces)
-               (face-spec-recalc face frame)))))))))
 
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -2020,7 +1917,8 @@
 Finally, apply any relevant face attributes found amongst the
 frame parameters in PARAMETERS."
   (let ((window-system-p (memq (window-system frame) '(x w32))))
-    (dolist (face (nreverse (face-list))) ;Why reverse?  --Stef
+    ;; The `reverse' is so that `default' goes first.
+    (dolist (face (nreverse (face-list)))
       (condition-case ()
          (progn
            ;; Initialize faces from face spec and custom theme.

=== modified file 'lisp/frame.el'
--- a/lisp/frame.el     2011-06-22 07:12:18 +0000
+++ b/lisp/frame.el     2011-07-03 22:16:07 +0000
@@ -847,6 +847,116 @@
     (if frame
        (select-frame-set-input-focus frame)
       (error "There is no frame named `%s'" name))))
+
+
+;;;; Background mode.
+
+(defcustom frame-background-mode nil
+  "The brightness of the background.
+Set this to the symbol `dark' if your background color is dark,
+`light' if your background is light, or nil (automatic by default)
+if you want Emacs to examine the brightness for you.  Don't set this
+variable with `setq'; this won't have the expected effect."
+  :group 'faces
+  :set #'(lambda (var value)
+          (set-default var value)
+          (mapc 'frame-set-background-mode (frame-list)))
+  :initialize 'custom-initialize-changed
+  :type '(choice (const dark)
+                (const light)
+                (const :tag "automatic" nil)))
+
+(declare-function x-get-resource "frame.c"
+                 (attribute class &optional component subclass))
+
+(defvar inhibit-frame-set-background-mode nil)
+
+(defun frame-set-background-mode (frame &optional keep-face-specs)
+  "Set up display-dependent faces on FRAME.
+Display-dependent faces are those which have different definitions
+according to the `background-mode' and `display-type' frame parameters.
+
+If optional arg KEEP-FACE-SPECS is non-nil, don't recalculate
+face specs for the new background mode."
+  (unless inhibit-frame-set-background-mode
+    (let* ((frame-default-bg-mode (frame-terminal-default-bg-mode frame))
+          (bg-color (frame-parameter frame 'background-color))
+          (tty-type (tty-type frame))
+          (default-bg-mode
+            (if (or (window-system frame)
+                    (and tty-type
+                         (string-match "^\\(xterm\\|\\rxvt\\|dtterm\\|eterm\\)"
+                                       tty-type)))
+                'light
+              'dark))
+          (non-default-bg-mode (if (eq default-bg-mode 'light) 'dark 'light))
+          (bg-mode
+           (cond (frame-default-bg-mode)
+                 ((equal bg-color "unspecified-fg") ; inverted colors
+                  non-default-bg-mode)
+                 ((not (color-values bg-color frame))
+                  default-bg-mode)
+                 ((>= (apply '+ (color-values bg-color frame))
+                      ;; Just looking at the screen, colors whose
+                      ;; values add up to .6 of the white total
+                      ;; still look dark to me.
+                      (* (apply '+ (color-values "white" frame)) .6))
+                  'light)
+                 (t 'dark)))
+          (display-type
+           (cond ((null (window-system frame))
+                  (if (tty-display-color-p frame) 'color 'mono))
+                 ((display-color-p frame)
+                  'color)
+                 ((x-display-grayscale-p frame)
+                  'grayscale)
+                 (t 'mono)))
+          (old-bg-mode
+           (frame-parameter frame 'background-mode))
+          (old-display-type
+           (frame-parameter frame 'display-type)))
+
+      (unless (and (eq bg-mode old-bg-mode) (eq display-type old-display-type))
+       (let ((locally-modified-faces nil)
+             ;; Prevent face-spec-recalc from calling this function
+             ;; again, resulting in a loop (bug#911).
+             (inhibit-frame-set-background-mode t)
+             (params (list (cons 'background-mode bg-mode)
+                           (cons 'display-type display-type))))
+         (if keep-face-specs
+             (modify-frame-parameters frame params)
+           ;; If we are recomputing face specs, first collect a list
+           ;; of faces that don't match their face-specs.  These are
+           ;; the faces modified on FRAME, and we avoid changing them
+           ;; below.  Use a negative list to avoid consing (we assume
+           ;; most faces are unmodified).
+           (dolist (face (face-list))
+             (and (not (get face 'face-override-spec))
+                  (not (face-spec-match-p face
+                                          (face-user-default-spec face)
+                                          (selected-frame)))
+                  (push face locally-modified-faces)))
+           ;; Now change to the new frame parameters
+           (modify-frame-parameters frame params)
+           ;; For all unmodified named faces, choose face specs
+           ;; matching the new frame parameters.
+           (dolist (face (face-list))
+             (unless (memq face locally-modified-faces)
+               (face-spec-recalc face frame)))))))))
+
+(defun frame-terminal-default-bg-mode (frame)
+  "Return the default background mode of FRAME.
+This checks the `frame-background-mode' variable, the X resource
+named \"backgroundMode\" (if FRAME is an X frame), and finally
+the `background-mode' terminal parameter."
+  (or frame-background-mode
+      (let ((bg-resource
+            (and (window-system frame)
+                 (x-get-resource "backgroundMode" "BackgroundMode"))))
+       (if bg-resource
+           (intern (downcase bg-resource))))
+      (terminal-parameter frame 'background-mode)))
+
 
 ;;;; Frame configurations
 

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2011-07-03 13:02:43 +0000
+++ b/src/ChangeLog     2011-07-03 22:16:07 +0000
@@ -1,3 +1,8 @@
+2011-07-03  Chong Yidong  <address@hidden>
+
+       * xfaces.c (Finternal_merge_in_global_face): Modify the foreground
+       and background color parameters if they have been changed.
+
 2011-07-03  Lars Magne Ingebrigtsen  <address@hidden>
 
        * editfns.c (Fformat): Clarify the - and 0 flags (bug#6659).

=== modified file 'src/xfaces.c'
--- a/src/xfaces.c      2011-06-24 21:25:22 +0000
+++ b/src/xfaces.c      2011-07-03 22:16:07 +0000
@@ -3813,6 +3813,18 @@
              Fmodify_frame_parameters (frame, Fcons (Fcons (Qfont, name),
                                                      Qnil));
            }
+
+         if (STRINGP (gvec[LFACE_FOREGROUND_INDEX]))
+           Fmodify_frame_parameters (frame,
+                                     Fcons (Fcons (Qforeground_color,
+                                                   
gvec[LFACE_FOREGROUND_INDEX]),
+                                            Qnil));
+
+         if (STRINGP (gvec[LFACE_BACKGROUND_INDEX]))
+           Fmodify_frame_parameters (frame,
+                                     Fcons (Fcons (Qbackground_color,
+                                                   
gvec[LFACE_BACKGROUND_INDEX]),
+                                            Qnil));
        }
     }