* bug#5358: 23.1; Adding comment loses customizations in progress
@ 2010-01-11 19:41 David Abrahams
2016-02-17 3:48 ` Andrew Hyatt
2019-09-06 14:47 ` Mauro Aranda
0 siblings, 2 replies; 10+ messages in thread
From: David Abrahams @ 2010-01-11 19:41 UTC (permalink / raw)
To: bug-gnu-emacs
Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.
Your bug report will be posted to the bug-gnu-emacs@gnu.org mailing list,
and to the gnu.emacs.bug news group.
Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:
Using the customization interface, I can make a change to a variable,
then try to add a comment before saving. When I do that, the changes
I've already made are lost.
If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
`bt full' and `xbacktrace'.
If you would like to further debug the crash, please read the file
/Applications/Emacs.app/Contents/Resources/etc/DEBUG for instructions.
In GNU Emacs 23.1.1 (i386-apple-darwin9.8.0, NS apple-appkit-949.54)
of 2009-08-16 on black.local
Windowing system distributor `Apple', version 10.3.1038
configured using `configure '--with-ns''
Important settings:
value of $LC_ALL: nil
value of $LC_COLLATE: nil
value of $LC_CTYPE: nil
value of $LC_MESSAGES: nil
value of $LC_MONETARY: nil
value of $LC_NUMERIC: nil
value of $LC_TIME: nil
value of $LANG: nil
value of $XMODIFIERS: nil
locale-coding-system: nil
default-enable-multibyte-characters: t
Major mode: Custom
Minor modes in effect:
diff-auto-refine-mode: t
shell-dirtrack-mode: t
show-paren-mode: t
server-mode: t
global-auto-revert-mode: t
delete-selection-mode: t
tooltip-mode: t
mouse-wheel-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
global-auto-composition-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t
Recent input:
C-n C-n C-p C-p C-. C-. C-. C-b C-b <return> C-n C-n
C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n
C-e <backspace> 0 C-n C-n C-n C-n C-n C-n C-n C-n C-n
C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-n C-n C-n C-n C-n C-n C-p C-p C-p C-p C-n
C-n C-n C-n C-n C-n C-n C-n C-n C-p C-n C-n C-n C-n
C-n C-n C-n C-n C-n C-n C-n C-a C-p C-. C-. C-, <return>
7 C-n C-n C-n C-e M-< C-n C-n C-n C-n C-n C-n C-n C-n
C-n C-n C-n C-n C-n C-n C-n C-n C-e C-p C-e <backspace>
0 C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-x C-s C-n C-n C-n C-n
C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n
C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p <M-return>
<C-return> C-q C-j I SPC r e a l l y SPC d o n ' t
SPC w a n t SPC t o SPC s e t e <backspace> <backspace>
e SPC H T M L S-SPC u n l e s s SPC i <backspace> t
h a t ' s SPC a l l SPC t h e r e SPC i s . <backspace>
C-c C-c C-x C-s C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p M-x r e p o <tab> r <tab> b <tab> <return>
Recent messages:
Mark set [2 times]
Saving file /Users/dave/src/elisp.repo/elisp/custom.el...
Wrote /Users/dave/src/elisp.repo/elisp/custom.el
Compiling /Users/dave/src/elisp.repo/elisp/custom.el...done
Wrote /Users/dave/src/elisp.repo/elisp/custom.elc
Saving file /Users/dave/src/elisp.repo/elisp/custom.el...
Wrote /Users/dave/src/elisp.repo/elisp/custom.el
Compiling /Users/dave/src/elisp.repo/elisp/custom.el...done
Wrote /Users/dave/src/elisp.repo/elisp/custom.elc
Making completion list...
--
Dave Abrahams Meet me at BoostCon: http://www.boostcon.com
BoostPro Computing
http://www.boostpro.com
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#5358: 23.1; Adding comment loses customizations in progress 2010-01-11 19:41 bug#5358: 23.1; Adding comment loses customizations in progress David Abrahams @ 2016-02-17 3:48 ` Andrew Hyatt 2016-02-18 15:29 ` Dave Abrahams 2019-09-06 14:47 ` Mauro Aranda 1 sibling, 1 reply; 10+ messages in thread From: Andrew Hyatt @ 2016-02-17 3:48 UTC (permalink / raw) To: David Abrahams; +Cc: 5358 Sorry, I don't understand this bug report. What do you mean by adding comments? David Abrahams <dave@boostpro.com> writes: > Please write in English if possible, because the Emacs maintainers > usually do not have translators to read other languages for them. > > Your bug report will be posted to the bug-gnu-emacs@gnu.org mailing list, > and to the gnu.emacs.bug news group. > > Please describe exactly what actions triggered the bug > and the precise symptoms of the bug: > > Using the customization interface, I can make a change to a variable, > then try to add a comment before saving. When I do that, the changes > I've already made are lost. > > If Emacs crashed, and you have the Emacs process in the gdb debugger, > please include the output from the following gdb commands: > `bt full' and `xbacktrace'. > If you would like to further debug the crash, please read the file > /Applications/Emacs.app/Contents/Resources/etc/DEBUG for instructions. > > > In GNU Emacs 23.1.1 (i386-apple-darwin9.8.0, NS apple-appkit-949.54) > of 2009-08-16 on black.local > Windowing system distributor `Apple', version 10.3.1038 > configured using `configure '--with-ns'' > > Important settings: > value of $LC_ALL: nil > value of $LC_COLLATE: nil > value of $LC_CTYPE: nil > value of $LC_MESSAGES: nil > value of $LC_MONETARY: nil > value of $LC_NUMERIC: nil > value of $LC_TIME: nil > value of $LANG: nil > value of $XMODIFIERS: nil > locale-coding-system: nil > default-enable-multibyte-characters: t > > Major mode: Custom > > Minor modes in effect: > diff-auto-refine-mode: t > shell-dirtrack-mode: t > show-paren-mode: t > server-mode: t > global-auto-revert-mode: t > delete-selection-mode: t > tooltip-mode: t > mouse-wheel-mode: t > menu-bar-mode: t > file-name-shadow-mode: t > global-font-lock-mode: t > font-lock-mode: t > global-auto-composition-mode: t > auto-composition-mode: t > auto-encryption-mode: t > auto-compression-mode: t > line-number-mode: t > transient-mark-mode: t > > Recent input: > C-n C-n C-p C-p C-. C-. C-. C-b C-b <return> C-n C-n > C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n > C-e <backspace> 0 C-n C-n C-n C-n C-n C-n C-n C-n C-n > C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n > C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p > C-p C-p C-p C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p > C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p > C-p C-p C-n C-n C-n C-n C-n C-n C-p C-p C-p C-p C-n > C-n C-n C-n C-n C-n C-n C-n C-n C-p C-n C-n C-n C-n > C-n C-n C-n C-n C-n C-n C-n C-a C-p C-. C-. C-, <return> > 7 C-n C-n C-n C-e M-< C-n C-n C-n C-n C-n C-n C-n C-n > C-n C-n C-n C-n C-n C-n C-n C-n C-e C-p C-e <backspace> > 0 C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p C-p C-p C-p > C-p C-p C-p C-p C-p C-p C-p C-x C-s C-n C-n C-n C-n > C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n > C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p <M-return> > <C-return> C-q C-j I SPC r e a l l y SPC d o n ' t > SPC w a n t SPC t o SPC s e t e <backspace> <backspace> > e SPC H T M L S-SPC u n l e s s SPC i <backspace> t > h a t ' s SPC a l l SPC t h e r e SPC i s . <backspace> > C-c C-c C-x C-s C-p C-p C-p C-p C-p C-p C-p C-p C-p > C-p C-p M-x r e p o <tab> r <tab> b <tab> <return> > > Recent messages: > Mark set [2 times] > Saving file /Users/dave/src/elisp.repo/elisp/custom.el... > Wrote /Users/dave/src/elisp.repo/elisp/custom.el > Compiling /Users/dave/src/elisp.repo/elisp/custom.el...done > Wrote /Users/dave/src/elisp.repo/elisp/custom.elc > Saving file /Users/dave/src/elisp.repo/elisp/custom.el... > Wrote /Users/dave/src/elisp.repo/elisp/custom.el > Compiling /Users/dave/src/elisp.repo/elisp/custom.el...done > Wrote /Users/dave/src/elisp.repo/elisp/custom.elc > Making completion list... ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2016-02-17 3:48 ` Andrew Hyatt @ 2016-02-18 15:29 ` Dave Abrahams 2016-02-18 16:47 ` Glenn Morris 0 siblings, 1 reply; 10+ messages in thread From: Dave Abrahams @ 2016-02-18 15:29 UTC (permalink / raw) To: Andrew Hyatt; +Cc: 5358 There's an choice in the customization UI on emacsen < 24.5 that lets you add a comment to any customization, but now I don't see it. Apparently that feature was dropped :( on Tue Feb 16 2016, Andrew Hyatt <ahyatt-AT-gmail.com> wrote: > Sorry, I don't understand this bug report. What do you mean by adding > comments? > > David Abrahams <dave@boostpro.com> writes: > >> Please write in English if possible, because the Emacs maintainers >> usually do not have translators to read other languages for them. >> >> Your bug report will be posted to the bug-gnu-emacs@gnu.org mailing list, >> and to the gnu.emacs.bug news group. >> >> Please describe exactly what actions triggered the bug >> and the precise symptoms of the bug: >> >> Using the customization interface, I can make a change to a variable, >> then try to add a comment before saving. When I do that, the changes >> I've already made are lost. >> >> If Emacs crashed, and you have the Emacs process in the gdb debugger, >> please include the output from the following gdb commands: >> `bt full' and `xbacktrace'. >> If you would like to further debug the crash, please read the file >> /Applications/Emacs.app/Contents/Resources/etc/DEBUG for instructions. >> >> >> In GNU Emacs 23.1.1 (i386-apple-darwin9.8.0, NS apple-appkit-949.54) >> of 2009-08-16 on black.local >> Windowing system distributor `Apple', version 10.3.1038 >> configured using `configure '--with-ns'' >> >> Important settings: >> value of $LC_ALL: nil >> value of $LC_COLLATE: nil >> value of $LC_CTYPE: nil >> value of $LC_MESSAGES: nil >> value of $LC_MONETARY: nil >> value of $LC_NUMERIC: nil >> value of $LC_TIME: nil >> value of $LANG: nil >> value of $XMODIFIERS: nil >> locale-coding-system: nil >> default-enable-multibyte-characters: t >> >> Major mode: Custom >> >> Minor modes in effect: >> diff-auto-refine-mode: t >> shell-dirtrack-mode: t >> show-paren-mode: t >> server-mode: t >> global-auto-revert-mode: t >> delete-selection-mode: t >> tooltip-mode: t >> mouse-wheel-mode: t >> menu-bar-mode: t >> file-name-shadow-mode: t >> global-font-lock-mode: t >> font-lock-mode: t >> global-auto-composition-mode: t >> auto-composition-mode: t >> auto-encryption-mode: t >> auto-compression-mode: t >> line-number-mode: t >> transient-mark-mode: t >> >> Recent input: >> C-n C-n C-p C-p C-. C-. C-. C-b C-b <return> C-n C-n >> C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n >> C-e <backspace> 0 C-n C-n C-n C-n C-n C-n C-n C-n C-n >> C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n >> C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p >> C-p C-p C-p C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p >> C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p >> C-p C-p C-n C-n C-n C-n C-n C-n C-p C-p C-p C-p C-n >> C-n C-n C-n C-n C-n C-n C-n C-n C-p C-n C-n C-n C-n >> C-n C-n C-n C-n C-n C-n C-n C-a C-p C-. C-. C-, <return> >> 7 C-n C-n C-n C-e M-< C-n C-n C-n C-n C-n C-n C-n C-n >> C-n C-n C-n C-n C-n C-n C-n C-n C-e C-p C-e <backspace> >> 0 C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p C-p C-p C-p >> C-p C-p C-p C-p C-p C-p C-p C-x C-s C-n C-n C-n C-n >> C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n >> C-n C-n C-n C-n C-n C-n C-n C-n C-n C-p <M-return> >> <C-return> C-q C-j I SPC r e a l l y SPC d o n ' t >> SPC w a n t SPC t o SPC s e t e <backspace> <backspace> >> e SPC H T M L S-SPC u n l e s s SPC i <backspace> t >> h a t ' s SPC a l l SPC t h e r e SPC i s . <backspace> >> C-c C-c C-x C-s C-p C-p C-p C-p C-p C-p C-p C-p C-p >> C-p C-p M-x r e p o <tab> r <tab> b <tab> <return> >> >> Recent messages: >> Mark set [2 times] >> Saving file /Users/dave/src/elisp.repo/elisp/custom.el... >> Wrote /Users/dave/src/elisp.repo/elisp/custom.el >> Compiling /Users/dave/src/elisp.repo/elisp/custom.el...done >> Wrote /Users/dave/src/elisp.repo/elisp/custom.elc >> Saving file /Users/dave/src/elisp.repo/elisp/custom.el... >> Wrote /Users/dave/src/elisp.repo/elisp/custom.el >> Compiling /Users/dave/src/elisp.repo/elisp/custom.el...done >> Wrote /Users/dave/src/elisp.repo/elisp/custom.elc >> Making completion list... -- -Dave ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2016-02-18 15:29 ` Dave Abrahams @ 2016-02-18 16:47 ` Glenn Morris 2016-02-19 16:18 ` Andrew Hyatt 0 siblings, 1 reply; 10+ messages in thread From: Glenn Morris @ 2016-02-18 16:47 UTC (permalink / raw) To: Dave Abrahams; +Cc: Andrew Hyatt, 5358 ? M-x customize-variable RET fill-column RET Click "State", see "Add Comment". (Current master.) ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2016-02-18 16:47 ` Glenn Morris @ 2016-02-19 16:18 ` Andrew Hyatt 0 siblings, 0 replies; 10+ messages in thread From: Andrew Hyatt @ 2016-02-19 16:18 UTC (permalink / raw) To: Glenn Morris; +Cc: Dave Abrahams, 5358 Thanks, now I see it. Now that I see it, I was able to reproduce this bug on Emacs 25. Glenn Morris <rgm@gnu.org> writes: > ? > > M-x customize-variable RET fill-column RET > Click "State", see "Add Comment". > > (Current master.) ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2010-01-11 19:41 bug#5358: 23.1; Adding comment loses customizations in progress David Abrahams 2016-02-17 3:48 ` Andrew Hyatt @ 2019-09-06 14:47 ` Mauro Aranda 2019-09-27 12:31 ` Mauro Aranda 1 sibling, 1 reply; 10+ messages in thread From: Mauro Aranda @ 2019-09-06 14:47 UTC (permalink / raw) To: 5358; +Cc: dave, ahyatt [-- Attachment #1.1: Type: text/plain, Size: 960 bytes --] The possibility to preserve the customizations in progress is already present in the customize machinery, but for some reason it wasn't used in this case. I propose a patch that makes use of the :shown-value property of the custom-variable and custom-face widgets, thus allowing to preserve the customizations in progress when redrawing a widget. Since after redrawing customize recomputes the state of the widget, it is necessary to check if the widget was modified. That isn't possible with `custom-variable-state' or `custom-face-state', because they are pretty "widget unaware". So I added a check in `custom-variable-state-set' and `custom-face-state-set' to catch this situation. Finally, to be able to use the value of the widget in `custom-face-state-set', it is necessary to rearrange the final part of `custom-face-value-create'. Namely, add the children to the :children property, before calling `custom-face-state-set'. Best regards, Mauro. [-- Attachment #1.2: Type: text/html, Size: 1094 bytes --] [-- Attachment #2: 0001-Don-t-discard-customizations-in-progress-when-adding.patch --] [-- Type: text/x-patch, Size: 5155 bytes --] From d2656c7da97acf689ec12f1fde496e608d8e90fb Mon Sep 17 00:00:00 2001 From: Mauro Aranda <maurooaranda@gmail.com> Date: Fri, 6 Sep 2019 09:55:39 -0300 Subject: [PATCH] Don't discard customizations in progress when adding comments (Bug#5358) * lisp/cus-edit.el (custom-comment-show): Add docstring. Save the widget value in the :shown-value property, before redrawing. (custom-variable-modified-p): New function, to complement the return values of custom-variable-state. (custom-variable-state-set): Use it. (custom-face-value-create): Add children to the custom-face widget before setting the state, to be able to check for user edits. (custom-face-state-set): Check for user edits before calling custom-face-state. --- lisp/cus-edit.el | 63 +++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el index 8a8bad9..60e6996 100644 --- a/lisp/cus-edit.el +++ b/lisp/cus-edit.el @@ -2411,9 +2411,21 @@ custom-comment-hide ;; Those functions are for the menu. WIDGET is NOT the comment widget. It's ;; the global custom one (defun custom-comment-show (widget) - (widget-put widget :comment-shown t) - (custom-redraw widget) - (widget-setup)) + "Show the comment editable field that belongs to WIDGET." + (let ((child (car (widget-get widget :children))) + ;; Just to be safe, we will restore this value after redrawing. + (old-shown-value (widget-get widget :shown-value))) + (widget-put widget :comment-shown t) + ;; Save the changes made by the user before redrawing, to avoid + ;; losing customizations in progress. (Bug#5358) + (if (eq (widget-type widget) 'custom-face) + (if (eq (widget-type child) 'custom-face-edit) + (widget-put widget :shown-value `((t ,(widget-value child)))) + (widget-put widget :shown-value (widget-value child))) + (widget-put widget :shown-value (list (widget-value child)))) + (custom-redraw widget) + (widget-put widget :shown-value old-shown-value) + (widget-setup))) (defun custom-comment-invisible-p (widget) (let ((val (widget-value (widget-get widget :comment-widget)))) @@ -2805,12 +2817,34 @@ custom-variable-state 'changed)) (t 'rogue)))) +(defun custom-variable-modified-p (widget) + "Non-nil if the variable value of WIDGET has been modified. +WIDGET should be a custom-variable widget, whose first child is the widget +that holds the value. +Modified means that the widget that holds the value has been edited by the user +in a customize buffer. +To check for other states, call `custom-variable-state'." + (condition-case nil + (let* ((symbol (widget-get widget :value)) + (get (or (get symbol 'custom-get) 'default-value)) + (value (if (default-boundp symbol) + (funcall get symbol) + (symbol-value symbol)))) + (not (equal value (widget-value (car (widget-get widget :children)))))) + (error t))) + (defun custom-variable-state-set (widget &optional state) "Set the state of WIDGET to STATE. -If STATE is nil, the value is computed by `custom-variable-state'." +If STATE is nil, the new state is computed by `custom-variable-modified-p' if +WIDGET has been edited in the Custom buffer, or by `custom-variable-state' +otherwise." (widget-put widget :custom-state - (or state (custom-variable-state (widget-value widget) - (widget-get widget :value))))) + (or state + (and (custom-variable-modified-p widget) 'modified) + (custom-variable-state (widget-value widget) + (widget-value + (car + (widget-get widget :children))))))) (defun custom-variable-standard-value (widget) (get (widget-value widget) 'standard-value)) @@ -3630,9 +3664,9 @@ custom-face-value-create (insert-char ?\s indent)) (widget-create-child-and-convert widget 'sexp :value spec)))) - (custom-face-state-set widget) - (push editor children) - (widget-put widget :children children)))))) + (push editor children) + (widget-put widget :children children) + (custom-face-state-set widget)))))) (defvar custom-face-menu `(("Set for Current Session" custom-face-set) @@ -3718,9 +3752,14 @@ custom-face-state state))) (defun custom-face-state-set (widget) - "Set the state of WIDGET." - (widget-put widget :custom-state - (custom-face-state (widget-value widget)))) + "Set the state of WIDGET, a custom-face widget. +If the user edited the widget, set the state to modified. If not, the new +state is one of the return values of `custom-face-state'." + (let ((face (widget-value widget))) + (widget-put widget :custom-state + (if (face-spec-match-p face (custom-face-widget-to-spec widget)) + (custom-face-state face) + 'modified)))) (defun custom-face-action (widget &optional event) "Show the menu for `custom-face' WIDGET. -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2019-09-06 14:47 ` Mauro Aranda @ 2019-09-27 12:31 ` Mauro Aranda 2019-09-27 14:09 ` Lars Ingebrigtsen 0 siblings, 1 reply; 10+ messages in thread From: Mauro Aranda @ 2019-09-27 12:31 UTC (permalink / raw) To: 5358; +Cc: Dave Abrahams, ahyatt [-- Attachment #1.1: Type: text/plain, Size: 54 bytes --] I added a test for this change. Best regards, Mauro. [-- Attachment #1.2: Type: text/html, Size: 124 bytes --] [-- Attachment #2: 0001-Don-t-discard-customizations-in-progress-when-adding.patch --] [-- Type: text/x-patch, Size: 7124 bytes --] From 69ec406a67b11d2398ce795b31db7a0ae4cb479c Mon Sep 17 00:00:00 2001 From: Mauro Aranda <maurooaranda@gmail.com> Date: Fri, 6 Sep 2019 09:55:39 -0300 Subject: [PATCH] Don't discard customizations in progress when adding comments (Bug#5358) * lisp/cus-edit.el (custom-comment-show): Add docstring. Save the widget value in the :shown-value property, before redrawing. (custom-variable-modified-p): New function, to complement the return values of custom-variable-state. (custom-variable-state-set): Use it. (custom-face-value-create): Add children to the custom-face widget before setting the state, to be able to check for user edits. (custom-face-state-set): Check for user edits before calling custom-face-state. * test/lisp/custom-tests.el (custom-test-show-comment-preserves-changes): New test. --- lisp/cus-edit.el | 63 ++++++++++++++++++++++++++++++++++++++--------- test/lisp/custom-tests.el | 28 +++++++++++++++++++++ 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el index 2496963..f3445c6 100644 --- a/lisp/cus-edit.el +++ b/lisp/cus-edit.el @@ -2416,9 +2416,21 @@ custom-comment-hide ;; Those functions are for the menu. WIDGET is NOT the comment widget. It's ;; the global custom one (defun custom-comment-show (widget) - (widget-put widget :comment-shown t) - (custom-redraw widget) - (widget-setup)) + "Show the comment editable field that belongs to WIDGET." + (let ((child (car (widget-get widget :children))) + ;; Just to be safe, we will restore this value after redrawing. + (old-shown-value (widget-get widget :shown-value))) + (widget-put widget :comment-shown t) + ;; Save the changes made by the user before redrawing, to avoid + ;; losing customizations in progress. (Bug#5358) + (if (eq (widget-type widget) 'custom-face) + (if (eq (widget-type child) 'custom-face-edit) + (widget-put widget :shown-value `((t ,(widget-value child)))) + (widget-put widget :shown-value (widget-value child))) + (widget-put widget :shown-value (list (widget-value child)))) + (custom-redraw widget) + (widget-put widget :shown-value old-shown-value) + (widget-setup))) (defun custom-comment-invisible-p (widget) (let ((val (widget-value (widget-get widget :comment-widget)))) @@ -2810,12 +2822,34 @@ custom-variable-state 'changed)) (t 'rogue)))) +(defun custom-variable-modified-p (widget) + "Non-nil if the variable value of WIDGET has been modified. +WIDGET should be a custom-variable widget, whose first child is the widget +that holds the value. +Modified means that the widget that holds the value has been edited by the user +in a customize buffer. +To check for other states, call `custom-variable-state'." + (condition-case nil + (let* ((symbol (widget-get widget :value)) + (get (or (get symbol 'custom-get) 'default-value)) + (value (if (default-boundp symbol) + (funcall get symbol) + (symbol-value symbol)))) + (not (equal value (widget-value (car (widget-get widget :children)))))) + (error t))) + (defun custom-variable-state-set (widget &optional state) "Set the state of WIDGET to STATE. -If STATE is nil, the value is computed by `custom-variable-state'." +If STATE is nil, the new state is computed by `custom-variable-modified-p' if +WIDGET has been edited in the Custom buffer, or by `custom-variable-state' +otherwise." (widget-put widget :custom-state - (or state (custom-variable-state (widget-value widget) - (widget-get widget :value))))) + (or state + (and (custom-variable-modified-p widget) 'modified) + (custom-variable-state (widget-value widget) + (widget-value + (car + (widget-get widget :children))))))) (defun custom-variable-standard-value (widget) (get (widget-value widget) 'standard-value)) @@ -3635,9 +3669,9 @@ custom-face-value-create (insert-char ?\s indent)) (widget-create-child-and-convert widget 'sexp :value spec)))) - (custom-face-state-set widget) - (push editor children) - (widget-put widget :children children)))))) + (push editor children) + (widget-put widget :children children) + (custom-face-state-set widget)))))) (defvar custom-face-menu `(("Set for Current Session" custom-face-set) @@ -3723,9 +3757,14 @@ custom-face-state state))) (defun custom-face-state-set (widget) - "Set the state of WIDGET." - (widget-put widget :custom-state - (custom-face-state (widget-value widget)))) + "Set the state of WIDGET, a custom-face widget. +If the user edited the widget, set the state to modified. If not, the new +state is one of the return values of `custom-face-state'." + (let ((face (widget-value widget))) + (widget-put widget :custom-state + (if (face-spec-match-p face (custom-face-widget-to-spec widget)) + (custom-face-state face) + 'modified)))) (defun custom-face-action (widget &optional event) "Show the menu for `custom-face' WIDGET. diff --git a/test/lisp/custom-tests.el b/test/lisp/custom-tests.el index 0c49db6..270acda 100644 --- a/test/lisp/custom-tests.el +++ b/test/lisp/custom-tests.el @@ -21,6 +21,10 @@ (require 'ert) +(require 'wid-edit) +(require 'cus-edit) +(require 'seq) ; For `seq-find'. + (ert-deftest custom-theme--load-path () "Test `custom-theme--load-path' behavior." (let ((tmpdir (file-name-as-directory (make-temp-file "custom-tests-" t)))) @@ -123,4 +127,28 @@ custom--test-variable (should (equal custom--test-user-option 'baz)) (should (equal custom--test-variable 'baz)))) +;; This tests Bug#5358. +(ert-deftest custom-test-show-comment-preserves-changes () + "Test that adding a comment doesn't discard modifications in progress." + (customize-option 'custom--test-user-option) + (let* ((field (seq-find (lambda (widget) + (eq custom--test-user-option (widget-value widget))) + widget-field-list)) + (parent (widget-get field :parent)) + (origvalue (widget-value field))) + ;; Move to the end of the text of the widget, and modify it. This + ;; modification should be preserved after showing the comment field. + (goto-char (widget-field-text-end field)) + (insert "bar") + (custom-comment-show parent) + ;; From now on, must use `widget-at' to get the value of the widget. + (should-not (eq origvalue (widget-value (widget-at)))) + (should (eq (widget-get parent :custom-state) 'modified)) + (should (eq (widget-value (widget-at)) + (widget-apply field + :value-to-external + (concat + (widget-apply field :value-to-internal origvalue) + "bar")))))) + ;;; custom-tests.el ends here -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2019-09-27 12:31 ` Mauro Aranda @ 2019-09-27 14:09 ` Lars Ingebrigtsen 2019-09-27 15:04 ` Mauro Aranda 0 siblings, 1 reply; 10+ messages in thread From: Lars Ingebrigtsen @ 2019-09-27 14:09 UTC (permalink / raw) To: Mauro Aranda; +Cc: Dave Abrahams, ahyatt, 5358 Mauro Aranda <maurooaranda@gmail.com> writes: > Subject: [PATCH] Don't discard customizations in progress when adding comments > (Bug#5358) Looks good to me. Just one tiny comment: > + (condition-case nil > + (let* ((symbol (widget-get widget :value)) > + (get (or (get symbol 'custom-get) 'default-value)) > + (value (if (default-boundp symbol) > + (funcall get symbol) > + (symbol-value symbol)))) > + (not (equal value (widget-value (car (widget-get widget :children)))))) > + (error t))) If it's just the funcall you expect that might fail, then moving the condition-case down there might be a better choice. Having a condition-case around code that shouldn't fail can hide errors you don't want to hide. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2019-09-27 14:09 ` Lars Ingebrigtsen @ 2019-09-27 15:04 ` Mauro Aranda 2019-09-27 16:07 ` Lars Ingebrigtsen 0 siblings, 1 reply; 10+ messages in thread From: Mauro Aranda @ 2019-09-27 15:04 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Dave Abrahams, ahyatt, 5358 [-- Attachment #1.1: Type: text/plain, Size: 1089 bytes --] Lars Ingebrigtsen <larsi@gnus.org> writes: > Mauro Aranda <maurooaranda@gmail.com> writes: > >> Subject: [PATCH] Don't discard customizations in progress when adding comments >> (Bug#5358) > > Looks good to me. Just one tiny comment: > >> + (condition-case nil >> + (let* ((symbol (widget-get widget :value)) >> + (get (or (get symbol 'custom-get) 'default-value)) >> + (value (if (default-boundp symbol) >> + (funcall get symbol) >> + (symbol-value symbol)))) >> + (not (equal value (widget-value (car (widget-get widget :children)))))) >> + (error t))) > > If it's just the funcall you expect that might fail, then moving the > condition-case down there might be a better choice. Having a > condition-case around code that shouldn't fail can hide errors you don't > want to hide. Right, thanks for noticing that. I've moved down the condition-case, and I've added an outer catch, to avoid the comparison in case of an error. I think that's OK, but if you see something wrong, please let me know. [-- Attachment #1.2: Type: text/html, Size: 1459 bytes --] [-- Attachment #2: 0001-Don-t-discard-customizations-in-progress-when-adding.patch --] [-- Type: text/x-patch, Size: 7197 bytes --] From 39334b56d71ae309e9deba1568c2b21347c519e0 Mon Sep 17 00:00:00 2001 From: Mauro Aranda <maurooaranda@gmail.com> Date: Fri, 6 Sep 2019 09:55:39 -0300 Subject: [PATCH] Don't discard customizations in progress when adding comments (Bug#5358) * lisp/cus-edit.el (custom-comment-show): Add docstring. Save the widget value in the :shown-value property, before redrawing. (custom-variable-modified-p): New function, to complement the return values of custom-variable-state. (custom-variable-state-set): Use it. (custom-face-value-create): Add children to the custom-face widget before setting the state, to be able to check for user edits. (custom-face-state-set): Check for user edits before calling custom-face-state. * test/lisp/custom-tests.el (custom-test-show-comment-preserves-changes): New test. --- lisp/cus-edit.el | 64 ++++++++++++++++++++++++++++++++++++++--------- test/lisp/custom-tests.el | 28 +++++++++++++++++++++ 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el index 2496963..b9fd3e0 100644 --- a/lisp/cus-edit.el +++ b/lisp/cus-edit.el @@ -2416,9 +2416,21 @@ custom-comment-hide ;; Those functions are for the menu. WIDGET is NOT the comment widget. It's ;; the global custom one (defun custom-comment-show (widget) - (widget-put widget :comment-shown t) - (custom-redraw widget) - (widget-setup)) + "Show the comment editable field that belongs to WIDGET." + (let ((child (car (widget-get widget :children))) + ;; Just to be safe, we will restore this value after redrawing. + (old-shown-value (widget-get widget :shown-value))) + (widget-put widget :comment-shown t) + ;; Save the changes made by the user before redrawing, to avoid + ;; losing customizations in progress. (Bug#5358) + (if (eq (widget-type widget) 'custom-face) + (if (eq (widget-type child) 'custom-face-edit) + (widget-put widget :shown-value `((t ,(widget-value child)))) + (widget-put widget :shown-value (widget-value child))) + (widget-put widget :shown-value (list (widget-value child)))) + (custom-redraw widget) + (widget-put widget :shown-value old-shown-value) + (widget-setup))) (defun custom-comment-invisible-p (widget) (let ((val (widget-value (widget-get widget :comment-widget)))) @@ -2810,12 +2822,35 @@ custom-variable-state 'changed)) (t 'rogue)))) +(defun custom-variable-modified-p (widget) + "Non-nil if the variable value of WIDGET has been modified. +WIDGET should be a custom-variable widget, whose first child is the widget +that holds the value. +Modified means that the widget that holds the value has been edited by the user +in a customize buffer. +To check for other states, call `custom-variable-state'." + (catch 'get-error + (let* ((symbol (widget-get widget :value)) + (get (or (get symbol 'custom-get) 'default-value)) + (value (if (default-boundp symbol) + (condition-case nil + (funcall get symbol) + (error (throw 'get-error t))) + (symbol-value symbol)))) + (not (equal value (widget-value (car (widget-get widget :children)))))))) + (defun custom-variable-state-set (widget &optional state) "Set the state of WIDGET to STATE. -If STATE is nil, the value is computed by `custom-variable-state'." +If STATE is nil, the new state is computed by `custom-variable-modified-p' if +WIDGET has been edited in the Custom buffer, or by `custom-variable-state' +otherwise." (widget-put widget :custom-state - (or state (custom-variable-state (widget-value widget) - (widget-get widget :value))))) + (or state + (and (custom-variable-modified-p widget) 'modified) + (custom-variable-state (widget-value widget) + (widget-value + (car + (widget-get widget :children))))))) (defun custom-variable-standard-value (widget) (get (widget-value widget) 'standard-value)) @@ -3635,9 +3670,9 @@ custom-face-value-create (insert-char ?\s indent)) (widget-create-child-and-convert widget 'sexp :value spec)))) - (custom-face-state-set widget) - (push editor children) - (widget-put widget :children children)))))) + (push editor children) + (widget-put widget :children children) + (custom-face-state-set widget)))))) (defvar custom-face-menu `(("Set for Current Session" custom-face-set) @@ -3723,9 +3758,14 @@ custom-face-state state))) (defun custom-face-state-set (widget) - "Set the state of WIDGET." - (widget-put widget :custom-state - (custom-face-state (widget-value widget)))) + "Set the state of WIDGET, a custom-face widget. +If the user edited the widget, set the state to modified. If not, the new +state is one of the return values of `custom-face-state'." + (let ((face (widget-value widget))) + (widget-put widget :custom-state + (if (face-spec-match-p face (custom-face-widget-to-spec widget)) + (custom-face-state face) + 'modified)))) (defun custom-face-action (widget &optional event) "Show the menu for `custom-face' WIDGET. diff --git a/test/lisp/custom-tests.el b/test/lisp/custom-tests.el index 0c49db6..270acda 100644 --- a/test/lisp/custom-tests.el +++ b/test/lisp/custom-tests.el @@ -21,6 +21,10 @@ (require 'ert) +(require 'wid-edit) +(require 'cus-edit) +(require 'seq) ; For `seq-find'. + (ert-deftest custom-theme--load-path () "Test `custom-theme--load-path' behavior." (let ((tmpdir (file-name-as-directory (make-temp-file "custom-tests-" t)))) @@ -123,4 +127,28 @@ custom--test-variable (should (equal custom--test-user-option 'baz)) (should (equal custom--test-variable 'baz)))) +;; This tests Bug#5358. +(ert-deftest custom-test-show-comment-preserves-changes () + "Test that adding a comment doesn't discard modifications in progress." + (customize-option 'custom--test-user-option) + (let* ((field (seq-find (lambda (widget) + (eq custom--test-user-option (widget-value widget))) + widget-field-list)) + (parent (widget-get field :parent)) + (origvalue (widget-value field))) + ;; Move to the end of the text of the widget, and modify it. This + ;; modification should be preserved after showing the comment field. + (goto-char (widget-field-text-end field)) + (insert "bar") + (custom-comment-show parent) + ;; From now on, must use `widget-at' to get the value of the widget. + (should-not (eq origvalue (widget-value (widget-at)))) + (should (eq (widget-get parent :custom-state) 'modified)) + (should (eq (widget-value (widget-at)) + (widget-apply field + :value-to-external + (concat + (widget-apply field :value-to-internal origvalue) + "bar")))))) + ;;; custom-tests.el ends here -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#5358: 23.1; Adding comment loses customizations in progress 2019-09-27 15:04 ` Mauro Aranda @ 2019-09-27 16:07 ` Lars Ingebrigtsen 0 siblings, 0 replies; 10+ messages in thread From: Lars Ingebrigtsen @ 2019-09-27 16:07 UTC (permalink / raw) To: Mauro Aranda; +Cc: Dave Abrahams, ahyatt, 5358 Mauro Aranda <maurooaranda@gmail.com> writes: > I've moved down the condition-case, and I've added an outer catch, to > avoid the comparison in case of an error. I think that's OK, but if you > see something wrong, please let me know. Looks good to me, so I've applied it to the trunk now. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-09-27 16:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-11 19:41 bug#5358: 23.1; Adding comment loses customizations in progress David Abrahams 2016-02-17 3:48 ` Andrew Hyatt 2016-02-18 15:29 ` Dave Abrahams 2016-02-18 16:47 ` Glenn Morris 2016-02-19 16:18 ` Andrew Hyatt 2019-09-06 14:47 ` Mauro Aranda 2019-09-27 12:31 ` Mauro Aranda 2019-09-27 14:09 ` Lars Ingebrigtsen 2019-09-27 15:04 ` Mauro Aranda 2019-09-27 16:07 ` 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).