unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: npostavs@users.sourceforge.net
To: Eli Zaretskii <eliz@gnu.org>
Cc: 12204@debbugs.gnu.org
Subject: bug#12204: 24.1.50; Binding mouse-movement interferes with "C-h c" for mouse clicks
Date: Sat, 24 Jun 2017 23:18:03 -0400	[thread overview]
Message-ID: <87d19spxg4.fsf@users.sourceforge.net> (raw)
In-Reply-To: <83poe93yxx.fsf@gnu.org> (Eli Zaretskii's message of "Mon, 12 Jun 2017 20:20:26 +0300")

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

tags 12204 + patch
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Mon, 12 Jun 2017 13:15:24 -0400
>> Cc: 12204@debbugs.gnu.org
>> 
>> > I was actually thinking seriously to not show the help message for
>> > mouse-movement bindings.  Would that break something important?
>> 
>> It should only break discovery of what's bound to <mouse-movement> I
>> guess, and we could still leave "C-h k" for that so it shouldn't be
>> much of a problem.
>
> Then I think this is what we should do, and perhaps leave some knob to
> get back the old behavior.

Here are some patches, the first deduplicates the code of describe-key
and describe-key-briefly since they had some almost-identical sections
that confused me when I started by accidentally modifying the wrong one.

The second patch just ignores mouse-movement for "C-h c", I don't think
it's worth an option to get the old behaviour, but it's easy enough to
add it if you want.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 14379 bytes --]

From b973070650626de588fa64ffd08348c39de7be77 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 17 Jun 2017 20:33:56 -0400
Subject: [PATCH 1/2] Refactor key describing commands

* lisp/help.el (help-read-key-sequence, help--analyze-key): New
functions, extracted from `describe-key' and `describe-key-briefly'.
(describe-key, describe-key-briefly): Use them.
---
 lisp/help.el | 251 ++++++++++++++++++++++++-----------------------------------
 1 file changed, 103 insertions(+), 148 deletions(-)

diff --git a/lisp/help.el b/lisp/help.el
index 4d98c1dc81..24e07662e8 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -593,6 +593,39 @@ (defun help-key-description (key untranslated)
 	    string
 	  (format "%s (translated from %s)" string otherstring))))))
 
