From: Mauro Aranda <maurooaranda@gmail.com>
To: Thomas Fitzsimmons <fitzsim@fitzsim.org>
Cc: 63290@debbugs.gnu.org
Subject: bug#63290: 30.0.50; Customize UI shows extra fields for (choice (const ...) (alist ...))
Date: Tue, 15 Aug 2023 19:46:35 -0300 [thread overview]
Message-ID: <08306097-a6d2-ffbe-0991-31afbb71c037@gmail.com> (raw)
In-Reply-To: <m3cyztso5n.fsf@fitzsim.org>
[-- Attachment #1: Type: text/plain, Size: 1694 bytes --]
Hi Thomas,
Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
> Mauro Aranda <maurooaranda@gmail.com> writes:
>>> Can you try this patch with: M-x package-install RET excorporate
>>> RET Then: M-x customize-variable RET excorporate-configuration RET
>>> then select "Value Menu" and 3, which is "EWS URL OAuth 2.0
>>> settings (no autodiscovery)". With your wis-edit.el patch applied
>>> I still get empty values for: INS DEL Argument
>>> name: Argument value: and: INS DEL OAuth 2.0
>>> setting name: OAuth 2.0 setting value: and when I
>>> apply the setting the value contains: (... (... (#1# . #1#))
>>> (#1# . #1#)) Maybe this is a more complicated case than the test
>>> case I provided (which does now work for me with your patch)?
>> I think this ammended patch fixes it. Since we want
>> widget-list-default-get to respect a nil :value property, the alist
>> widget needs to be modified so that its default value is nil.
>
> With the updated patch, when I select "EWS URL OAuth 2.0 settings (no
> autodiscovery)", all the widgets are disabled. The blank values are
> no longer added though. However, if I then set the value, without
> configuring anything, excorporate-configuration stays nil. So I don't
> think the patch is correct yet.
So, in case the :value is missing for the alist widget, we want to
compute the default-value with the :options, and without including the
editable-list.
Hopefully the attached patch is 100% correct now. I did try it with
excorporate-configuration and I think it works OK, but please give it
yourself a try. And thank you for your patience.
[-- Attachment #2: 0001-Specialize-default-get-for-alist-widgets-Bug-63290.patch --]
[-- Type: text/x-patch, Size: 7430 bytes --]
From 228001795ab15936cd2e8d1fec38ec15b77c5b91 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Tue, 15 Aug 2023 19:35:39 -0300
Subject: [PATCH] Specialize default-get for alist widgets (Bug#63290)
* lisp/wid-edit.el (widget-list-default-get)
(widget-alist-default-get): New functions.
(list, alist): Use it.
* test/lisp/cus-edit-tests.el (cus-edit-test-bug63290-option)
(cus-edit-test-bug63290-option-2): New test options.
(cus-edit-test-bug63290): New test.
* test/lisp/wid-edit-tests.el (widget-test-alist-default-value-1)
(widget-test-alist-default-value-2, widget-test-alist-default-value-3)
(widget-test-alist-default-value-4): New tests.
---
lisp/wid-edit.el | 32 ++++++++++++++++++++++++++-
test/lisp/cus-edit-tests.el | 43 +++++++++++++++++++++++++++++++++++++
test/lisp/wid-edit-tests.el | 31 ++++++++++++++++++++++++++
3 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 47531113ba8..5dbdd7127aa 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -3812,8 +3812,19 @@ widget-character-notify
(define-widget 'list 'group
"A Lisp list."
:tag "List"
+ :default-get #'widget-list-default-get
:format "%{%t%}:\n%v")
+(defun widget-list-default-get (widget)
+ "Return the default external value for a list WIDGET.
+
+The default value is the one stored in the :value property, even if it is nil,
+or a list with the default value of each component of the list WIDGET."
+ (widget-apply widget :value-to-external
+ (if (widget-member widget :value)
+ (widget-get widget :value)
+ (widget-group-default-get widget))))
+
(define-widget 'vector 'group
"A Lisp vector."
:tag "Vector"
@@ -3942,7 +3953,6 @@ widget-plist-convert-option
value-type widget-plist-value-type))
`(group :format "Key: %v" :inline t ,key-type ,value-type)))
-
;;; The `alist' Widget.
;;
;; Association lists.
@@ -3952,6 +3962,7 @@ 'alist
:key-type '(sexp :tag "Key")
:value-type '(sexp :tag "Value")
:convert-widget 'widget-alist-convert-widget
+ :default-get #'widget-alist-default-get
:tag "Alist")
(defvar widget-alist-value-type) ;Dynamic variable
@@ -3986,6 +3997,25 @@ widget-alist-convert-option
(setq key-type `(const ,option)
value-type widget-alist-value-type))
`(cons :format "Key: %v" ,key-type ,value-type)))
+
+(defun widget-alist-default-get (widget)
+ "Return the default value for WIDGET, an alist widget.
+
+The default value may be one of:
+- The one stored in the :value property, even if it is nil.
+- If WIDGET has options available, an alist consisting of the
+default values for each option.
+- nil, otherwise."
+ (widget-apply widget :value-to-external
+ (cond ((widget-member widget :value)
+ (widget-get widget :value))
+ ((widget-get widget :options)
+ (mapcar #'widget-default-get
+ ;; Last one is the editable-list part, and
+ ;; we don't want those showing up as
+ ;; part of the default value. (Bug#63290)
+ (butlast (widget-get widget :args))))
+ (t nil))))
\f
(define-widget 'choice 'menu-choice
"A union of several sexp types.
diff --git a/test/lisp/cus-edit-tests.el b/test/lisp/cus-edit-tests.el
index eca35d7c96a..3a788f19745 100644
--- a/test/lisp/cus-edit-tests.el
+++ b/test/lisp/cus-edit-tests.el
@@ -92,5 +92,48 @@ test-setopt
(buffer-substring-no-properties (point-min) (point-max)))))
(should (string-search "Value `:foo' does not match type number"
warn-txt))))
+
+(defcustom cus-edit-test-bug63290-option nil
+ "Choice option for testing Bug#63290."
+ :type '(choice (alist
+ :key-type (string :tag "key")
+ :value-type (string :tag "value"))
+ (const :tag "auto" auto)))
+
+(defcustom cus-edit-test-bug63290-option2 'some
+ "Choice option for testing Bug#63290."
+ :type '(choice
+ (const :tag "some" some)
+ (alist
+ :key-type (string :tag "key")
+ :value-type (string :tag "value"))))
+
+(ert-deftest cus-edit-test-bug63290 ()
+ "Test that changing a choice value back to an alist respects its nil value."
+ (customize-variable 'cus-edit-test-bug63290-option)
+ (search-forward "Value")
+ ;; Simulate changing the value.
+ (let* ((choice (widget-at))
+ (args (widget-get choice :args))
+ (list-opt (car (widget-get choice :children)))
+ (const-opt (nth 1 args)))
+ (widget-put choice :explicit-choice const-opt)
+ (widget-value-set choice (widget-default-get const-opt))
+ (widget-put choice :explicit-choice list-opt)
+ (widget-value-set choice (widget-default-get list-opt)))
+ ;; No empty key/value pairs should show up.
+ (should-not (search-forward "key" nil t))
+ (customize-variable 'cus-edit-test-bug63290-option2)
+ (search-forward "Value")
+ ;; Simulate changing the value.
+ (let* ((choice (widget-at))
+ (args (widget-get choice :args))
+ (const-opt (car (widget-get choice :children)))
+ (list-opt (nth 1 args)))
+ (widget-put choice :explicit-choice list-opt)
+ (widget-value-set choice (widget-default-get list-opt)))
+ ;; No empty key/value pairs should show up.
+ (should-not (search-forward "key" nil t)))
+
(provide 'cus-edit-tests)
;;; cus-edit-tests.el ends here
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
index b379c7c91a8..ebfe729bc9a 100644
--- a/test/lisp/wid-edit-tests.el
+++ b/test/lisp/wid-edit-tests.el
@@ -349,4 +349,35 @@ widget-test-color-match
(should-not (widget-apply widget :match "someundefinedcolorihope"))
(should-not (widget-apply widget :match "#11223"))))
+(ert-deftest widget-test-alist-default-value-1 ()
+ "Test getting the default value for an alist widget with options."
+ (with-temp-buffer
+ (let ((w (widget-create '(alist :key-type string
+ :value-type integer
+ :options (("0" (integer)))))))
+ (should (equal '(("0" . 0)) (widget-default-get w))))))
+
+(ert-deftest widget-test-alist-default-value-2 ()
+ "Test getting the default value for an alist widget without :value."
+ (with-temp-buffer
+ (let ((w (widget-create '(alist :key-type string
+ :value-type integer))))
+ (should-not (widget-default-get w)))))
+
+(ert-deftest widget-test-alist-default-value-3 ()
+ "Test getting the default value for an alist widget with nil :value."
+ (with-temp-buffer
+ (let ((w (widget-create '(alist :key-type string
+ :value-type integer
+ :value nil))))
+ (should-not (widget-default-get w)))))
+
+(ert-deftest widget-test-alist-default-value-4 ()
+ "Test getting the default value for an alist widget with non-nil :value."
+ (with-temp-buffer
+ (let ((w (widget-create '(alist :key-type string
+ :value-type integer
+ :value (("1" . 1) ("2" . 2))))))
+ (should (equal '(("1" . 1) ("2" . 2)) (widget-default-get w))))))
+
;;; wid-edit-tests.el ends here
--
2.34.1
next prev parent reply other threads:[~2023-08-15 22:46 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-05 6:02 bug#63290: 30.0.50; Customize UI shows extra fields for (choice (const ...) (alist ...)) Thomas Fitzsimmons
2023-07-16 13:15 ` Mauro Aranda
2023-07-17 2:37 ` Thomas Fitzsimmons
2023-08-09 12:19 ` Mauro Aranda
2023-08-09 12:53 ` Eli Zaretskii
2023-08-09 15:51 ` Thomas Fitzsimmons
2023-08-09 15:56 ` Mauro Aranda
2023-08-09 18:03 ` Mauro Aranda
2023-08-10 22:58 ` Mauro Aranda
2023-08-11 13:29 ` Thomas Fitzsimmons
2023-08-15 22:46 ` Mauro Aranda [this message]
2023-08-16 15:16 ` Thomas Fitzsimmons
2023-08-19 8:34 ` Eli Zaretskii
2023-08-21 12:23 ` Mattias Engdegård
2023-08-21 14:43 ` Mauro Aranda
2023-08-21 15:24 ` Mattias Engdegård
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=08306097-a6d2-ffbe-0991-31afbb71c037@gmail.com \
--to=maurooaranda@gmail.com \
--cc=63290@debbugs.gnu.org \
--cc=fitzsim@fitzsim.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.