bug#3210: face customization fails after set-face-attribute

bug#3210: face customization fails after set-face-attribute

[David Reitter]

Emacs -Q

(set-face-attribute 'default nil
		    :height 130
		    :family "Lucida Grande")
;; modifies default face

(customize-face 'default)
;; switch back manually to Monaco and "set for current session"

(make-frame-command)  ;; C-x 5 2
;; the new frame is shown in Lucida.  Why?


Upon inspection of some code, it appears unclear why `face-spec-set'  
iterates over all frame faces and sets the new spec there, but doesn't  
set the right property to set the face for future frames in case for- 
defface is t.

bug#3210: face customization fails after set-face-attribute

[Chong Yidong]

David Reitter <david.reitter@gmail.com> writes:

> (set-face-attribute 'default nil
> 		    :height 130
> 		    :family "Lucida Grande")
> ;; modifies default face
>
> (customize-face 'default)
> ;; switch back manually to Monaco and "set for current session"
>
> (make-frame-command)  ;; C-x 5 2
> ;; the new frame is shown in Lucida.  Why?

I have edited the docstring of set-face-attribute, to make it clearer
that this function overrides face specs.

bug#3408: bug#3210: face customization fails after set-face-attribute

[Drew Adams]

> > (set-face-attribute 'default nil :height 130 :family "Lucida Grande")
> > ;; modifies default face
> >
> > (customize-face 'default)
> > ;; switch back manually to Monaco and "set for current session"
> >
> > (make-frame-command)  ;; C-x 5 2
> > ;; the new frame is shown in Lucida.  Why?
> 
> I have edited the docstring of set-face-attribute, to make it clearer
> that this function overrides face specs.

Huh?  You baptize the bugged behvior as design by documenting it as intended?
No one intended or intends such behavior, AFAIK.  It is just an unfortunate,
unintended side effect of some implementation changes that someone made.  IOW, a
bug.

And why close bug #3408 at the same time?  That bugged behavior remains.  And it
is a regression from the behavior in Emacs 22 (and 21 and 20 and...).

As I said in the #3408 thread, and to which there was no reply: 

| Customize is for changing user preferences, and those apply most
| importantly to future use, not just to existing objects.
| If Customize becomes just about repainting what's there already,
| then Customize is no longer about customizing.
...
| That is a ridiculous workaround, just to get a face change for
| future frames: save, end the session, new session to get where you
| wanted to be.  Then restore the definition, save again, and exit,
| so your change lasted only for the "macro-session" (split into two
| sessions, just for the workaround).
|
| What was wrong with what we had before? What problem does this
| significant change solve?
|
| *Any* way of changing a face (or an option, for that matter) should
| affect it for the future.

If you do not have the time now to fix a particular bug (a regression, no less),
then classify it as `wishlist'.  If you do not want to fix a bug, ever, then
classify it as `wont-fix'.

But please do not classify it for such reasons as `notabug'.  A bug is a bug.
It is not the same as intentional design.  Emacs Dev made implementation changes
in Emacs 23 that broke things.  If you will not fix them then `wont-fix' is the
right category.

Or if you really claim that this is a design change, then be clear to users:
document it generally for Customize:

  In the case of faces, Customize is about repainting what's
  there already.  It is not about customizing for the future.
  The advantages of this exception for faces are...

And add that design change to the NEWS (for Emacs 23), as a new "feature".

bug#3210: bug#3408: bug#3210: face customization fails after set-face-attribute

[Drew Adams]

Bug #3408 is a _regression_, and it has not been fixed.
A regression is certainly NOT `notabug'.

As I said on 2012-06-09:

> If you do not have the time now to fix a particular bug (a 
> regression, no less), then classify it as `wishlist'.
> If you do not want to fix a bug, ever, then classify it as
> `wont-fix'.
> 
> But please do not classify it for such reasons as `notabug'.  
> A bug is a bug.  It is not the same as intentional design.
> Emacs Dev made implementation changes in Emacs 23 that broke
> things.  If you will not fix them then `wont-fix' is the
> right category.
> 
> Or if you really claim that this is a design change, then be 
> clear to users: document it generally for Customize:
> 
>   In the case of faces, Customize is about repainting what's
>   there already.  It is not about customizing for the future.
>   The advantages of this exception for faces are...
> 
> And add that design change to the NEWS (for Emacs 23), as a 
> new "feature".

bug#3210: bug#3408: bug#3210: face customization fails after set-face-attribute

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> Bug #3408 is a _regression_, and it has not been fixed.
> A regression is certainly NOT `notabug'.

I disagree that this is a bug.  I have now taken another look at the
code, and my earlier conclusion stands.  Modifications to internal Lisp
faces (via set-face-attribute) take precedence over the effect of face
specs, so the behavior is working as intended and as documented
(including in the docstring of set-face-attribute).

The trunk now contains a change to face-spec-set which allows it to be
more easily called from Lisp code to override defface specs.  If you
want to modify faces from Lisp, use that rather than set-face-attribute.

Don't re-open this bug based on uninformed opinion.  Thanks.