* bug#14635: 24.3.50; Regression in Customize: no revert changes @ 2013-06-16 9:18 Drew Adams 2013-06-16 10:30 ` Juanma Barranquero 2020-10-30 13:35 ` Mauro Aranda 0 siblings, 2 replies; 9+ messages in thread From: Drew Adams @ 2013-06-16 9:18 UTC (permalink / raw) To: 14635 emacs -Q M-x customize-face default Make some changes. Then choose Set for current session from the State button. Then try to revert your changes using button `Revert...' > `Revert this session's customizations'. There is no effect: no change in the appearance of the buffer. And trying to revert or undo edits using the State button is also impossible: `Revert this session's customizations' is now dimmed out. This is with emacs -Q. The change of state to revert to no changes seems completely broken (a regression). In GNU Emacs 24.3.50.1 (i686-pc-mingw32) of 2013-06-13 on ODIEONE Bzr revision: 112978 xfq.free@gmail.com-20130613224333-3yfl8navh3c1vmxy Windowing system distributor `Microsoft Corp.', version 6.1.7601 Configured using: `configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs CFLAGS='-O0 -g3' CPPFLAGS='-Ic:/Devel/emacs/include' LDFLAGS='-Lc:/Devel/emacs/lib'' ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2013-06-16 9:18 bug#14635: 24.3.50; Regression in Customize: no revert changes Drew Adams @ 2013-06-16 10:30 ` Juanma Barranquero 2020-10-30 13:35 ` Mauro Aranda 1 sibling, 0 replies; 9+ messages in thread From: Juanma Barranquero @ 2013-06-16 10:30 UTC (permalink / raw) To: Drew Adams; +Cc: 14635 On Sun, Jun 16, 2013 at 11:18 AM, Drew Adams <drew.adams@oracle.com> wrote: > emacs -Q > M-x customize-face default > > Make some changes. Then choose Set for current session from the State > button. > > Then try to revert your changes using button `Revert...' > `Revert this > session's customizations'. There is no effect: no change in the > appearance of the buffer. Some things can be reverted (underline, etc.) and some not (font family, slant...). Weird. Juanma ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2013-06-16 9:18 bug#14635: 24.3.50; Regression in Customize: no revert changes Drew Adams 2013-06-16 10:30 ` Juanma Barranquero @ 2020-10-30 13:35 ` Mauro Aranda 2020-10-30 13:43 ` Eli Zaretskii 1 sibling, 1 reply; 9+ messages in thread From: Mauro Aranda @ 2020-10-30 13:35 UTC (permalink / raw) To: Drew Adams; +Cc: 14635 [-- Attachment #1: Type: text/plain, Size: 1566 bytes --] Drew Adams <drew.adams@oracle.com> writes: > emacs -Q > M-x customize-face default > > Make some changes. Then choose Set for current session from the State > button. > > Then try to revert your changes using button `Revert...' > `Revert this > session's customizations'. There is no effect: no change in the > appearance of the buffer. And trying to revert or undo edits using the > State button is also impossible: `Revert this session's customizations' > is now dimmed out. This is with emacs -Q. > > The change of state to revert to no changes seems completely broken (a > regression). > I can reproduce this issue on master. IIUC, this bug is very similar, if not a duplicate, to Bug#13476. But here, we are dealing with the default face, so perhaps it is trickier. For the default face, face-spec-reset-face only sets all attributes to default values if (display-graphic-p frame) returns nil. So on a graphical display, it never resets :family, :foundry, :width, :height, :weight, :slant, :foreground and :background. So, if customizing the foreground color: M-x customize-face RET default Move to Foreground and change it to green C-c C-c to set it for the session Click State and select: Revert this session's customizations The default face is still green. And: (face-attribute 'default :foreground) ==> "green" On a TTY the above recipe works just fine, because we do pass default values here. What would be the right way for face-spec-reset-face to reset all the attributes of the default face to the default values, in a graphic display? [-- Attachment #2: Type: text/html, Size: 1866 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2020-10-30 13:35 ` Mauro Aranda @ 2020-10-30 13:43 ` Eli Zaretskii 2020-10-30 14:03 ` Mauro Aranda 0 siblings, 1 reply; 9+ messages in thread From: Eli Zaretskii @ 2020-10-30 13:43 UTC (permalink / raw) To: Mauro Aranda; +Cc: 14635 > From: Mauro Aranda <maurooaranda@gmail.com> > Date: Fri, 30 Oct 2020 10:35:33 -0300 > Cc: 14635@debbugs.gnu.org > > For the default face, face-spec-reset-face only sets all attributes to > default values if (display-graphic-p frame) returns nil. So on a > graphical display, it never resets :family, :foundry, :width, :height, > :weight, :slant, :foreground and :background. That's because on GUI frames there's no real default for these attributes (unlike on a TTY where we inherit the colors of the terminal). So we simply _cannot_ reset the attributes like that, because there's nothing to reset to. E.g., unspecified-fg only has meaning on a TTY frame. > What would be the right way for face-spec-reset-face to reset all the > attributes of the default face to the default values, in a graphic > display? Doesn't customizing a face record the original value in some property of the face symbol? If so, reverting the customizations should use those recorded values, I think. ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2020-10-30 13:43 ` Eli Zaretskii @ 2020-10-30 14:03 ` Mauro Aranda 2020-10-30 14:16 ` Eli Zaretskii 0 siblings, 1 reply; 9+ messages in thread From: Mauro Aranda @ 2020-10-30 14:03 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 14635 [-- Attachment #1: Type: text/plain, Size: 1645 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: Mauro Aranda <maurooaranda@gmail.com> >> Date: Fri, 30 Oct 2020 10:35:33 -0300 >> Cc: 14635@debbugs.gnu.org >> >> For the default face, face-spec-reset-face only sets all attributes to >> default values if (display-graphic-p frame) returns nil. So on a >> graphical display, it never resets :family, :foundry, :width, :height, >> :weight, :slant, :foreground and :background. > > That's because on GUI frames there's no real default for these > attributes (unlike on a TTY where we inherit the colors of the > terminal). So we simply _cannot_ reset the attributes like that, > because there's nothing to reset to. E.g., unspecified-fg only has > meaning on a TTY frame. I see, thanks. >> What would be the right way for face-spec-reset-face to reset all the >> attributes of the default face to the default values, in a graphic >> display? > > Doesn't customizing a face record the original value in some property > of the face symbol? If so, reverting the customizations should use > those recorded values, I think. AFAICT, it doesn't right now. I followed the recipe I gave, and then: (symbol-plist 'default) The relevant properties I see are: * face-defface-spec ==> ((t nil)) which won't take us anywhere. * theme-face, which has the customized value for the user theme, and * customized-face, which again, has the customized value. But I see no immediate reason why we shouldn't start doing it, if you think it is OK to use that to solve this issue. My only questions are if it would be best to do it in Customize or in faces.el, and if we should only special case the default face. [-- Attachment #2: Type: text/html, Size: 2056 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2020-10-30 14:03 ` Mauro Aranda @ 2020-10-30 14:16 ` Eli Zaretskii 2020-10-30 14:23 ` Mauro Aranda 0 siblings, 1 reply; 9+ messages in thread From: Eli Zaretskii @ 2020-10-30 14:16 UTC (permalink / raw) To: Mauro Aranda; +Cc: 14635 > From: Mauro Aranda <maurooaranda@gmail.com> > Date: Fri, 30 Oct 2020 11:03:39 -0300 > Cc: Drew Adams <drew.adams@oracle.com>, 14635@debbugs.gnu.org > > > Doesn't customizing a face record the original value in some property > > of the face symbol? If so, reverting the customizations should use > > those recorded values, I think. > > AFAICT, it doesn't right now. I followed the recipe I gave, and then: > (symbol-plist 'default) > The relevant properties I see are: > * face-defface-spec ==> ((t nil)) > which won't take us anywhere. > * theme-face, which has the customized value for the user theme, and > * customized-face, which again, has the customized value. > > But I see no immediate reason why we shouldn't start doing it, if you > think it is OK to use that to solve this issue. My only questions are > if it would be best to do it in Customize or in faces.el, and if we > should only special case the default face. I'd begin with doing this in Customize, since it is the only user of this property. ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2020-10-30 14:16 ` Eli Zaretskii @ 2020-10-30 14:23 ` Mauro Aranda 2020-10-31 14:56 ` Mauro Aranda 0 siblings, 1 reply; 9+ messages in thread From: Mauro Aranda @ 2020-10-30 14:23 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 14635 [-- Attachment #1: Type: text/plain, Size: 1113 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: Mauro Aranda <maurooaranda@gmail.com> >> Date: Fri, 30 Oct 2020 11:03:39 -0300 >> Cc: Drew Adams <drew.adams@oracle.com>, 14635@debbugs.gnu.org >> >> > Doesn't customizing a face record the original value in some property >> > of the face symbol? If so, reverting the customizations should use >> > those recorded values, I think. >> >> AFAICT, it doesn't right now. I followed the recipe I gave, and then: >> (symbol-plist 'default) >> The relevant properties I see are: >> * face-defface-spec ==> ((t nil)) >> which won't take us anywhere. >> * theme-face, which has the customized value for the user theme, and >> * customized-face, which again, has the customized value. >> >> But I see no immediate reason why we shouldn't start doing it, if you >> think it is OK to use that to solve this issue. My only questions are >> if it would be best to do it in Customize or in faces.el, and if we >> should only special case the default face. > > I'd begin with doing this in Customize, since it is the only user of > this property. I'll try to do it. Thank you. [-- Attachment #2: Type: text/html, Size: 1571 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2020-10-30 14:23 ` Mauro Aranda @ 2020-10-31 14:56 ` Mauro Aranda 2022-02-05 23:39 ` Lars Ingebrigtsen 0 siblings, 1 reply; 9+ messages in thread From: Mauro Aranda @ 2020-10-31 14:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 14635 [-- Attachment #1.1: Type: text/plain, Size: 1466 bytes --] Mauro Aranda <maurooaranda@gmail.com> writes: > Eli Zaretskii <eliz@gnu.org> writes: > >>> From: Mauro Aranda <maurooaranda@gmail.com> >>> Date: Fri, 30 Oct 2020 11:03:39 -0300 >>> Cc: Drew Adams <drew.adams@oracle.com>, 14635@debbugs.gnu.org >>> >>> > Doesn't customizing a face record the original value in some property >>> > of the face symbol? If so, reverting the customizations should use >>> > those recorded values, I think. >>> >>> AFAICT, it doesn't right now. I followed the recipe I gave, and then: >>> (symbol-plist 'default) >>> The relevant properties I see are: >>> * face-defface-spec ==> ((t nil)) >>> which won't take us anywhere. >>> * theme-face, which has the customized value for the user theme, and >>> * customized-face, which again, has the customized value. >>> >>> But I see no immediate reason why we shouldn't start doing it, if you >>> think it is OK to use that to solve this issue. My only questions are >>> if it would be best to do it in Customize or in faces.el, and if we >>> should only special case the default face. >> >> I'd begin with doing this in Customize, since it is the only user of >> this property. > > I'll try to do it. Thank you. I attach a patch that makes Customize store the original value of the default face if the user ever changes it, and then uses it when reverting. Basically, Customize tell faces.el the exactly spec to set, by putting a fake entry under the user theme (and later removing it). [-- Attachment #1.2: Type: text/html, Size: 2086 bytes --] [-- Attachment #2: 0001-Fix-reverting-the-default-face-to-standard-themed-st.patch --] [-- Type: text/x-patch, Size: 5560 bytes --] From 88bcf54a4138acd6782c70ac8ffe9db5e098956a Mon Sep 17 00:00:00 2001 From: Mauro Aranda <maurooaranda@gmail.com> Date: Sat, 31 Oct 2020 10:39:31 -0300 Subject: [PATCH] Fix reverting the default face to standard/themed state in GUIs * lisp/cus-edit.el (custom-face-set, custom-face-mark-to-save) (custom-face-save): Record the default (standard or themed) attributes of the default face in a symbol property. (custom-face-reset-saved, custom-face-mark-to-reset-standard): When reverting the default face to the standard or themed state, use the default attributes we recorded, instead of relying in the defface spec of the default face, since that doesn't give enough information to reset all attributes, like foreground, family, etc. (Bug#14635) --- lisp/cus-edit.el | 50 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el index 769a69a50f..db748c1096 100644 --- a/lisp/cus-edit.el +++ b/lisp/cus-edit.el @@ -3895,6 +3895,18 @@ custom-face-set (setq comment nil) ;; Make the comment invisible by hand if it's empty (custom-comment-hide comment-widget)) + ;; When modifying the default face, we need to save the standard or themed + ;; attrs, in case the user asks to revert to them in the future. + ;; In GUIs, when resetting the attributes of the default face, the frame + ;; parameters associated with this face won't change, unless explicitly + ;; passed a value. Storing this known attrs allows us to tell faces.el to + ;; set those attributes to specified values, making the relevant frame + ;; parameters stay in sync with the default face. + (when (and (eq symbol 'default) + (not (get symbol 'custom-face-default-attrs)) + (memq (custom-face-state symbol) '(standard themed))) + (put symbol 'custom-face-default-attrs + (custom-face-get-current-spec symbol))) (custom-push-theme 'theme-face symbol 'user 'set value) (face-spec-set symbol value 'customized-face) (put symbol 'face-comment comment) @@ -3913,6 +3925,12 @@ custom-face-mark-to-save (setq comment nil) ;; Make the comment invisible by hand if it's empty (custom-comment-hide comment-widget)) + ;; See the comments in `custom-face-set'. + (when (and (eq symbol 'default) + (not (get symbol 'custom-face-default-attrs)) + (memq (custom-face-state symbol) '(standard themed))) + (put symbol 'custom-face-default-attrs + (custom-face-get-current-spec symbol))) (custom-push-theme 'theme-face symbol 'user 'set value) (face-spec-set symbol value (if standard 'reset 'saved-face)) (put symbol 'face-comment comment) @@ -3926,7 +3944,14 @@ custom-face-state-set-and-redraw (defun custom-face-save (widget) "Save the face edited by WIDGET." - (let ((form (widget-get widget :custom-form))) + (let ((form (widget-get widget :custom-form)) + (symbol (widget-value widget))) + ;; See the comments in `custom-face-set'. + (when (and (eq symbol 'default) + (not (get symbol 'custom-face-default-attrs)) + (memq (custom-face-state symbol) '(standard themed))) + (put symbol 'custom-face-default-attrs + (custom-face-get-current-spec symbol))) (if (memq form '(all lisp)) (custom-face-mark-to-save widget) ;; The user is working on only a selected terminal type; @@ -3949,10 +3974,20 @@ custom-face-reset-saved (saved-face (get face 'saved-face)) (comment (get face 'saved-face-comment)) (comment-widget (widget-get widget :comment-widget))) + ;; If resetting the default face and there isn't a saved value, + ;; push a fake user setting, so that reverting to the default + ;; attributes works. (custom-push-theme 'theme-face face 'user - (if saved-face 'set 'reset) - saved-face) + (if (or saved-face (eq face 'default)) 'set 'reset) + (or saved-face + ;; If this is t, then MODE is 'reset, + ;; and `custom-push-theme' ignores this argument. + (not (eq face 'default)) + (get face 'custom-face-default-attrs))) (face-spec-set face saved-face 'saved-face) + (when (and (not saved-face) (eq face 'default)) + ;; Remove the fake user setting. + (custom-push-theme 'theme-face face 'user 'reset)) (put face 'face-comment comment) (put face 'customized-face-comment nil) (widget-value-set child saved-face) @@ -3974,8 +4009,15 @@ custom-face-mark-to-reset-standard (comment-widget (widget-get widget :comment-widget))) (unless value (user-error "No standard setting for this face")) - (custom-push-theme 'theme-face symbol 'user 'reset) + ;; If erasing customizations for the default face, push a fake user setting, + ;; so that reverting to the default attributes works. + (custom-push-theme 'theme-face symbol 'user + (if (eq symbol 'default) 'set 'reset) + (or (not (eq symbol 'default)) + (get symbol 'custom-face-default-attrs))) (face-spec-set symbol value 'reset) + ;; Remove the fake user setting. + (custom-push-theme 'theme-face symbol 'user 'reset) (put symbol 'face-comment nil) (put symbol 'customized-face-comment nil) (if (and custom-reset-standard-faces-list -- 2.29.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#14635: 24.3.50; Regression in Customize: no revert changes 2020-10-31 14:56 ` Mauro Aranda @ 2022-02-05 23:39 ` Lars Ingebrigtsen 0 siblings, 0 replies; 9+ messages in thread From: Lars Ingebrigtsen @ 2022-02-05 23:39 UTC (permalink / raw) To: Mauro Aranda; +Cc: 14635 Mauro Aranda <maurooaranda@gmail.com> writes: > I attach a patch that makes Customize store the original value of the > default face if the user ever changes it, and then uses it when > reverting. > > Basically, Customize tell faces.el the exactly spec to set, by putting a > fake entry under the user theme (and later removing it). Looks like this patch was forgotten, so I've now pushed it to Emacs 29. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-05 23:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-06-16 9:18 bug#14635: 24.3.50; Regression in Customize: no revert changes Drew Adams 2013-06-16 10:30 ` Juanma Barranquero 2020-10-30 13:35 ` Mauro Aranda 2020-10-30 13:43 ` Eli Zaretskii 2020-10-30 14:03 ` Mauro Aranda 2020-10-30 14:16 ` Eli Zaretskii 2020-10-30 14:23 ` Mauro Aranda 2020-10-31 14:56 ` Mauro Aranda 2022-02-05 23:39 ` Lars Ingebrigtsen
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).