* bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong @ 2023-08-30 15:34 Mauro Aranda 2023-08-30 19:21 ` Stefan Kangas 0 siblings, 1 reply; 4+ messages in thread From: Mauro Aranda @ 2023-08-30 15:34 UTC (permalink / raw) To: 65625 Looking at the electric-pair-skip-whitespace-chars defcustom: (defcustom electric-pair-skip-whitespace-chars (list ?\t ?\s ?\n) "Whitespace characters considered by `electric-pair-skip-whitespace'." :version "24.4" :group 'electricity :type '(choice (set (const :tag "Space" ?\s) (const :tag "Tab" ?\t) (const :tag "Newline" ?\n)) (list character))) I think the 2nd choice should be (repeat character) rather than (list character). ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong 2023-08-30 15:34 bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong Mauro Aranda @ 2023-08-30 19:21 ` Stefan Kangas 2023-08-30 22:48 ` Mauro Aranda 0 siblings, 1 reply; 4+ messages in thread From: Stefan Kangas @ 2023-08-30 19:21 UTC (permalink / raw) To: Mauro Aranda; +Cc: 65625 > Looking at the electric-pair-skip-whitespace-chars defcustom: > > (defcustom electric-pair-skip-whitespace-chars (list ?\t ?\s ?\n) > "Whitespace characters considered by `electric-pair-skip-whitespace'." > :version "24.4" > :group 'electricity > :type '(choice (set (const :tag "Space" ?\s) > (const :tag "Tab" ?\t) > (const :tag "Newline" ?\n)) > (list character))) > > I think the 2nd choice should be (repeat character) rather than > (list character). With `list' I get this: Hide Electric Pair Skip Whitespace Chars: Choice: Value Menu List: Character: ^@ State : EDITED, shown value does not take effect until you set or save it. Whitespace characters considered by ‘electric-pair-skip-whitespace’. Groups: Electricity And with `repeat', I get this: Hide Electric Pair Skip Whitespace Chars: Choice: Value Menu Repeat: INS State : EDITED, shown value does not take effect until you set or save it. Whitespace characters considered by ‘electric-pair-skip-whitespace’. Groups: Electricity If I now hit "INS", I see this: Hide Electric Pair Skip Whitespace Chars: Choice: Value Menu Repeat: INS DEL Character: ^@ INS State : EDITED, shown value does not take effect until you set or save it. Whitespace characters considered by ‘electric-pair-skip-whitespace’. Groups: Electricity And then when I try to delete "^@" to edit it, it changes to "\s", and then I can't delete it to enter something else. So this all seems pretty wonky to me. Is `repeat' that much of an improvement? ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong 2023-08-30 19:21 ` Stefan Kangas @ 2023-08-30 22:48 ` Mauro Aranda 2023-08-31 6:24 ` Stefan Kangas 0 siblings, 1 reply; 4+ messages in thread From: Mauro Aranda @ 2023-08-30 22:48 UTC (permalink / raw) To: Stefan Kangas; +Cc: 65625 [-- Attachment #1: Type: text/plain, Size: 2501 bytes --] Stefan Kangas <stefankangas@gmail.com> writes: >> Looking at the electric-pair-skip-whitespace-chars defcustom: >> >> (defcustom electric-pair-skip-whitespace-chars (list ?\t ?\s ?\n) >> "Whitespace characters considered by `electric-pair-skip-whitespace'." >> :version "24.4" >> :group 'electricity >> :type '(choice (set (const :tag "Space" ?\s) >> (const :tag "Tab" ?\t) >> (const :tag "Newline" ?\n)) >> (list character))) >> >> I think the 2nd choice should be (repeat character) rather than >> (list character). > > With `list' I get this: > > Hide Electric Pair Skip Whitespace Chars: Choice: Value Menu List: > Character: ^@ > State : EDITED, shown value does not take effect until you set or save it. > Whitespace characters considered by ‘electric-pair-skip-whitespace’. > Groups: Electricity > > And with `repeat', I get this: > > Hide Electric Pair Skip Whitespace Chars: Choice: Value Menu Repeat: > INS > State : EDITED, shown value does not take effect until you set or save it. > Whitespace characters considered by ‘electric-pair-skip-whitespace’. > Groups: Electricity > > If I now hit "INS", I see this: > > Hide Electric Pair Skip Whitespace Chars: Choice: Value Menu Repeat: > INS DEL Character: ^@ > INS > State : EDITED, shown value does not take effect until you set or save it. > Whitespace characters considered by ‘electric-pair-skip-whitespace’. > Groups: Electricity > > And then when I try to delete "^@" to edit it, it changes to "\s", and > then I can't delete it to enter something else. So this all seems > pretty wonky to me. Is `repeat' that much of an improvement? The character widget might be wonky, but that shouldn't matter when choosing between list or repeat. You would have observed the same behavior if the character widget were part of the list widget (delete "^@" to edit it, changes to "\s", then hitting C-d looks like it does nothing). What's important here is that the 2nd choice lets the user choose only one whitespace character, while it seems pretty clear to me that the intention was for the user to be able to customize it to a list of whitespace characters, not just to a list of one. And that's what the repeat widget is for, so yes, it is a real improvement. I attach a patch. [-- Attachment #2: 0001-Fix-choice-in-electric-pair-skip-whitespace-chars.patch --] [-- Type: text/x-patch, Size: 930 bytes --] From 403b6c216bfaf27d1ce53c44905683d44644571d Mon Sep 17 00:00:00 2001 From: Mauro Aranda <maurooaranda@gmail.com> Date: Wed, 30 Aug 2023 19:45:09 -0300 Subject: [PATCH] Fix choice in electric-pair-skip-whitespace-chars * lisp/elec-pair.el (electric-pair-skip-whitespace-chars): Use repeat instead of list. (Bug#65625) --- lisp/elec-pair.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el index e101cbc9e6f..223c7d89773 100644 --- a/lisp/elec-pair.el +++ b/lisp/elec-pair.el @@ -153,7 +153,7 @@ electric-pair-skip-whitespace-chars :type '(choice (set (const :tag "Space" ?\s) (const :tag "Tab" ?\t) (const :tag "Newline" ?\n)) - (list character))) + (repeat (character :value " ")))) (defvar-local electric-pair-skip-whitespace-function #'electric-pair--skip-whitespace -- 2.34.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong 2023-08-30 22:48 ` Mauro Aranda @ 2023-08-31 6:24 ` Stefan Kangas 0 siblings, 0 replies; 4+ messages in thread From: Stefan Kangas @ 2023-08-31 6:24 UTC (permalink / raw) To: Mauro Aranda; +Cc: 65625-done Version: 30.1 Mauro Aranda <maurooaranda@gmail.com> writes: > What's important here is that the 2nd choice lets the user choose only > one whitespace character, while it seems pretty clear to me > that the intention was for the user to be able to customize it to a list > of whitespace characters, not just to a list of one. And that's what > the repeat widget is for, so yes, it is a real improvement. OK, makes sense. > I attach a patch. Thanks. Installed on master as commit 4adedd29961, and I'm closing this bug. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-31 6:24 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-30 15:34 bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong Mauro Aranda 2023-08-30 19:21 ` Stefan Kangas 2023-08-30 22:48 ` Mauro Aranda 2023-08-31 6:24 ` Stefan Kangas
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).