bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong

bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong

[Mauro Aranda]

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).

bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong

[Stefan Kangas]

> 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?

bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong

[Mauro Aranda]

[-- 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

bug#65625: 30.0.50; electric-pair-skip-whitespace-chars choice looks wrong

[Stefan Kangas]

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.