bug#44331: 28.0.50; Customize won't save a face setting, if not set first

bug#44331: 28.0.50; Customize won't save a face setting, if not set first

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 6195 bytes --]

Since this is about saving a Custom setting, it is not reproducible
without emacs -Q.  So, before following the recipe, make sure you backup
your custom-file, or at least make sure it doesn't have any face
settings for the face you choose to customize.

If possible, use a face you don't have customized under a theme, or
something like that.  I'll use the error face for my recipe:

Start emacs
M-x customize-face RET error
I see that the state is STANDARD, and the foreground is Red1.
Then change foreground to some color, like orange.
In the State button, choose "Save for Future Sessions".  Don't choose
"Set for Current Session", because that way there's no bug.
Now Save For Future Sessions should *set* the setting, and save it into
the custom file.

I see that the state says THEMED, which I don't think is right: it
should say SAVED and SET.  There is no theme (other than the user theme)
that is customizing the error face in this session.

I see that the foreground value came back to being Red1.  So, it didn't
set the face.

I get a message that says "Wrote /path/to/custom-file"

C-x C-f "/path/to/custom-file"
And when I look at the custom-set-faces entry, I see no setting at all.


If in another session, I choose "Set for Current Session" and then "Save
for Future Sessions", Customize saves the setting just fine.


In GNU Emacs 28.0.50 (build 10, x86_64-pc-linux-gnu, cairo version 1.15.10)
 of 2020-10-30 built on tbb-desktop
Repository revision: 0d8c6df6c1f5e1150b34d4a57acee25471b47340
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Ubuntu 18.04.5 LTS

Configured using:
 'configure --with-x-toolkit=no'

Configured features:
XPM JPEG TIFF GIF PNG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY INOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ ZLIB OLDXMENU X11 XDBE XIM
MODULES THREADS PDUMPER

Important settings:
  value of $LC_MONETARY: es_AR.UTF-8
  value of $LC_NUMERIC: es_AR.UTF-8
  value of $LC_TIME: es_AR.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Custom

Minor modes in effect:
  global-ede-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow mail-extr emacsbug message rmc puny rfc822 mml mml-sec epa epg
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail color help-mode crm cus-edit
emms-librefm-stream emms-librefm-scrobbler emms-playlist-limit
emms-volume emms-volume-mixerctl emms-volume-pulse emms-volume-amixer
emms-i18n emms-history emms-score emms-stream-info
emms-metaplaylist-mode emms-bookmarks emms-cue emms-mode-line-icon
emms-browser sort emms-playlist-sort emms-last-played emms-player-xine
emms-player-mpd tq emms-playing-time emms-lyrics emms-url emms-streams
emms-show-all emms-tag-editor emms-mark emms-mode-line emms-cache
emms-info-tinytag emms-info-metaflac emms-info-opusinfo
emms-info-ogginfo emms-info-mp3info emms-info later-do
emms-playlist-mode emms-player-vlc emms-player-mpv emms-player-mplayer
emms-player-simple emms-source-playlist emms-source-file locate dired
dired-loaddefs emms-setup emms emms-compat gnus nnheader gnus-util rmail
rmail-loaddefs rfc2047 rfc2045 ietf-drums mail-utils mm-util mail-prsvr
cus-start cus-load org-capture deeper-blue-theme init-org org-clock org
ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote
org-src ob-comint org-pcomplete pcomplete org-list org-faces
org-entities org-version ob-emacs-lisp ob-core ob-eval org-table ol
org-keys org-compat advice org-macs org-loaddefs format-spec find-func
cal-menu calendar cal-loaddefs init-social newsticker newst-treeview
tree-widget newst-plainview newst-reader newst-ticker newst-backend
iso8601 time-date xml derived init-cedet ede/speedbar ede/files ede
ede/detect ede/base ede/auto ede/source eieio-base eieio-speedbar
speedbar ezimage dframe eieio-custom wid-edit cedet init-octave
init-rmail init-vc vc vc-dispatcher init-cc-mode init-c init-yasnippet
init-eshell eshell esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg
esh-module esh-groups esh-util init-lisp slime compile
text-property-search etags fileloop generator xref project arc-mode
archive-mode noutline outline easy-mmode pp comint ansi-color ring
hyperspec thingatpt init-global finder-inf init-package slime-autoloads
info package easymenu browse-url url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util mailcap
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo x
multi-tty make-network-process emacs)

