Re: Faces applies to new frames

[Stefan Monnier]

> The trouble is that the `font' frame parameter interferes with the
> normal semantics of Lisp faces.  It should always be defined for each
> frame,

That's why I introduced font-parameter: it stores the setting specified
by the user (can be nil) rather than the actual font used for the frame
(never nil, always fully-specified, AFAIK).

> so if it always takes precedence, there would be no good way to
> make changes to the default face apply to new frames, because any change
> would be overriden by `font'.  And, I don't think it's easy to calculate
> what `font' ought to be ahead of time.  That's why I suggested setting
> `font' in face-set-after-frame-default, after processing the defface
> spec.

> By the way, a more limited change than what I suggested earlier seems to
> DTRT: with this, calling make-frame with a `font' frame parameter seems
> to DTRT, as do other tests of set-face-attribute I tried out.

> *** trunk/lisp/faces.el.~1.416.~      2008-06-28 22:14:35.000000000 -0400
> --- trunk/lisp/faces.el       2008-06-29 00:13:28.000000000 -0400
> ***************
> *** 2056,2061 ****
> --- 2056,2065 ----
>               (make-face-x-resource-internal face frame))
>           (internal-merge-in-global-face face frame))
>       (error nil)))
> +     ;; The face specs may specify a different default font.  Save this
> +     ;; in the `font' frame parameter.
> +     (when (face-font 'default)
> +       (set-frame-parameter frame 'font (face-font 'default)))

That can't be right.  What you're doing here is copying the
selected-frame's default font to `frame'.  Why should we rely on the
selected frame?

>> This implies that set-face-attribute should probably store global
>> settings elsewhere than on default-frame-alist.  It sounds like
>> face-new-frame-defaults would be a good place for that instead (th0o
>> this is only based on its name)
> That's currently the case.

Oh, yes, I see it now.  Good.

>> Here's my current understanding of the various places where we store
>> face info:
>> 
>> - face-new-frame-defaults, should be global and seems to only ever
>> contain "unspecified".  I.e. pretty much unused.

> face-new-frame-defaults is where set-face-attribute stores changes that
> are supposed to be applied to faces on furture frames.  So if you do

>   (set-face-attribute 'default nil :slant 'italic)

> Then you'll see that the `default' entry in face-new-frame-defaults
> contains `italic' in its slant attribute.

Yes, I now see it, and found the code that does it.

>> 1 - rule out frame-specific Xresources settings
>> 2 - make face-new-frame-defaults terminal-local, so it's initialized at
>> terminal-creation time by applying defface and Xresources settings.
>> 3 - rule that "global" set-face-attribute settings only apply to future
>> frames on the same terminals (i.e. they're not truly global any
>> more), so it can work by modifying the terminal's
>> face-new-frame-defaults and it will hence take precedence over
>> customize (i.e. defface) and Xresources settings.

> I don't see how this makes sense.  As long as each frame is allowed to
> have its own face settings, the current dance in face-new-frame-defaults
> will be necessary each time we create a new frace.

Why?  Here's the dance, as I understand it:

  (unless inhibit-face-set-after-frame-default
    (if (face-attribute 'default :font t)
        (set-face-attribute 'default frame :font
                            (face-attribute 'default :font t))
      (set-face-attribute 'default frame :family
                          (face-attribute 'default :family t))
      (set-face-attribute 'default frame :height
                          (face-attribute 'default :height t))
      (set-face-attribute 'default frame :slant
                          (face-attribute 'default :slant t))
      (set-face-attribute 'default frame :weight
                          (face-attribute 'default :weight t))
      (set-face-attribute 'default frame :width
                          (face-attribute 'default :width t))))

No idea what this is about, really.  It appears to read some of the
settings for the `default' face from face-new-frame-defaults and then
applies them to the `default' face on the current frame.

This seems both useless and dubious: face-new-frame-defaults is
presumably obeyed elsewhere already, and why should we only play with
the `font' part of the `default' face?

  ;; Find attributes that should be initialized from frame parameters.
  (let ((face-params '((foreground-color default :foreground)
                       (background-color default :background)
                       (font-parameter default :font)
                       (border-color border :background)
                       (cursor-color cursor :background)
                       (scroll-bar-foreground scroll-bar :foreground)
                       (scroll-bar-background scroll-bar :background)
                       (mouse-color mouse :background)))
        apply-params)
    (dolist (param face-params)
      (let* ((value (frame-parameter frame (nth 0 param)))
             (face (nth 1 param))
             (attr (nth 2 param))
             (default-value (face-attribute face attr t)))
        ;; Compile a list of face attributes to set, but don't set
        ;; them yet.  The call to make-face-x-resource-internal,
        ;; below, can change frame parameters, and the final set of
        ;; frame parameters should be the ones acquired at this step.
        (if (eq default-value 'unspecified)
            ;; The face spec does not specify a new-frame value for
            ;; this attribute.  Check if the existing frame parameter
            ;; specifies it.
            (if value
                (push (list face frame attr value) apply-params))
          ;; The face spec specifies a value for this attribute, to be
          ;; applied to the face on all new frames.
          (push (list face frame attr default-value) apply-params))))

Extract face info from the frame parameters.  It's not applied yet,
because we want those frame parameter settings to take precedence.

    ;; Initialize faces from face specs and X resources.  The
    ;; condition-case prevents invalid specs from causing frame
    ;; creation to fail.
    (dolist (face (face-list))
      ;; This loop used to exclude the `default' face for an unknown reason.
      ;; It lead to odd behaviors where face-spec settings on the `default'
      ;; face weren't obeyed for new frame.
      (condition-case ()
          (progn
            (face-spec-recalc face frame)

Apply the defface specs.

            (if (memq (window-system frame) '(x w32 mac))
                (make-face-x-resource-internal face frame))

Apply the Xresources specs.

            (internal-merge-in-global-face face frame))

Apply the face-new-frame-defaults.  Note that although global, they take
precedence over defface and Xresources.

        (error nil)))
    ;; Apply the attributes specified by frame parameters.  This
    ;; rewrites parameters changed by make-face-x-resource-internal
    (dolist (param apply-params)
      (apply 'set-face-attribute param))))

Finally, apply the face info extracted from the frame-parameters.

So, ignoring the first part, we do:

- apply defface
- apply Xresources
- apply face-new-frame-defaults
- apply frame-parameters

The first 3 will always return the exact same result for every frame on
the same terminal (at least, if we ignore frame-specific Xresource
settings).

So I don't see why we can't just precompute this result at
terminal-creation time and store it directly in face-new-frame-defaults
(after making this variable terminal-local).


        Stefan