* Change in face-spec-set
@ 2007-10-19 17:42 Richard Stallman
2007-10-19 20:10 ` Glenn Morris
0 siblings, 1 reply; 9+ messages in thread
From: Richard Stallman @ 2007-10-19 17:42 UTC (permalink / raw)
To: emacs-devel
I wrote the following patch to fix one of the bugs in `face-spec-set'
which was introduced by my first change there. Other changes have
been made subsequently, and I don't know whether this patch is still
needed, or whether any problems remain there. Can someone tell me
the situation?
diff -c -c -r1.381 faces.el
*** faces.el 18 Oct 2007 19:02:22 -0000 1.381
--- faces.el 19 Oct 2007 17:25:59 -0000
***************
*** 1472,1477 ****
--- 1472,1483 ----
;; When we reset the face based on its spec, then it is unmodified
;; as far as Custom is concerned.
(put (or (get face 'face-alias) face) 'face-modified nil)
+ ;; Clear all the new-frame defaults for this face.
+ ;; face-spec-reset-face won't do it right.
+ (let ((facevec (cdr (assq face face-new-frame-defaults))))
+ (dotimes (i (length facevec))
+ (unless (= i 0)
+ (aset facevec i 'unspecified))))
;; Set each frame according to the rules implied by SPEC.
(dolist (frame (frame-list))
(face-spec-set face spec frame))))
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Change in face-spec-set
2007-10-19 17:42 Change in face-spec-set Richard Stallman
@ 2007-10-19 20:10 ` Glenn Morris
2007-10-19 21:12 ` Glenn Morris
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Glenn Morris @ 2007-10-19 20:10 UTC (permalink / raw)
To: rms; +Cc: emacs-devel
Richard Stallman wrote:
> I wrote the following patch to fix one of the bugs in `face-spec-set'
> which was introduced by my first change there. Other changes have
> been made subsequently, and I don't know whether this patch is still
> needed, or whether any problems remain there. Can someone tell me
> the situation?
The only change to face-spec-set is mine, to set the default for new
frames when FRAME is nil. If I apply your patch, this is once again
broken. By design it seems ("Clear all the new-frame defaults for this
face"). Why do you want to do this?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Change in face-spec-set
2007-10-19 20:10 ` Glenn Morris
@ 2007-10-19 21:12 ` Glenn Morris
2007-10-20 23:47 ` Johan Bockgård
2007-10-20 14:57 ` Richard Stallman
2007-10-27 23:41 ` Richard Stallman
2 siblings, 1 reply; 9+ messages in thread
From: Glenn Morris @ 2007-10-19 21:12 UTC (permalink / raw)
To: rms; +Cc: emacs-devel
Glenn Morris wrote:
> Richard Stallman wrote:
>
>> I wrote the following patch to fix one of the bugs in `face-spec-set'
>> which was introduced by my first change there. Other changes have
>> been made subsequently, and I don't know whether this patch is still
>> needed, or whether any problems remain there. Can someone tell me
>> the situation?
>
> The only change to face-spec-set is mine, to set the default for new
> frames when FRAME is nil. If I apply your patch, this is once again
> broken.
Changing your patch to look like this seems better, if you think there
is some problem in face-spec-reset-face. Is fixing face-spec-reset-face
not an option?
*** faces.el 18 Oct 2007 19:02:22 -0000 1.381
--- faces.el 19 Oct 2007 21:11:31 -0000
***************
*** 1449,1455 ****
See `defface' for information about SPEC. If SPEC is nil, do nothing."
(let ((attrs (face-spec-choose spec frame)))
(when spec
! (face-spec-reset-face face (or frame t)))
(while attrs
(let ((attribute (car attrs))
(value (car (cdr attrs))))
--- 1449,1462 ----
See `defface' for information about SPEC. If SPEC is nil, do nothing."
(let ((attrs (face-spec-choose spec frame)))
(when spec
! (if frame
! (face-spec-reset-face face frame)
! ;; Clear all the new-frame defaults for this face.
! ;; face-spec-reset-face won't do it right.
! (let ((facevec (cdr (assq face face-new-frame-defaults))))
! (dotimes (i (length facevec))
! (unless (= i 0)
! (aset facevec i 'unspecified))))))
(while attrs
(let ((attribute (car attrs))
(value (car (cdr attrs))))
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Change in face-spec-set
2007-10-19 21:12 ` Glenn Morris
@ 2007-10-20 23:47 ` Johan Bockgård
0 siblings, 0 replies; 9+ messages in thread
From: Johan Bockgård @ 2007-10-20 23:47 UTC (permalink / raw)
To: emacs-devel
Glenn Morris <rgm@gnu.org> writes:
> Changing your patch to look like this seems better, if you think there
> is some problem in face-spec-reset-face. Is fixing
> face-spec-reset-face not an option?
The effect of face-spec-reset-face on the new-frame default for a face
is not to "set if from zero". It produces an entry like this in
`face-new-frame-defaults'
(FACE . [face :ignore-defface :ignore-defface ...])
But we need `unspecified', not :ignore-defface. (`unspecified' means "no
global definition", whereas :ignore-defface means "the global definition
is `unspecified'").
(http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-10/msg00208.html)
> *** faces.el 18 Oct 2007 19:02:22 -0000 1.381
> --- faces.el 19 Oct 2007 21:11:31 -0000
> ***************
> *** 1449,1455 ****
> See `defface' for information about SPEC. If SPEC is nil, do nothing."
> (let ((attrs (face-spec-choose spec frame)))
> (when spec
> ! (face-spec-reset-face face (or frame t)))
> (while attrs
> (let ((attribute (car attrs))
> (value (car (cdr attrs))))
> --- 1449,1462 ----
> See `defface' for information about SPEC. If SPEC is nil, do nothing."
> (let ((attrs (face-spec-choose spec frame)))
> (when spec
> ! (if frame
> ! (face-spec-reset-face face frame)
> ! ;; Clear all the new-frame defaults for this face.
> ! ;; face-spec-reset-face won't do it right.
> ! (let ((facevec (cdr (assq face face-new-frame-defaults))))
> ! (dotimes (i (length facevec))
> ! (unless (= i 0)
> ! (aset facevec i 'unspecified))))))
> (while attrs
> (let ((attribute (car attrs))
> (value (car (cdr attrs))))
--
Johan Bockgård
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Change in face-spec-set
2007-10-19 20:10 ` Glenn Morris
2007-10-19 21:12 ` Glenn Morris
@ 2007-10-20 14:57 ` Richard Stallman
2007-10-27 23:41 ` Richard Stallman
2 siblings, 0 replies; 9+ messages in thread
From: Richard Stallman @ 2007-10-20 14:57 UTC (permalink / raw)
To: Glenn Morris; +Cc: emacs-devel
The only change to face-spec-set is mine, to set the default for new
frames when FRAME is nil. If I apply your patch, this is once again
broken. By design it seems ("Clear all the new-frame defaults for this
face"). Why do you want to do this?
Because face-spec-set is supposed to completely override the settings
of the face, setting it from zero.
With the current code, and without my patch, does that work?
If so, my patch is not necessary.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Change in face-spec-set
2007-10-19 20:10 ` Glenn Morris
2007-10-19 21:12 ` Glenn Morris
2007-10-20 14:57 ` Richard Stallman
@ 2007-10-27 23:41 ` Richard Stallman
2007-10-29 6:39 ` Glenn Morris
2 siblings, 1 reply; 9+ messages in thread
From: Richard Stallman @ 2007-10-27 23:41 UTC (permalink / raw)
To: Glenn Morris; +Cc: emacs-devel
[I sent this message a week ago but did not get a response.
Could we get the discussion moving again?]
The only change to face-spec-set is mine, to set the default for new
frames when FRAME is nil. If I apply your patch, this is once again
broken. By design it seems ("Clear all the new-frame defaults for this
face"). Why do you want to do this?
Because face-spec-set is supposed to completely override the settings
of the face, setting it from zero.
With the current code, and without my patch, does that work?
If so, my patch is not necessary.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Change in face-spec-set
2007-10-27 23:41 ` Richard Stallman
@ 2007-10-29 6:39 ` Glenn Morris
2007-10-29 14:02 ` Johan Bockgård
2007-10-29 23:35 ` Richard Stallman
0 siblings, 2 replies; 9+ messages in thread
From: Glenn Morris @ 2007-10-29 6:39 UTC (permalink / raw)
To: rms; +Cc: emacs-devel
Richard Stallman wrote:
> Because face-spec-set is supposed to completely override the settings
> of the face, setting it from zero.
>
> With the current code, and without my patch, does that work?
Give me a test case starting from emacs -Q and I'll tell you.
If I try to guess what you might mean, then with the current code:
emacs -Q
(describe-face 'font-lock-warning-face)
-> weight = bold
(face-spec-set 'font-lock-warning-face '((t :underline t)))
(make-frame)
On new frame:
(describe-face 'font-lock-warning-face)
-> weight = unspecified
Is this the kind of thing you are talking about?
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-10-29 23:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-19 17:42 Change in face-spec-set Richard Stallman
2007-10-19 20:10 ` Glenn Morris
2007-10-19 21:12 ` Glenn Morris
2007-10-20 23:47 ` Johan Bockgård
2007-10-20 14:57 ` Richard Stallman
2007-10-27 23:41 ` Richard Stallman
2007-10-29 6:39 ` Glenn Morris
2007-10-29 14:02 ` Johan Bockgård
2007-10-29 23:35 ` Richard Stallman
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).