Memory information:
((conses 16 394955 20573)
 (symbols 48 27006 1)
 (strings 32 118231 5862)
 (string-bytes 1 3318356)
 (vectors 16 45818)
 (vector-slots 8 1259408 184943)
 (floats 8 225 329)
 (intervals 56 4361 402)
 (buffers 992 14))

[-- Attachment #2: Type: text/html, Size: 6783 bytes --]

bug#44331: 28.0.50; Customize won't save a face setting, if not set first

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 444 bytes --]

tags 44331 patch
quit

Looks like this was introduced in the fix for Bug#40866.  When only
customizing for the selected terminal, now we recreate the custom-face
widget for all kind of displays, but lose the user editions in the
process of doing that.

The attached patch fixes it, because it records the edited value in
the :shown-value property, which a custom widget prioritizes over
everything else, when finding out the value of the face.

[-- Attachment #1.2: Type: text/html, Size: 501 bytes --]

[-- Attachment #2: 0001-Fix-saving-a-face-setting-with-Customize.patch --]
[-- Type: text/x-patch, Size: 1241 bytes --]

From c3f92a5431defff64c6956bbcbee12638ae4ecef Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Fri, 30 Oct 2020 13:05:08 -0300
Subject: [PATCH] Fix saving a face setting with Customize

* lisp/cus-edit.el (custom-face-save): Make sure we back up into the
:shown-value property what the user has edited so far, if we are going
to recreate the custom-face widget.  (Bug#44331)
---
 lisp/cus-edit.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 769a69a50f..01dec6964d 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -3931,6 +3931,10 @@ custom-face-save
         (custom-face-mark-to-save widget)
       ;; The user is working on only a selected terminal type;
       ;; make sure we save the entire spec to `custom-file'. (Bug #40866)
+      ;; If recreating a widget that may have been edited by the user, remember
+      ;; to always save the edited value into the :shown-value property, so
+      ;; we use that value for the recreated widget.  (Bug#44331)
+      (widget-put widget :shown-value (custom-face-widget-to-spec widget))
       (custom-face-edit-all widget)
       (custom-face-mark-to-save widget)
       (if (eq form 'selected)
-- 
2.29.0

bug#44331: 28.0.50; Customize won't save a face setting, if not set first

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 81 bytes --]

Sending a better patch, that cleans up the :shown-value property after
using it.

[-- Attachment #1.2: Type: text/html, Size: 105 bytes --]

[-- Attachment #2: 0001-Fix-saving-a-face-setting-with-Customize.patch --]
[-- Type: text/x-patch, Size: 1361 bytes --]

From 4a2764c73663bc2d0c6a8a0fcbea55b85aaa9a7e Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Fri, 30 Oct 2020 13:05:08 -0300
Subject: [PATCH] Fix saving a face setting with Customize

* lisp/cus-edit.el (custom-face-save): Make sure we back up into the
:shown-value property what the user has edited so far, if we are going
to recreate the custom-face widget.  (Bug#44331)
---
 lisp/cus-edit.el | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 769a69a50f..3eef446839 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -3931,7 +3931,12 @@ custom-face-save
         (custom-face-mark-to-save widget)
       ;; The user is working on only a selected terminal type;
       ;; make sure we save the entire spec to `custom-file'. (Bug #40866)
+      ;; If recreating a widget that may have been edited by the user, remember
+      ;; to always save the edited value into the :shown-value property, so
+      ;; we use that value for the recreated widget.  (Bug#44331)
+      (widget-put widget :shown-value (custom-face-widget-to-spec widget))
       (custom-face-edit-all widget)
+      (widget-put widget :shown-value nil) ; Reset it after we used it.
       (custom-face-mark-to-save widget)
       (if (eq form 'selected)
           (custom-face-edit-selected widget)
-- 
2.29.0

bug#44331: 28.0.50; Customize won't save a face setting, if not set first

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> * lisp/cus-edit.el (custom-face-save): Make sure we back up into the
> :shown-value property what the user has edited so far, if we are going
> to recreate the custom-face widget.  (Bug#44331)

Sounds good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44331: 28.0.50; Customize won't save a face setting, if not set first

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

tags 44331 fixed
close 44331
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> * lisp/cus-edit.el (custom-face-save): Make sure we back up into the
>> :shown-value property what the user has edited so far, if we are going
>> to recreate the custom-face widget.  (Bug#44331)
>
> Sounds good to me.

Thanks.  Pushed to master, and closing.

[-- Attachment #2: Type: text/html, Size: 579 bytes --]