unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* 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).