bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

[Nicolas Richard]

Hello,

from emacs -Q :

M-x cust-var RET
cua-mode RET
C-s state RET ;; this is the "State" button.
RET ;; activate it

Now hitting 5 on my numeric keypad gives the error "Canceled". If I hit
"normal" 5 (which requires me to hold shift key, because french/belgian
layout), it works as expected.

(Besides, it would be nice to be able to press TAB and cycle through the
options.)

In GNU Emacs 25.0.50.2 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2014-11-28 on localhost
Repository revision: 692322388c9f5bdebf2e8c4f7434c1d4769a04cc
Windowing system distributor `The X.Org Foundation', version 11.0.11500000
System Description:	Gentoo Base System release 2.2

-- 
Nicolas Richard

bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

[Eli Zaretskii]

> From: Nicolas Richard <theonewiththeevillook@yahoo.fr>
> Date: Thu, 04 Dec 2014 13:57:44 +0100
> 
> M-x cust-var RET
> cua-mode RET
> C-s state RET ;; this is the "State" button.
> RET ;; activate it
> 
> Now hitting 5 on my numeric keypad gives the error "Canceled".

I suspect that your numpad-5 key sends something that begins with ESC
ESC ESC.  What does "C-h l" show?

bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

[Nicolas Richard]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Nicolas Richard <theonewiththeevillook@yahoo.fr>
>> Date: Thu, 04 Dec 2014 13:57:44 +0100
>> 
>> M-x cust-var RET
>> cua-mode RET
>> C-s state RET ;; this is the "State" button.
>> RET ;; activate it
>> 
>> Now hitting 5 on my numeric keypad gives the error "Canceled".
>
> I suspect that your numpad-5 key sends something that begins with ESC
> ESC ESC.  What does "C-h l" show?

It shows <kp-5>

Sorry, I was a bit in a hurry and forgot to include the backtrace.

I'll try to explain how I understand the problem :

It happens in widget-choose, where that function does :
 (define-key map (vector next-digit) function)
Here, next-digit is the integer ?5. Moreover there's a
default binding
 (define-key map [t] 'keyboard-quit)
which is what is triggered when I hit <kp-5>

Simply trying to bind [kp-5] to function there won't work either,
because further in the code, there is this test:
 (while (not (or (and (integerp char)
		      (>= char ?0) (< char next-digit))
		 (eq value 'keyboard-quit)))
where "char" is the symbol kp-5 (it is the integer ?5 in the
working case)

Does that make any sense ?

-- 
Nicolas Richard

bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

[Stefan Monnier]

>  (define-key map [t] 'keyboard-quit)

This is the culprit: such a default binding prevents function-key-map
from being applied, since all keys have a (default) binding.
And the remapping of [kp-5] to [?5] is done in function-key-map.

So the fix should probably start by removing this default binding.


        Stefan

bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

[Nicolas Richard]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>  (define-key map [t] 'keyboard-quit)
>
> This is the culprit: such a default binding prevents function-key-map
> from being applied, since all keys have a (default) binding.
> And the remapping of [kp-5] to [?5] is done in function-key-map.
>
> So the fix should probably start by removing this default binding.

I now realize that the fix to most of my problems is (setq
widget-menu-minibuffer-flag t), but here's a fix to the reported bug.

Please let me know what you think.

FWIW, I do not like that scroll-other-window and negative-argument are
special-cased but I don't know what to do with them atm.


From dfbfebd1f403fd566fcffb5448b0450c7d2e746b Mon Sep 17 00:00:00 2001
From: Nicolas Richard <theonewiththeevillook@yahoo.fr>
Date: Fri, 5 Dec 2014 07:29:26 +0100
Subject: [PATCH] (widget-choose) Let numeric keypad work

Fixes: debbugs:19268

* wid-edit.el (widget-choose): Let numeric keypad work
and remove old menu-related code.
---
 lisp/ChangeLog   |  5 ++++
 lisp/wid-edit.el | 69 +++++++++++++++++++++-----------------------------------
 2 files changed, 31 insertions(+), 43 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 26572c3..40dd453 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2014-12-05  Nicolas Richard  <theonewiththeevillook@yahoo.fr>
+
+	* wid-edit.el (widget-choose): Let numeric keypad work (bug#19268)
+	and remove old menu-related code.
+
 2014-12-05  Juri Linkov  <juri@linkov.net>
 
 	Compare with the most recent window by default.
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 92e52bf..e901431 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -232,23 +232,20 @@ minibuffer."
 	(t
 	 ;; Construct a menu of the choices
 	 ;; and then use it for prompting for a single character.
-	 (let* ((overriding-terminal-local-map (make-sparse-keymap))
-		(next-digit ?0)
-		map choice some-choice-enabled value)
-	   ;; Define SPC as a prefix char to get to this menu.
-	   (define-key overriding-terminal-local-map " "
-	     (setq map (make-sparse-keymap title)))
+	 (let* ((next-digit ?0)
+		(map (make-sparse-keymap))
+                choice some-choice-enabled value)
 	   (with-current-buffer (get-buffer-create " widget-choose")
 	     (erase-buffer)
 	     (insert "Available choices:\n\n")
 	     (while items
-	       (setq choice (car items) items (cdr items))
-	       (if (consp choice)
-		   (let* ((name (car choice))
-			 (function (cdr choice)))
-		     (insert (format "%c = %s\n" next-digit name))
-		     (define-key map (vector next-digit) function)
-		     (setq some-choice-enabled t)))
+	       (setq choice (pop items))
+	       (when (consp choice)
+                 (let* ((name (car choice))
+                        (function (cdr choice)))
+                   (insert (format "%c = %s\n" next-digit name))
+                   (define-key map (vector next-digit) function)
+                   (setq some-choice-enabled t)))
 	       ;; Allocate digits to disabled alternatives
 	       ;; so that the digit of a given alternative never varies.
 	       (setq next-digit (1+ next-digit)))
@@ -257,43 +254,29 @@ minibuffer."
 	     (forward-line))
 	   (or some-choice-enabled
 	       (error "None of the choices is currently meaningful"))
-	   (define-key map [?\C-g] 'keyboard-quit)
-	   (define-key map [t] 'keyboard-quit)
 	   (define-key map [?\M-\C-v] 'scroll-other-window)
 	   (define-key map [?\M--] 'negative-argument)
-	   (setcdr map (nreverse (cdr map)))
-	   ;; Read a char with the menu, and return the result
-	   ;; that corresponds to it.
 	   (save-window-excursion
 	     (let ((buf (get-buffer " widget-choose")))
 	       (fit-window-to-buffer (display-buffer buf))
 	       (let ((cursor-in-echo-area t)
-		     keys
-		     (char 0)
 		     (arg 1))
-		 (while (not (or (and (integerp char)
-				      (>= char ?0) (< char next-digit))
-				 (eq value 'keyboard-quit)))
-		   ;; Unread a SPC to lead to our new menu.
-		   (setq unread-command-events (cons ?\s unread-command-events))
-		   (setq keys (read-key-sequence title))
-		   (setq value
-			 (lookup-key overriding-terminal-local-map keys t)
-			 char (aref keys 1))
-		   (cond ((eq value 'scroll-other-window)
-			  (let ((minibuffer-scroll-window
-				 (get-buffer-window buf)))
-			    (if (> 0 arg)
-				(scroll-other-window-down
-				 (window-height minibuffer-scroll-window))
-			      (scroll-other-window))
-			    (setq arg 1)))
-			 ((eq value 'negative-argument)
-			  (setq arg -1))
-			 (t
-			  (setq arg 1)))))))
-	   (when (eq value 'keyboard-quit)
-	     (error "Canceled"))
+                 (while (not value)
+                   (setq value (lookup-key map (read-key-sequence (format "%s: " title))))
+                   (unless value
+                     (user-error "Canceled"))
+                   (when
+                       (cond ((eq value 'scroll-other-window)
+                               (let ((minibuffer-scroll-window
+                                      (get-buffer-window buf)))
+                                 (if (> 0 arg)
+                                     (scroll-other-window-down
+                                      (window-height minibuffer-scroll-window))
+                                   (scroll-other-window))
+                                 (setq arg 1)))
+                              ((eq value 'negative-argument)
+                               (setq arg -1)))
+                     (setq value nil))))))
 	   value))))
 
 ;;; Widget text specifications.
-- 
2.0.2

-- 
Nicolas Richard

bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

[Stefan Monnier]

> Please let me know what you think.

Looks good, feel free to install it.

> FWIW, I do not like that scroll-other-window and negative-argument are
> special-cased but I don't know what to do with them atm.

It's no worse than before,


        Stefan

bug#19268: 25.0.50; Customize widgets vs <kp-...> keys

[Nicolas Richard]

Le 05/12/2014 16:11, Stefan Monnier a écrit :
>> Please let me know what you think.
> 
> Looks good, feel free to install it.

Done, and closing.

  commit 2a06fc15b2a3e6287f18025806fb2eabec801fc0
  Author: Nicolas Richard <theonewiththeevillook@yahoo.fr>
  Date:   Fri Dec 5 07:29:26 2014 +0100

  (widget-choose) Let numeric keypad work

  Fixes: debbugs:19268

  * wid-edit.el (widget-choose): Let numeric keypad work
  and remove old menu-related code.

-- 
Nico.