+(defun help--analyze-key (key untranslated)
+  "Get information about KEY its corresponding UNTRANSLATED events.
+Returns a list of the form (BRIEF-DESC DEFN EVENT MOUSE-MSG)."
+  (if (numberp untranslated)
+      (setq untranslated (this-single-command-raw-keys)))
+  (let* ((event (aref key (if (and (symbolp (aref key 0))
+				   (> (length key) 1)
+				   (consp (aref key 1)))
+			      1
+			    0)))
+	 (modifiers (event-modifiers event))
+	 (mouse-msg (if (or (memq 'click modifiers) (memq 'down modifiers)
+			    (memq 'drag modifiers)) " at that spot" ""))
+	 (defn (key-binding key t)))
+    ;; Handle the case where we faked an entry in "Select and Paste" menu.
+    (when (and (eq defn nil)
+	       (stringp (aref key (1- (length key))))
+	       (eq (key-binding (substring key 0 -1)) 'yank-menu))
+      (setq defn 'menu-bar-select-yank))
+    ;; Don't bother user with strings from (e.g.) the select-paste menu.
+    (when (stringp (aref key (1- (length key))))
+      (aset key (1- (length key)) "(any string)"))
+    (when (and untranslated
+               (stringp (aref untranslated (1- (length untranslated)))))
+      (aset untranslated (1- (length untranslated)) "(any string)"))
+    (list
+     ;; Now describe the key, perhaps as changed.
+     (let ((key-desc (help-key-description key untranslated)))
+       (if (or (null defn) (integerp defn) (equal defn 'undefined))
+           (format "%s%s is undefined" key-desc mouse-msg)
+         (format "%s%s runs the command %S" key-desc mouse-msg defn)))
+     defn event mouse-msg)))
+
 (defun describe-key-briefly (&optional key insert untranslated)
   "Print the name of the function KEY invokes.  KEY is a string.
 If INSERT (the prefix arg) is non-nil, insert the message in the buffer.
@@ -603,73 +636,10 @@ (defun describe-key-briefly (&optional key insert untranslated)
 If KEY is a menu item or a tool-bar button that is disabled, this command
 temporarily enables it to allow getting help on disabled items and buttons."
   (interactive
-   (let ((enable-disabled-menus-and-buttons t)
-	 (cursor-in-echo-area t)
-	 saved-yank-menu)
-     (unwind-protect
-	 (let (key)
-	   ;; If yank-menu is empty, populate it temporarily, so that
-	   ;; "Select and Paste" menu can generate a complete event.
-	   (when (null (cdr yank-menu))
-	     (setq saved-yank-menu (copy-sequence yank-menu))
-	     (menu-bar-update-yank-menu "(any string)" nil))
-           (while
-               (progn
-                 (setq key (read-key-sequence "Describe the following key, mouse click, or menu item: "))
-                 (and (vectorp key)
-                      (consp (aref key 0))
-                      (symbolp (car (aref key 0)))
-                      (string-match "\\(mouse\\|down\\|click\\|drag\\)"
-                                    (symbol-name (car (aref key 0))))
-                      (not (sit-for (/ double-click-time 1000.0) t)))))
-	   ;; Clear the echo area message (Bug#7014).
-	   (message nil)
-	   ;; If KEY is a down-event, read and discard the
-	   ;; corresponding up-event.  Note that there are also
-	   ;; down-events on scroll bars and mode lines: the actual
-	   ;; event then is in the second element of the vector.
-	   (and (vectorp key)
-		(let ((last-idx (1- (length key))))
-		  (and (eventp (aref key last-idx))
-		       (memq 'down (event-modifiers (aref key last-idx)))))
-		(read-event))
-	   (list
-	    key
-	    (if current-prefix-arg (prefix-numeric-value current-prefix-arg))
-	    1))
-       ;; Put yank-menu back as it was, if we changed it.
-       (when saved-yank-menu
-	 (setq yank-menu (copy-sequence saved-yank-menu))
-	 (fset 'yank-menu (cons 'keymap yank-menu))))))
-  (if (numberp untranslated)
-      (setq untranslated (this-single-command-raw-keys)))
-  (let* ((event (if (and (symbolp (aref key 0))
-			 (> (length key) 1)
-			 (consp (aref key 1)))
-		    (aref key 1)
-		  (aref key 0)))
-	 (modifiers (event-modifiers event))
-	 (standard-output (if insert (current-buffer) standard-output))
-	 (mouse-msg (if (or (memq 'click modifiers) (memq 'down modifiers)
-			    (memq 'drag modifiers)) " at that spot" ""))
-	 (defn (key-binding key t))
-	 key-desc)
-    ;; Handle the case where we faked an entry in "Select and Paste" menu.
-    (if (and (eq defn nil)
-	     (stringp (aref key (1- (length key))))
-	     (eq (key-binding (substring key 0 -1)) 'yank-menu))
-	(setq defn 'menu-bar-select-yank))
-    ;; Don't bother user with strings from (e.g.) the select-paste menu.
-    (if (stringp (aref key (1- (length key))))
-	(aset key (1- (length key)) "(any string)"))
-    (if (and (> (length untranslated) 0)
-	     (stringp (aref untranslated (1- (length untranslated)))))
-	(aset untranslated (1- (length untranslated)) "(any string)"))
-    ;; Now describe the key, perhaps as changed.
-    (setq key-desc (help-key-description key untranslated))
-    (if (or (null defn) (integerp defn) (equal defn 'undefined))
-	(princ (format "%s%s is undefined" key-desc mouse-msg))
-      (princ (format "%s%s runs the command %S" key-desc mouse-msg defn)))))
+   (pcase-let ((`(,key ,_up-event) (help-read-key-sequence)))
+     `(,key ,current-prefix-arg 1)))
+  (princ (car (help--analyze-key key untranslated))
+         (if insert (current-buffer) standard-output)))
 
 (defun help--key-binding-keymap (key &optional accept-default no-remap position)
   "Return a keymap holding a binding for KEY within current keymaps.
@@ -734,6 +704,55 @@ (defun help--binding-locus (key position)
                (throw 'found x))))
           nil)))))
 
+(defun help-read-key-sequence ()
+  "Reads a key sequence from the user.
+Returns a list of the form (KEY UP-EVENT), where KEY is the key
+sequence, and UP-EVENT is the up-event that was discarded by
+reading KEY, or nil."
+  (let ((enable-disabled-menus-and-buttons t)
+        (cursor-in-echo-area t)
+        saved-yank-menu)
+    (unwind-protect
+        (let (key)
+          ;; If yank-menu is empty, populate it temporarily, so that
+          ;; "Select and Paste" menu can generate a complete event.
+          (when (null (cdr yank-menu))
+            (setq saved-yank-menu (copy-sequence yank-menu))
+            (menu-bar-update-yank-menu "(any string)" nil))
+          (while
+              (pcase (setq key (read-key-sequence "\
+Describe the following key, mouse click, or menu item: "))
+                ((and (pred vectorp) (let `(,key0 . ,_) (aref key 0))
+                      (guard (symbolp key0)) (let keyname (symbol-name key0)))
+                 (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
+                                    keyname)
+                      (not (sit-for (/ double-click-time 1000.0) t))))))
+          (list
+           key
+           ;; If KEY is a down-event, read and include the
+           ;; corresponding up-event.  Note that there are also
+           ;; down-events on scroll bars and mode lines: the actual
+           ;; event then is in the second element of the vector.
+           (and (vectorp key)
+                (let ((last-idx (1- (length key))))
+                  (and (eventp (aref key last-idx))
+                       (memq 'down (event-modifiers (aref key last-idx)))))
+                (or (and (eventp (aref key 0))
+                         (memq 'down (event-modifiers (aref key 0)))
+                         ;; However, for the C-down-mouse-2 popup
+                         ;; menu, there is no subsequent up-event.  In
+                         ;; this case, the up-event is the next
+                         ;; element in the supplied vector.
+                         (= (length key) 1))
+                    (and (> (length key) 1)
+                         (eventp (aref key 1))
+                         (memq 'down (event-modifiers (aref key 1)))))
+                (read-event))))
+      ;; Put yank-menu back as it was, if we changed it.
+      (when saved-yank-menu
+        (setq yank-menu (copy-sequence saved-yank-menu))
+        (fset 'yank-menu (cons 'keymap yank-menu))))))
+
 (defun describe-key (&optional key untranslated up-event)
   "Display documentation of the function invoked by KEY.
 KEY can be any kind of a key sequence; it can include keyboard events,
@@ -748,83 +767,20 @@ (defun describe-key (&optional key untranslated up-event)
 If KEY is a menu item or a tool-bar button that is disabled, this command
 temporarily enables it to allow getting help on disabled items and buttons."
   (interactive
-   (let ((enable-disabled-menus-and-buttons t)
-	 (cursor-in-echo-area t)
-	 saved-yank-menu)
-     (unwind-protect
-	 (let (key)
-	   ;; If yank-menu is empty, populate it temporarily, so that
-	   ;; "Select and Paste" menu can generate a complete event.
-	   (when (null (cdr yank-menu))
-	     (setq saved-yank-menu (copy-sequence yank-menu))
-	     (menu-bar-update-yank-menu "(any string)" nil))
-           (while
-               (progn
-                 (setq key (read-key-sequence "Describe the following key, mouse click, or menu item: "))
-                 (and (vectorp key)
-                      (consp (aref key 0))
-                      (symbolp (car (aref key 0)))
-                      (string-match "\\(mouse\\|down\\|click\\|drag\\)"
-                                    (symbol-name (car (aref key 0))))
-                      (not (sit-for (/ double-click-time 1000.0) t)))))
-	   (list
-	    key
-	    (prefix-numeric-value current-prefix-arg)
-	    ;; If KEY is a down-event, read and include the
-	    ;; corresponding up-event.  Note that there are also
-	    ;; down-events on scroll bars and mode lines: the actual
-	    ;; event then is in the second element of the vector.
-	    (and (vectorp key)
-		 (let ((last-idx (1- (length key))))
-		   (and (eventp (aref key last-idx))
-			(memq 'down (event-modifiers (aref key last-idx)))))
-		 (or (and (eventp (aref key 0))
-			  (memq 'down (event-modifiers (aref key 0)))
-			  ;; However, for the C-down-mouse-2 popup
-			  ;; menu, there is no subsequent up-event.  In
-			  ;; this case, the up-event is the next
-			  ;; element in the supplied vector.
-			  (= (length key) 1))
-		     (and (> (length key) 1)
-			  (eventp (aref key 1))
-			  (memq 'down (event-modifiers (aref key 1)))))
-		 (read-event))))
-       ;; Put yank-menu back as it was, if we changed it.
-       (when saved-yank-menu
-	 (setq yank-menu (copy-sequence saved-yank-menu))
-	 (fset 'yank-menu (cons 'keymap yank-menu))))))
-  (if (numberp untranslated)
-      (setq untranslated (this-single-command-raw-keys)))
-  (let* ((event (aref key (if (and (symbolp (aref key 0))
-				   (> (length key) 1)
-				   (consp (aref key 1)))
-			      1
-			    0)))
-	 (modifiers (event-modifiers event))
-	 (mouse-msg (if (or (memq 'click modifiers) (memq 'down modifiers)
-			    (memq 'drag modifiers)) " at that spot" ""))
-	 (defn (key-binding key t))
-         key-locus key-locus-up key-locus-up-tricky
-	 defn-up defn-up-tricky ev-type
-	 mouse-1-remapped mouse-1-tricky)
-
-    ;; Handle the case where we faked an entry in "Select and Paste" menu.
-    (when (and (eq defn nil)
-	       (stringp (aref key (1- (length key))))
-	       (eq (key-binding (substring key 0 -1)) 'yank-menu))
-      (setq defn 'menu-bar-select-yank))
-    (if (or (null defn) (integerp defn) (equal defn 'undefined))
-	(message "%s%s is undefined"
-		 (help-key-description key untranslated) mouse-msg)
+   (pcase-let ((`(,key ,up-event) (help-read-key-sequence)))
+     `(,key ,(prefix-numeric-value current-prefix-arg) ,up-event)))
+  (pcase-let ((`(,brief-desc ,defn ,event ,mouse-msg)
+               (help--analyze-key key untranslated))
+              (defn-up nil) (defn-up-tricky nil)
+              (key-locus-up nil) (key-locus-up-tricky nil)
+              (mouse-1-remapped nil) (mouse-1-tricky nil)
+              (ev-type nil))
+    (if (or (null defn)
+            (integerp defn)
+            (equal defn 'undefined))
+        (message "%s" brief-desc)
       (help-setup-xref (list #'describe-function defn)
 		       (called-interactively-p 'interactive))
-      ;; Don't bother user with strings from (e.g.) the select-paste menu.
-      (when (stringp (aref key (1- (length key))))
-	(aset key (1- (length key)) "(any string)"))
-      (when (and untranslated
-		 (stringp (aref untranslated (1- (length untranslated)))))
-	(aset untranslated (1- (length untranslated))
-	      "(any string)"))
       ;; Need to do this before erasing *Help* buffer in case event
       ;; is a mouse click in an existing *Help* buffer.
       (when up-event
@@ -849,13 +805,12 @@ (defun describe-key (&optional key untranslated up-event)
 	    (aset sequence 0 'mouse-1)
 	    (setq defn-up-tricky (key-binding sequence nil nil (event-start up-event)))
             (setq key-locus-up-tricky (help--binding-locus sequence (event-start up-event))))))
-      (setq key-locus (help--binding-locus key (event-start event)))
       (with-help-window (help-buffer)
-	(princ (help-key-description key untranslated))
-	(princ (format "%s runs the command %S%s, which is "
-		       mouse-msg defn (if key-locus
-                                          (format " (found in %s)" key-locus)
-                                        "")))
+        (princ brief-desc)
+        (let ((key-locus (help--binding-locus key (event-start event))))
+          (when key-locus
+            (princ (format " (found in %s)" key-locus))))
+        (princ ", which is ")
 	(describe-function-1 defn)
 	(when up-event
 	  (unless (or (null defn-up)
-- 
2.11.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch --]
[-- Type: text/x-diff, Size: 4113 bytes --]

From f44c0dfe8bd29374a980848db29890c1438fe3f3 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 18 Jun 2017 00:39:05 -0400
Subject: [PATCH 2/2] Ignore mouse-movement for describe-key-briefly
 (Bug#12204)

* lisp/help.el (help-read-key-sequence): Add optional argument ot
ignore `mouse-movement' events.
(describe-key-briefly): Use it.
* doc/emacs/help.texi (Key Help):
* etc/NEWS: Mention that mouse movement is ignored.
---
 doc/emacs/help.texi |  7 ++++---
 etc/NEWS            |  3 +++
 lisp/help.el        | 18 ++++++++++++------
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/doc/emacs/help.texi b/doc/emacs/help.texi
index 548ca6a1b4..fd6df1c7e5 100644
--- a/doc/emacs/help.texi
+++ b/doc/emacs/help.texi
@@ -203,9 +203,10 @@ Key Help
 describes the command corresponding to @var{key}.
 
   @kbd{C-h c}, @kbd{C-h k} and @kbd{C-h K} work for any sort of key
-sequences, including function keys, menus, and mouse events.  For
-instance, after @kbd{C-h k} you can select a menu item from the menu
-bar, to view the documentation string of the command it runs.
+sequences, including function keys, menus, and mouse events (except
+that @kbd{C-h c} ignores mouse movement events).  For instance, after
+@kbd{C-h k} you can select a menu item from the menu bar, to view the
+documentation string of the command it runs.
 
 @kindex C-h w
 @findex where-is
diff --git a/etc/NEWS b/etc/NEWS
index 60c98a35c0..281bacffd0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -373,6 +373,9 @@ use the local Emacs to edit remote files via Tramp.  See the node
 "emacsclient Options" in the user manual for the details.
 
 +++
+** 'describe-key-briefly' now ignores mouse movement events.
+
++++
 ** The new variable 'eval-expression-print-maximum-character' prevents
 large integers from being displayed as characters.
 
diff --git a/lisp/help.el b/lisp/help.el
index 24e07662e8..dc341b0635 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -636,7 +636,9 @@ (defun describe-key-briefly (&optional key insert untranslated)
 If KEY is a menu item or a tool-bar button that is disabled, this command
 temporarily enables it to allow getting help on disabled items and buttons."
   (interactive
-   (pcase-let ((`(,key ,_up-event) (help-read-key-sequence)))
+   ;; Ignore mouse movement events because it's too easy to miss the
+   ;; message while moving the mouse.
+   (pcase-let ((`(,key ,_up-event) (help-read-key-sequence 'no-mouse-movement)))
      `(,key ,current-prefix-arg 1)))
   (princ (car (help--analyze-key key untranslated))
          (if insert (current-buffer) standard-output)))
@@ -704,11 +706,13 @@ (defun help--binding-locus (key position)
                (throw 'found x))))
           nil)))))
 
-(defun help-read-key-sequence ()
+(defun help-read-key-sequence (&optional no-mouse-movement)
   "Reads a key sequence from the user.
 Returns a list of the form (KEY UP-EVENT), where KEY is the key
 sequence, and UP-EVENT is the up-event that was discarded by
-reading KEY, or nil."
+reading KEY, or nil.
+If NO-MOUSE-MOVEMENT is non-nil, ignore key sequences starting
+with `mouse-movement' events."
   (let ((enable-disabled-menus-and-buttons t)
         (cursor-in-echo-area t)
         saved-yank-menu)
@@ -724,9 +728,11 @@ (defun help-read-key-sequence ()
 Describe the following key, mouse click, or menu item: "))
                 ((and (pred vectorp) (let `(,key0 . ,_) (aref key 0))
                       (guard (symbolp key0)) (let keyname (symbol-name key0)))
-                 (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
-                                    keyname)
-                      (not (sit-for (/ double-click-time 1000.0) t))))))
+                 (if no-mouse-movement
+                     (string-match "mouse-movement" keyname)
+                   (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
+                                      keyname)
+                        (not (sit-for (/ double-click-time 1000.0) t)))))))
           (list
            key
            ;; If KEY is a down-event, read and include the
-- 
2.11.1


  reply	other threads:[~2017-06-25  3:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-14 19:07 bug#12204: 24.1.50; Binding mouse-movement interferes with "C-h c" for mouse clicks Eli Zaretskii
2017-06-12  0:18 ` npostavs
2017-06-12 14:10   ` Eli Zaretskii
2017-06-12 16:58     ` Noam Postavsky
2017-06-12 17:07       ` Eli Zaretskii
2017-06-12 17:15         ` Noam Postavsky
2017-06-12 17:20           ` Eli Zaretskii
2017-06-25  3:18             ` npostavs [this message]
2017-06-25 13:49               ` Eli Zaretskii
2017-07-01 14:10                 ` npostavs

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

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d19spxg4.fsf@users.sourceforge.net \
    --to=npostavs@users.sourceforge.net \
    --cc=12204@debbugs.gnu.org \
    --cc=eliz@gnu.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 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).