GNU bug report logs - #29478
26.0.90; `C-h k' followed by mouse clicks no longer shows down event

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29478 in the body.
You can then email your comments to 29478 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.90; `C-h k' followed by mouse clicks no longer shows down event
Date: Mon, 27 Nov 2017 15:21:43 -0800 (PST)

The down mouse-button event is no longer listed along with the up
event when you use `C-h k' with a click event.  Dunno whether this was
by design or is an oversight.

Seems to me like it was better before.  When you click a mouse button
sometimes the action you're interested in is actually bound to the down
event, not the up event.  And sometimes you're interested in
descriptions of both events.

And how else can you get to see the help for the command bound to the
down-button event?  I don't see how to do that now.

Also, I don't see anything in NEWS about this.  I do see this, however,
for `describe-key-briefly' (which is not what `C-h k' does) - perhaps it
is related to this undocumented change?

** 'describe-key-briefly' now ignores mouse movement events.


In GNU Emacs 26.0.90 (build 3, x86_64-w64-mingw32)
 of 2017-10-13 built on LAPHROAIG
Repository revision: 906224eba147bdfc0514090064e8e8f53160f1d4
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Type C-x 1 to delete the help window.

Message #8 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Alan Mackenzie <acm <at> muc.de>, 29478 <at> debbugs.gnu.org
Subject: Re: bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Mon, 27 Nov 2017 20:50:11 -0500

tags 29272 + confirmed
quit

Drew Adams <drew.adams <at> oracle.com> writes:

> The down mouse-button event is no longer listed along with the up
> event when you use `C-h k' with a click event.  Dunno whether this was
> by design or is an oversight.

Looks like oversight.  Possibly related to Bug#29272.  The following
seems to fix it for me, but I'm having trouble convincing myself that
it's the right thing.

--- c/lisp/help.el
+++ i/lisp/help.el
@@ -738,7 +738,7 @@ help-read-key-sequence
                          ;; spuriously trigger the `sit-for'.
                          (sleep-for 0.01)
                          (while (read-event nil nil 0.01))
-                         (not (sit-for (/ double-click-time 1000.0) t))))))))
+                         (sit-for (/ double-click-time 1000.0) t)))))))
           (list
            key
            ;; If KEY is a down-event, read and include the


Alan, I believe you've been working on this code recently, any thoughts?

Message #13 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Tue, 28 Nov 2017 19:11:38 +0200

> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Mon, 27 Nov 2017 20:50:11 -0500
> Cc: Alan Mackenzie <acm <at> muc.de>, 29478 <at> debbugs.gnu.org
> 
> Drew Adams <drew.adams <at> oracle.com> writes:
> 
> > The down mouse-button event is no longer listed along with the up
> > event when you use `C-h k' with a click event.  Dunno whether this was
> > by design or is an oversight.
> 
> Looks like oversight.  Possibly related to Bug#29272.  The following
> seems to fix it for me, but I'm having trouble convincing myself that
> it's the right thing.
> 
> --- c/lisp/help.el
> +++ i/lisp/help.el
> @@ -738,7 +738,7 @@ help-read-key-sequence
>                           ;; spuriously trigger the `sit-for'.
>                           (sleep-for 0.01)
>                           (while (read-event nil nil 0.01))
> -                         (not (sit-for (/ double-click-time 1000.0) t))))))))
> +                         (sit-for (/ double-click-time 1000.0) t)))))))
>            (list
>             key
>             ;; If KEY is a down-event, read and include the
> 

Not an oversight: the above patch brings back bug#29272.

Message #16 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>,
 Drew Adams <drew.adams <at> oracle.com>
Cc: 29478 <at> debbugs.gnu.org
Subject: Re: bug#29478: 26.0.90; `C-h k' followed by mouse clicks no longer
 shows down event
Date: Tue, 28 Nov 2017 22:10:36 +0000

Hello, Noam and Drew.

On Mon, Nov 27, 2017 at 20:50:11 -0500, Noam Postavsky wrote:
> tags 29272 + confirmed
> quit

> Drew Adams <drew.adams <at> oracle.com> writes:

> > The down mouse-button event is no longer listed along with the up
> > event when you use `C-h k' with a click event.  Dunno whether this was
> > by design or is an oversight.

I think if you press and hold the mouse button for (default) half a
second, when you finally release it, C-h k will report the down mouse
event.  At least this is how GPM behaves for me on a Linux tty.  This
seems to make sense, because anything bound to a down mouse event is
going to be something like a drag event, where the mouse button is held
for an extended period of time.

> Looks like oversight.  Possibly related to Bug#29272.  The following
> seems to fix it for me, but I'm having trouble convincing myself that
> it's the right thing.

> --- c/lisp/help.el
> +++ i/lisp/help.el
> @@ -738,7 +738,7 @@ help-read-key-sequence
>                           ;; spuriously trigger the `sit-for'.
>                           (sleep-for 0.01)
>                           (while (read-event nil nil 0.01))
> -                         (not (sit-for (/ double-click-time 1000.0) t))))))))
> +                         (sit-for (/ double-click-time 1000.0) t)))))))
>            (list
>             key
>             ;; If KEY is a down-event, read and include the


> Alan, I believe you've been working on this code recently, any thoughts?

That I'm redoing some of the underlying C code after a suboptimal patch
some days ago, and this seems to have some effect on the mouse events
reported for C-h c and C-h k.

Eli has already reported that the proposed negation of that condition
would break the patch and lead to annoying infinite loops with mouse
events.

I think this code still may have some way to go before reaching its
final state.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #19 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29478 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Tue, 28 Nov 2017 19:44:01 -0500

Alan Mackenzie <acm <at> muc.de> writes:

> I think if you press and hold the mouse button for (default) half a
> second, when you finally release it, C-h k will report the down mouse
> event.  At least this is how GPM behaves for me on a Linux tty.  This
> seems to make sense, because anything bound to a down mouse event is
> going to be something like a drag event, where the mouse button is held
> for an extended period of time.

This works for me in both lucid and gtk builds.

> Eli has already reported that the proposed negation of that condition
> would break the patch and lead to annoying infinite loops with mouse
> events.

Oh, hmm.  I was testing in a lucid build where your fix for that doesn't
quite seem to work, so I didn't realize that this change breaks things.

Message #22 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org
Subject: Re: bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Wed, 29 Nov 2017 05:36:56 +0200

> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Tue, 28 Nov 2017 19:44:01 -0500
> Cc: 29478 <at> debbugs.gnu.org
> 
> > Eli has already reported that the proposed negation of that condition
> > would break the patch and lead to annoying infinite loops with mouse
> > events.
> 
> Oh, hmm.  I was testing in a lucid build where your fix for that doesn't
> quite seem to work

That's bad (and strange).  What exactly happens on Lucid when you try
that recipe?

Message #25 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Alan Mackenzie <acm <at> muc.de>, Noam Postavsky
 <npostavs <at> users.sourceforge.net>
Cc: 29478 <at> debbugs.gnu.org
Subject: RE: bug#29478: 26.0.90; `C-h k' followed by mouse clicks no longer
 shows down event
Date: Tue, 28 Nov 2017 20:33:00 -0800 (PST)

> > > The down mouse-button event is no longer listed along with the up
> > > event when you use `C-h k' with a click event.  Dunno whether this
> was
> > > by design or is an oversight.
> 
> I think if you press and hold the mouse button for (default) half a
> second, when you finally release it, C-h k will report the down mouse
> event.  At least this is how GPM behaves for me on a Linux tty.

Ah yes, thanks.  I see that now on MS Windows too.

> This seems to make sense, because anything bound to a down mouse
> event is going to be something like a drag event, where the mouse
> button is held for an extended period of time.

Sorry, no, I don't buy that.  I mean it's probably true that
most such bindings involve dragging.  But (1) a user who wants
to check what `mouse-N' does won't necessarily use it the same
way when checking with `C-h k' as when using it for its real
purpose.  And (2) the problem with this is that it is not
easily discoverable.

I've used Emacs for quite a while.  OK, I expected both up and
down to be documented.  But even I would likely never have
thought to hold the button pressed for a long time before
releasing, just to see what `C-h k' had to tell me.

If there is no other solution, then I suppose we could live
with this.  But in that case it should be called out in NEWS,
I think.  And if there is a possibility of restoring the
longstanding behavior then I think we should first try for that.

> > Looks like oversight.  Possibly related to Bug#29272.  The following
> > seems to fix it for me, but I'm having trouble convincing myself that
> > it's the right thing...
> 
> > Alan, I believe you've been working on this code recently, any
> thoughts?
> 
> That I'm redoing some of the underlying C code after a suboptimal patch
> some days ago, and this seems to have some effect on the mouse events
> reported for C-h c and C-h k.
> 
> Eli has already reported that the proposed negation of that condition
> would break the patch and lead to annoying infinite loops with mouse
> events.
> 
> I think this code still may have some way to go before reaching its
> final state.

Thanks for still taking a look and trying to work this out.

Message #28 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Fri, 01 Dec 2017 10:31:45 +0200

> Date: Tue, 28 Nov 2017 20:33:00 -0800 (PST)
> From: Drew Adams <drew.adams <at> oracle.com>
> Cc: 29478 <at> debbugs.gnu.org
> 
> > > > The down mouse-button event is no longer listed along with the up
> > > > event when you use `C-h k' with a click event.  Dunno whether this
> > was
> > > > by design or is an oversight.
> > 
> > I think if you press and hold the mouse button for (default) half a
> > second, when you finally release it, C-h k will report the down mouse
> > event.  At least this is how GPM behaves for me on a Linux tty.
> 
> Ah yes, thanks.  I see that now on MS Windows too.
> 
> > This seems to make sense, because anything bound to a down mouse
> > event is going to be something like a drag event, where the mouse
> > button is held for an extended period of time.
> 
> Sorry, no, I don't buy that.  I mean it's probably true that
> most such bindings involve dragging.  But (1) a user who wants
> to check what `mouse-N' does won't necessarily use it the same
> way when checking with `C-h k' as when using it for its real
> purpose.  And (2) the problem with this is that it is not
> easily discoverable.
> 
> I've used Emacs for quite a while.  OK, I expected both up and
> down to be documented.  But even I would likely never have
> thought to hold the button pressed for a long time before
> releasing, just to see what `C-h k' had to tell me.

Alan, can we arrange for the doc string of mouse-N clicks (with or
without all the possible modifiers) to say something like

  For documentation of the corresponding mouse-down event, click and
  hold the mouse button longer than %s sec.

(where %s gets replaced by double-click-time)?

Message #31 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Drew Adams <drew.adams <at> oracle.com>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: RE: bug#29478: 26.0.90;	`C-h k' followed by mouse clicks no longer
 shows down event
Date: Fri, 1 Dec 2017 07:35:08 -0800 (PST)

> > I would likely never have
> > thought to hold the button pressed for a long time before
> > releasing, just to see what `C-h k' had to tell me.
> 
> Alan, can we arrange for the doc string of mouse-N clicks (with or
> without all the possible modifiers) to say something like
> 
>   For documentation of the corresponding mouse-down event,
>   click and hold the mouse button longer than %s sec.
> 
> (where %s gets replaced by double-click-time)?

That's a hack/workaround, but yes, if we can't do better
then that would definitely help.

If that's at the end of the doc for the up event
then I suspect some users looking for info about
the down event might never notice it.

It should perhaps be made very prominent - ideally
just as prominent as before.  Previously we had two
headings for the two events.  That prominent, whether
done using headings or some other way - that would be
my suggestion.

Message #34 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: 26.0.90;	`C-h k' followed by mouse clicks no longer
 shows down event
Date: Fri, 01 Dec 2017 18:51:37 +0200

> Date: Fri, 1 Dec 2017 07:35:08 -0800 (PST)
> From: Drew Adams <drew.adams <at> oracle.com>
> Cc: acm <at> muc.de, npostavs <at> users.sourceforge.net, 29478 <at> debbugs.gnu.org
> 
> > > I would likely never have
> > > thought to hold the button pressed for a long time before
> > > releasing, just to see what `C-h k' had to tell me.
> > 
> > Alan, can we arrange for the doc string of mouse-N clicks (with or
> > without all the possible modifiers) to say something like
> > 
> >   For documentation of the corresponding mouse-down event,
> >   click and hold the mouse button longer than %s sec.
> > 
> > (where %s gets replaced by double-click-time)?
> 
> That's a hack/workaround

The previous way, where down-event was described before the up-event,
was also a hack.  The fundamental problem here is that Emacs doesn't
know what the user wants/needs, and the semi-kludgey way we intuit
that when processing mouse gestures is part of the reason.

> If that's at the end of the doc for the up event
> then I suspect some users looking for info about
> the down event might never notice it.

Well, at least they will now see the documentation of mouse-1, not
down-mouse-1, first.  And in "C-h c", we were previously showing
_only_ down-mouse-1, something that made me mad for years, because in
90% of cases you want to know about mouse-1.  So this is some
progress, even in this area (although it was not the main drive for
the change).

> Previously we had two headings for the two events.

For "C-h k", yes (and a user who wasn't expecting to see 2 doc strings
could easily miss the second one).  For "C-h c", no: we only was
showing the binding for the down-event.

Message #37 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: acm <at> muc.de
Cc: 29478 <at> debbugs.gnu.org, drew.adams <at> oracle.com,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 09 Dec 2017 18:13:40 +0200

I think I found one more problem with Help functions on mouse clicks.
Try this in "emacs -Q":

  C-h c S-mouse-1
  C-h k S-mouse-1

In Emacs 25.2 these produce the description of mouse-appearance-menu,
as expected, saying it's the binding of S-down-mouse-1.  Emacs
26.0.90 does the same for "C-h c", but claims S-mouse-1 is undefined
if you invoke "C-h k".  The current emacs-26 branch claims S-mouse-1
is undefined for both "C-h c" and "C-h k".

I think this means that we have similar problems with any explicit
bindings to S-down-mouse-N clicks.

This is a regression that I think we must fix before releasing Emacs
26.1.

Message #40 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29478 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: 26.0.90; `C-h k' followed by mouse clicks no longer
 shows down event
Date: Sat, 9 Dec 2017 17:18:16 +0000

Hello, Eli.

On Fri, Dec 01, 2017 at 10:31:45 +0200, Eli Zaretskii wrote:
> > Date: Tue, 28 Nov 2017 20:33:00 -0800 (PST)
> > From: Drew Adams <drew.adams <at> oracle.com>
> > Cc: 29478 <at> debbugs.gnu.org

> > > > > The down mouse-button event is no longer listed along with the up
> > > > > event when you use `C-h k' with a click event.  Dunno whether this
> > > was
> > > > > by design or is an oversight.

> > > I think if you press and hold the mouse button for (default) half a
> > > second, when you finally release it, C-h k will report the down mouse
> > > event.  At least this is how GPM behaves for me on a Linux tty.

> > Ah yes, thanks.  I see that now on MS Windows too.

> > > This seems to make sense, because anything bound to a down mouse
> > > event is going to be something like a drag event, where the mouse
> > > button is held for an extended period of time.

> > Sorry, no, I don't buy that.  I mean it's probably true that
> > most such bindings involve dragging.  But (1) a user who wants
> > to check what `mouse-N' does won't necessarily use it the same
> > way when checking with `C-h k' as when using it for its real
> > purpose.  And (2) the problem with this is that it is not
> > easily discoverable.

> > I've used Emacs for quite a while.  OK, I expected both up and
> > down to be documented.  But even I would likely never have
> > thought to hold the button pressed for a long time before
> > releasing, just to see what `C-h k' had to tell me.

> Alan, can we arrange for the doc string of mouse-N clicks (with or
> without all the possible modifiers) to say something like

>   For documentation of the corresponding mouse-down event, click and
>   hold the mouse button longer than %s sec.

> (where %s gets replaced by double-click-time)?

Yes, I'm sure I can manage that, hopefully in the coming week.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #43 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29478 <at> debbugs.gnu.org, drew.adams <at> oracle.com,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: 26.0.90;`C-h k' followed by mouse clicks no longer
 shows down event
Date: Sat, 9 Dec 2017 17:23:14 +0000

Hello, Eli.

On Sat, Dec 09, 2017 at 18:13:40 +0200, Eli Zaretskii wrote:
> I think I found one more problem with Help functions on mouse clicks.
> Try this in "emacs -Q":

>   C-h c S-mouse-1
>   C-h k S-mouse-1

> In Emacs 25.2 these produce the description of mouse-appearance-menu,
> as expected, saying it's the binding of S-down-mouse-1.  Emacs
> 26.0.90 does the same for "C-h c", but claims S-mouse-1 is undefined
> if you invoke "C-h k".  The current emacs-26 branch claims S-mouse-1
> is undefined for both "C-h c" and "C-h k".

> I think this means that we have similar problems with any explicit
> bindings to S-down-mouse-N clicks.

> This is a regression that I think we must fix before releasing Emacs
> 26.1.

Yes.  I'm sorry I've had next to no time for Emacs this last week.  That
should change on Monday.  Then I'll have a look at this bug.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #46 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29478 <at> debbugs.gnu.org, drew.adams <at> oracle.com,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: 26.0.90;`C-h k' followed by mouse clicks no longer
 shows down event
Date: Fri, 22 Dec 2017 22:05:49 +0000

Hello, Eli.

On Sat, Dec 09, 2017 at 18:13:40 +0200, Eli Zaretskii wrote:
> I think I found one more problem with Help functions on mouse clicks.
> Try this in "emacs -Q":

>   C-h c S-mouse-1
>   C-h k S-mouse-1

> In Emacs 25.2 these produce the description of mouse-appearance-menu,
> as expected, saying it's the binding of S-down-mouse-1.  Emacs
> 26.0.90 does the same for "C-h c", but claims S-mouse-1 is undefined
> if you invoke "C-h k".  The current emacs-26 branch claims S-mouse-1
> is undefined for both "C-h c" and "C-h k".

> I think this means that we have similar problems with any explicit
> bindings to S-down-mouse-N clicks.

> This is a regression that I think we must fix before releasing Emacs
> 26.1.

OK, I'm looking at this now.  It's not as easy as I was hoping.

One phenomenon is puzzling me - it feels like a bug:

(i) emacs-26 -Q
(ii) M-: (read-key-sequence "prompt: ") <CR>
(iii) <shift>-mouse-1

What happens now is the key sequence is displayed, a S-down-mouse-1
event, but immediately overwritten in the message area by 

    <S-mouse-1> is undefined

.  Why?  I merely asked for the key sequence.  I didn't ask for it to be
looked up in any key maps.  What is doing the translation from a
shift-down-mouse event to a S-click event?  What is looking up this
down-mouse event in a key map?

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #49 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: 26.0.90;`C-h k' followed by mouse clicks no longer
 shows down event
Date: Sat, 23 Dec 2017 11:42:10 +0200

> Date: Fri, 22 Dec 2017 22:05:49 +0000
> Cc: drew.adams <at> oracle.com, 29478 <at> debbugs.gnu.org,
>   npostavs <at> users.sourceforge.net
> From: Alan Mackenzie <acm <at> muc.de>
> 
> (i) emacs-26 -Q
> (ii) M-: (read-key-sequence "prompt: ") <CR>
> (iii) <shift>-mouse-1
> 
> What happens now is the key sequence is displayed, a S-down-mouse-1
> event, but immediately overwritten in the message area by 
> 
>     <S-mouse-1> is undefined
> 
> .  Why?  I merely asked for the key sequence.  I didn't ask for it to be
> looked up in any key maps.  What is doing the translation from a
> shift-down-mouse event to a S-click event?  What is looking up this
> down-mouse event in a key map?

read-key-sequence itself examines the possible remapping of the
sequence.  And it seems reasonable, since how can it otherwise know
when the sequence is complete?

What is perhaps unexpected here (or might be a bug) is that for some
reason the sequence is left in unread-command-events (or somewhere
similar, perhaps in read_key_sequence_remapped?), because if I set a
breakpoint in Fding, the backtrace from the "is undefined" message
clearly shows it was the command loop that invoked 'undefined':

  Thread 1 hit Breakpoint 3, Fding (arg=XIL(0)) at dispnew.c:5666
  5666      if (!NILP (arg))
  (gdb) bt
  #0  Fding (arg=XIL(0)) at dispnew.c:5666
  #1  0x012269c1 in funcall_subr (subr=0x1383100 <Sding>, numargs=0,
      args=0x82f2e0) at eval.c:2841
  #2  0x01226412 in Ffuncall (nargs=1, args=0x82f2d8) at eval.c:2766
  #3  0x0128cef7 in exec_byte_code (bytestr=XIL(0x8000000001397478),
      vector=XIL(0xa000000001397488), maxdepth=make_number(4),
      args_template=make_number(0), nargs=0, args=0x82f750) at bytecode.c:62
  #4  0x01227510 in funcall_lambda (fun=XIL(0xa000000001397440), nargs=0,
      arg_vector=0x40000000) at eval.c:2967
  #5  0x0122646c in Ffuncall (nargs=1, args=0x82f748) at eval.c:2768
  #6  0x01225913 in call0 (fn=XIL(0xfca8)) at eval.c:2609
  #7  0x0114fd0d in command_loop_1 () at keyboard.c:1457
  #8  0x01220de9 in internal_condition_case (bfun=0x114f33f <command_loop_1>
      handlers=XIL(0x5b70), hfun=0x114e5bf <cmd_error>) at eval.c:1332
  #9  0x0114edd3 in command_loop_2 (ignore=XIL(0)) at keyboard.c:1110
  #10 0x0121ff1b in internal_catch (tag=XIL(0xf570),
      func=0x114ed97 <command_loop_2>, arg=XIL(0)) at eval.c:1097
  #11 0x0114ed52 in command_loop () at keyboard.c:1089
  #12 0x0114dfe7 in recursive_edit_1 () at keyboard.c:695
  #13 0x0114e289 in Frecursive_edit () at keyboard.c:766
  #14 0x0114b990 in main (argc=2, argv=0xa428d8) at emacs.c:1713

  Lisp Backtrace:
  "ding" (0x82f2e0)
  "undefined" (0x82f750)

CC'ing Stefan in the hope that he could have some insights.

Message #52 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29478 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;`C-h k' followed by mouse clicks no
 longer shows down event
Date: Sat, 23 Dec 2017 11:17:26 +0000

Hello, Eli.

On Sat, Dec 23, 2017 at 11:42:10 +0200, Eli Zaretskii wrote:
> > Date: Fri, 22 Dec 2017 22:05:49 +0000
> > Cc: drew.adams <at> oracle.com, 29478 <at> debbugs.gnu.org,
> >   npostavs <at> users.sourceforge.net
> > From: Alan Mackenzie <acm <at> muc.de>

> > (i) emacs-26 -Q
> > (ii) M-: (read-key-sequence "prompt: ") <CR>
> > (iii) <shift>-mouse-1

> > What happens now is the key sequence is displayed, a S-down-mouse-1
> > event, but immediately overwritten in the message area by 

> >     <S-mouse-1> is undefined

> > .  Why?  I merely asked for the key sequence.  I didn't ask for it to be
> > looked up in any key maps.  What is doing the translation from a
> > shift-down-mouse event to a S-click event?  What is looking up this
> > down-mouse event in a key map?

> read-key-sequence itself examines the possible remapping of the
> sequence.  And it seems reasonable, since how can it otherwise know
> when the sequence is complete?

> What is perhaps unexpected here (or might be a bug) is that for some
> reason the sequence is left in unread-command-events (or somewhere
> similar, perhaps in read_key_sequence_remapped?), because if I set a
> breakpoint in Fding, the backtrace from the "is undefined" message
> clearly shows it was the command loop that invoked 'undefined':

[ .... ]

Of course!  read-key-sequence consumes all the events up to the
S-down-mouse-1, leaving the following S-mouse-1 in the event buffer.
This is read at the next iteration of the command loop.

> CC'ing Stefan in the hope that he could have some insights.

OK, I have a provisional fix.  The problem was that the double-click-time
loop in help-read-key-sequence was discarding all events but the last
one.  I have amended the loop so that _all_ these events are stored in a
list, and we then discard the most recent events till we find one with a
binding.

This patch seems to work with GPM in a Linux tty, and also in X-Windows
(I think).  (It is based on the emacs-26 branch, of course.)



diff --git a/lisp/help.el b/lisp/help.el
index 212e3679da..dd1676adb0 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -717,7 +717,7 @@ help-read-key-sequence
         (cursor-in-echo-area t)
         saved-yank-menu)
     (unwind-protect
-        (let (key down-ev)
+        (let (key keys down-ev discarded-up)
           ;; If yank-menu is empty, populate it temporarily, so that
           ;; "Select and Paste" menu can generate a complete event.
           (when (null (cdr yank-menu))
@@ -731,6 +731,7 @@ help-read-key-sequence
                  (or
                   (and no-mouse-movement
                        (string-match "mouse-movement" keyname))
+                  (progn (push key keys) nil)
                   (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
                                      keyname)
                        (progn
@@ -739,13 +740,31 @@ help-read-key-sequence
                          (sleep-for 0.01)
                          (while (read-event nil nil 0.01))
                          (not (sit-for (/ double-click-time 1000.0) t))))))))
+          ;; When we have a sequence of mouse events, discard the most
+          ;; recent ones till we find one with a binding.
+          (let ((keys-1 keys))
+            (while (and keys-1
+                        (not (key-binding (car keys-1))))
+              ;; If we discard the last event, and this was a mouse
+              ;; up, remember this.
+              (if (and (eq keys-1 keys)
+                       (vectorp (car keys-1))
+                       (let* ((last-idx (1- (length (car keys-1))))
+                              (last (aref (car keys-1) last-idx)))
+                         (and (eventp last)
+                              (memq 'click (event-modifiers last)))))
+                  (setq discarded-up t))
+              (setq keys-1 (cdr keys-1)))
+            (if keys-1
+                (setq key (car keys-1))))
           (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)
+           (and (not discarded-up) ; Don't attempt to ignore the up-event twice.
+                (vectorp key)
                 (let ((last-idx (1- (length key))))
                   (and (eventp (aref key last-idx))
                        (memq 'down (event-modifiers (aref key last-idx)))))


-- 
Alan Mackenzie (Nuremberg, Germany).

Message #55 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29478 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;`C-h k' followed by mouse clicks no
 longer shows down event
Date: Sat, 23 Dec 2017 14:41:42 +0200

> Date: Sat, 23 Dec 2017 11:17:26 +0000
> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 29478 <at> debbugs.gnu.org,
>   npostavs <at> users.sourceforge.net
> From: Alan Mackenzie <acm <at> muc.de>
> 
> Of course!  read-key-sequence consumes all the events up to the
> S-down-mouse-1, leaving the following S-mouse-1 in the event buffer.
> This is read at the next iteration of the command loop.
> 
> > CC'ing Stefan in the hope that he could have some insights.
> 
> OK, I have a provisional fix.  The problem was that the double-click-time
> loop in help-read-key-sequence was discarding all events but the last
> one.  I have amended the loop so that _all_ these events are stored in a
> list, and we then discard the most recent events till we find one with a
> binding.
> 
> This patch seems to work with GPM in a Linux tty, and also in X-Windows
> (I think).  (It is based on the emacs-26 branch, of course.)

LGTM, thanks.  The problem I reported is gone with this patch.

Message #58 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29478 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch 2]: bug#29478: 26.0.90; `C-h k' followed by mouse clicks
 no longer shows down event
Date: Sat, 23 Dec 2017 14:28:09 +0000

Hello, Eli.

On Fri, Dec 01, 2017 at 10:31:45 +0200, Eli Zaretskii wrote:
> > Date: Tue, 28 Nov 2017 20:33:00 -0800 (PST)
> > From: Drew Adams <drew.adams <at> oracle.com>
> > Cc: 29478 <at> debbugs.gnu.org

> > > > > The down mouse-button event is no longer listed along with the up
> > > > > event when you use `C-h k' with a click event.  Dunno whether this
> > > > > was by design or is an oversight.

> > > I think if you press and hold the mouse button for (default) half a
> > > second, when you finally release it, C-h k will report the down mouse
> > > event.  At least this is how GPM behaves for me on a Linux tty.

> > Ah yes, thanks.  I see that now on MS Windows too.

> > > This seems to make sense, because anything bound to a down mouse
> > > event is going to be something like a drag event, where the mouse
> > > button is held for an extended period of time.

> > Sorry, no, I don't buy that.  I mean it's probably true that
> > most such bindings involve dragging.  But (1) a user who wants
> > to check what `mouse-N' does won't necessarily use it the same
> > way when checking with `C-h k' as when using it for its real
> > purpose.  And (2) the problem with this is that it is not
> > easily discoverable.

> > I've used Emacs for quite a while.  OK, I expected both up and
> > down to be documented.  But even I would likely never have
> > thought to hold the button pressed for a long time before
> > releasing, just to see what `C-h k' had to tell me.

> Alan, can we arrange for the doc string of mouse-N clicks (with or
> without all the possible modifiers) to say something like

>   For documentation of the corresponding mouse-down event, click and
>   hold the mouse button longer than %s sec.

> (where %s gets replaced by double-click-time)?

Yes, we can indeed.  The following patch is intended to achieve this.
It's based on the emacs-26 branch.  I've also corrected the bug where the
possibility of double-click-time being nil or t wasn't taken into
account.

Should I commit this one to emacs-26, too?



diff --git a/lisp/help.el b/lisp/help.el
index 212e3679da..3f013e8170 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -738,7 +738,11 @@ help-read-key-sequence
                          ;; spuriously trigger the `sit-for'.
                          (sleep-for 0.01)
                          (while (read-event nil nil 0.01))
-                         (not (sit-for (/ double-click-time 1000.0) t))))))))
+                         (not (sit-for
+                               (if (numberp double-click-time)
+                                   (/ double-click-time 1000.0)
+                                 3.0)
+                               t))))))))
           (list
            key
            ;; If KEY is a down-event, read and include the
@@ -769,6 +773,28 @@ help-read-key-sequence
         (setq yank-menu (copy-sequence saved-yank-menu))
         (fset 'yank-menu (cons 'keymap yank-menu))))))
 
+(defun help-downify-mouse-event-type (base)
+  "Add \"down-\" to BASE if it is not already there.
+BASE is a symbol, a mouse event type.  If the modification is done,
+return the new symbol.  Otherwise return nil."
+  (let ((base-s (symbol-name base)))
+    ;; Note: the order of the components in the following string is
+    ;; determined by `apply_modifiers_uncached' in src/keyboard.c.
+    (string-match "\\(A-\\)?\
+\\(C-\\)?\
+\\(H-\\)?\
+\\(M-\\)?\
+\\(S-\\)?\
+\\(s-\\)?\
+\\(double-\\)?\
+\\(triple-\\)?\
+\\(up-\\)?\
+\\(\\(down-\\)?\\)\
+\\(\\(drag-\\)?\\)" base-s)
+    (when (and (zerop (length (match-string 10 base-s)))  ; "down-"
+               (zerop (length (match-string 12 base-s)))) ; "drag-"
+      (intern (replace-match "down-" t t base-s 10)) )))
+
 (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,
@@ -828,6 +854,24 @@ describe-key
             (princ (format " (found in %s)" key-locus))))
         (princ ", which is ")
 	(describe-function-1 defn)
+        (when (vectorp key)
+          (let* ((last (1- (length key)))
+                 (elt (aref key last))
+                 (elt-1 (copy-sequence elt))
+                 key-1 down-event-type)
+            (when (and (eventp elt)
+                       (setq down-event-type (help-downify-mouse-event-type
+                                              (car elt))))
+              (setcar elt-1 down-event-type)
+              (setq key-1 (vector elt-1))
+              (when (key-binding key-1)
+                (princ (format "
+
+For documentation of the corresponding mouse down event <%s>,
+click and hold the mouse button longer than %s second(s)."
+                               down-event-type (if (numberp double-click-time)
+                                                   (/ double-click-time 1000.0)
+                                                 3)))))))
 	(when up-event
 	  (unless (or (null defn-up)
 		      (integerp defn-up)


-- 
Alan Mackenzie (Nuremberg, Germany).

Message #61 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: [Patch 2]: bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 23 Dec 2017 15:48:38 +0100

On Dez 23 2017, Alan Mackenzie <acm <at> muc.de> wrote:

> @@ -769,6 +773,28 @@ help-read-key-sequence
>          (setq yank-menu (copy-sequence saved-yank-menu))
>          (fset 'yank-menu (cons 'keymap yank-menu))))))
>  
> +(defun help-downify-mouse-event-type (base)
> +  "Add \"down-\" to BASE if it is not already there.
> +BASE is a symbol, a mouse event type.  If the modification is done,
> +return the new symbol.  Otherwise return nil."
> +  (let ((base-s (symbol-name base)))
> +    ;; Note: the order of the components in the following string is
> +    ;; determined by `apply_modifiers_uncached' in src/keyboard.c.
> +    (string-match "\\(A-\\)?\
> +\\(C-\\)?\
> +\\(H-\\)?\
> +\\(M-\\)?\
> +\\(S-\\)?\
> +\\(s-\\)?\
> +\\(double-\\)?\
> +\\(triple-\\)?\
> +\\(up-\\)?\
> +\\(\\(down-\\)?\\)\
> +\\(\\(drag-\\)?\\)" base-s)
> +    (when (and (zerop (length (match-string 10 base-s)))  ; "down-"
> +               (zerop (length (match-string 12 base-s)))) ; "drag-"

A group that didn't match can be detected by match-string (or
match-beginning) returning nil.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #64 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: `C-h k' followed by mouse clicks no longer shows down
 event
Date: Sat, 23 Dec 2017 11:19:02 -0500

>> >   For documentation of the corresponding mouse-down event,
>> >   click and hold the mouse button longer than %s sec.
>> > (where %s gets replaced by double-click-time)?
>> That's a hack/workaround

Agreed.

> The previous way, where down-event was described before the up-event,
> was also a hack.

Maybe its implementation was hackish, but as far as the UI goes I don't
think it was a hack.

> The fundamental problem here is that Emacs doesn't
> know what the user wants/needs, and the semi-kludgey way we intuit
> that when processing mouse gestures is part of the reason.

The way to solve this is to add a menu/index at the beginning indicating
that there were *several* events, such that the user is made aware that
he may have to skip some info to find the one of interest (and
presumably this menu is made clickable so the user can easily jump to
the part he wants).

The same kind of situation shows up with `C-h o` for symbols that
combine various roles (function, variable, type, face, ...), BTW.


        Stefan

Message #67 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 23 Dec 2017 11:33:11 -0500

> Of course!  read-key-sequence consumes all the events up to the
> S-down-mouse-1, leaving the following S-mouse-1 in the event buffer.
> This is read at the next iteration of the command loop.

Exactly.

>> CC'ing Stefan in the hope that he could have some insights.
>
> OK, I have a provisional fix.  The problem was that the double-click-time
> loop in help-read-key-sequence was discarding all events but the last
> one.  I have amended the loop so that _all_ these events are stored in a
> list, and we then discard the most recent events till we find one with a
> binding.

I must say after reading the whole thread and looking at the code,
I still don't really understand what's the idea behind this patch (nor
exactly which problems we're trying to solve).

The way I understand the problem (which may be naive because I haven't
tried to play with the code yet, so I'm probably missing some subtlety),
the code should "simply" read events until the event read is not a down
event (and skipping pseudo events like mouse-movements and help-echo).
And then return all those events (it's probably not going to be more
than 2, but I don't see any benefit in hard-coding the fact that we
return either 1 or 2 events, we could just say "returns a list of
events").

BTW, here are some comments about the current code:

>                    (and no-mouse-movement
>                         (string-match "mouse-movement" keyname))
> +                  (progn (push key keys) nil)
>                    (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
>                                       keyname)

Why string-match on event names rather than use things like
event-basic-type, and event-modifiers?

>                         (progn
> @@ -739,13 +740,31 @@ help-read-key-sequence
>                           (sleep-for 0.01)
>                           (while (read-event nil nil 0.01))

We're blindly throwing away events here, right?  Isn't that a problem?


        Stefan

Message #70 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;`C-h k' followed by mouse clicks no
 longer shows down event
Date: Sat, 23 Dec 2017 21:04:07 +0000

Hello, Stefan.

On Sat, Dec 23, 2017 at 11:33:11 -0500, Stefan Monnier wrote:
[ .... ]

> > OK, I have a provisional fix.  The problem was that the double-click-time
> > loop in help-read-key-sequence was discarding all events but the last
> > one.  I have amended the loop so that _all_ these events are stored in a
> > list, and we then discard the most recent events till we find one with a
> > binding.

> I must say after reading the whole thread and looking at the code,
> I still don't really understand what's the idea behind this patch (nor
> exactly which problems we're trying to solve).

To make C-h c/k <shift><mouse-1> display "mouse-appearance-menu" rather
than "key sequence unbound".  The idea behind the code is to return the
last key sequence which is bound, rather than the very last one, which
may not be.

> The way I understand the problem (which may be naive because I haven't
> tried to play with the code yet, so I'm probably missing some subtlety),
> the code should "simply" read events until the event read is not a down
> event (and skipping pseudo events like mouse-movements and help-echo).
> And then return all those events (it's probably not going to be more
> than 2, but I don't see any benefit in hard-coding the fact that we
> return either 1 or 2 events, we could just say "returns a list of
> events").

Out of all these key sequences, some code somewhere has to select the
one for which the help message will be displayed.  Perhaps arbitrarily,
this is now in help-read-key-sequence.

> BTW, here are some comments about the current code:

> >                    (and no-mouse-movement
> >                         (string-match "mouse-movement" keyname))
> > +                  (progn (push key keys) nil)
> >                    (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
> >                                       keyname)

> Why string-match on event names rather than use things like
> event-basic-type, and event-modifiers?

Lack of knowledge on the part of the person who wrote it (me).

> >                         (progn
> > @@ -739,13 +740,31 @@ help-read-key-sequence
> >                           (sleep-for 0.01)
> >                           (while (read-event nil nil 0.01))

> We're blindly throwing away events here, right?  Isn't that a problem?

No, it's a filter.  Things like <help-echo>, which otherwise triggered
the sit-for are now discarded, so that only user events are processed.
I know of nobody who can press mouse buttons as fast as 100 times a
second.  All this was diagnosed in the early stages of another bug
report, bug #29272.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #73 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 23 Dec 2017 23:55:52 -0500

> Out of all these key sequences, some code somewhere has to select the
> one for which the help message will be displayed.

I think this is a mistake.  We should give the answer for all the events
we've seen.  E.g. it may be important for the user to know that mouse-1
is not bound or that down-mouse-1 is not bound.

>> Why string-match on event names rather than use things like
>> event-basic-type, and event-modifiers?
> Lack of knowledge on the part of the person who wrote it (me).

Ah, good, I thought the author had bumped into some odd problem with the
other constructs.

>> >                         (progn
>> > @@ -739,13 +740,31 @@ help-read-key-sequence
>> >                           (sleep-for 0.01)
>> >                           (while (read-event nil nil 0.01))
>
>> We're blindly throwing away events here, right?  Isn't that a problem?
>
> No, it's a filter.  Things like <help-echo>, which otherwise triggered
> the sit-for are now discarded, so that only user events are processed.

But how can we know that we'll only drop help-echo events there?

> I know of nobody who can press mouse buttons as fast as 100 times a
> second.

Yeah, I guess in practice it's not too bad (it can still suffer in cases
where the events don't come from separate user motions but via input
devices which send combined sequences, but it's not too common).

But why not just go around the loop again, and skip the help-echo events
like we do for mouse-movement?  Hmm... oh I guess it's because the
read-key-sequence used here has a timeout whereas the other doesn't...
I'm beginning to understand.
Thanks,


        Stefan

Message #76 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>,Alan Mackenzie <acm <at> muc.de>
Cc: 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 24 Dec 2017 07:41:08 +0200

On December 24, 2017 6:55:52 AM GMT+02:00, Stefan Monnier <monnier <at> IRO.UMontreal.CA> wrote:
> > Out of all these key sequences, some code somewhere has to select
> the
> > one for which the help message will be displayed.
> 
> I think this is a mistake.  We should give the answer for all the
> events
> we've seen.  E.g. it may be important for the user to know that
> mouse-1
> is not bound or that down-mouse-1 is not bound.


First, giving an answer for more than one event is impossible for "C-h c"; not without a complete redesign of how we present the results.  Until very recently, we would show only the first event, which is useless for 90% of mouse clicks, since very few commands are bound to down-events.  This has been a source of constant annoyance for me for years, and now is finally solved.  So this is some progress.

For "C-h k", we probably should indeed show all the events, but the previous solution to show the down-event first was problematic here as well, as it showed the much less important event first, and a user who is in a hurry might not pay attention to the rest.

In most cases, there are 2 or less events to show, so Alan's patch that tells users how to get help on the down-event is a good stopgap, IMO.  In general, we should probably redesign how these multiple events are described, but that's a job for master, not for the release branch.

Message #79 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 24 Dec 2017 01:52:26 -0500

Here's another take on this problem (clearly not intended for emacs-26).

This is also able to provide help on double/triple mouse clicks
(tho I had to resort to ztree to test it because such bindings are very
rare).

What do you guys think?


        Stefan


diff --git a/lisp/help.el b/lisp/help.el
index ac7cf91801..1a38042a51 100644
*** a/lisp/help.el
--- b/lisp/help.el
***************
*** 593,611 ****
  	    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)
--- 593,618 ----
  	    string
  	  (format "%s (translated from %s)" string otherstring))))))
  
+ (defun help--first-event (keyseq)
+   (when (> (length keyseq) 0)
+     (aref key (if (and (symbolp (aref keyseq 0))
+ 		       (> (length keyseq) 1)
+ 		       (consp (aref keyseq 1)))
+                   ;; Look at the second event when the first
+                   ;; is a pseudo-event like `mode-line' of `left-fringe'.
+ 		  1
+ 	        0))))
+ 
  (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)
!       (error "Missing `untranslated'!"))
!   (let* ((event (help--first-event key))
  	 (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)
***************
*** 626,647 ****
           (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.
- If non-nil, UNTRANSLATED is a vector of the untranslated events.
- It can also be a number in which case the untranslated events from
- the last key hit are used.
  
! 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
     ;; 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)))
  
  (defun help--key-binding-keymap (key &optional accept-default no-remap position)
    "Return a keymap holding a binding for KEY within current keymaps.
--- 633,658 ----
           (format "%s%s runs the command %S" key-desc mouse-msg defn)))
       defn event mouse-msg)))
  
! (defun describe-key-briefly (key-list &optional insert)
!   "Print the name of the functions KEY-LIST invokes.
! KEY-LIST is a list of pairs (SEQ . RAW-SEQ) of key sequences, where
! RAW-SEQ is the untranslated form of the key sequence SEQ.
  If INSERT (the prefix arg) is non-nil, insert the message in the buffer.
  
! While reading KEY-LIST interactively, this command temporarily enables
! menu items or tool-bar buttons that are disabled to allow getting help
! on them."
    (interactive
     ;; Ignore mouse movement events because it's too easy to miss the
     ;; message while moving the mouse.
!    (let ((key-list (help-read-key-sequence 'no-mouse-movement)))
!      `(,key-list ,current-prefix-arg)))
!   (let ((msg (mapconcat (lambda (x)
!                           (pcase-let ((`(,seq . ,raw-seq) (pop key-list)))
!                             (car (help--analyze-key seq raw-seq))))
!                         key-list
!                         "\n")))
!     (if insert (insert msg) (message "%s" msg))))
  
  (defun help--key-binding-keymap (key &optional accept-default no-remap position)
    "Return a keymap holding a binding for KEY within current keymaps.
***************
*** 706,865 ****
            nil)))))
  
  (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.
  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)
      (unwind-protect
!         (let (key down-ev)
            ;; 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)))
!                  (or
!                   (and no-mouse-movement
!                        (string-match "mouse-movement" keyname))
!                   (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
!                                      keyname)
!                        (progn
!                          ;; Discard events (e.g. <help-echo>) which might
!                          ;; spuriously trigger the `sit-for'.
!                          (sleep-for 0.01)
!                          (while (read-event nil nil 0.01))
!                          (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 (setq down-ev (aref key 0)))
!                          (memq 'down (event-modifiers down-ev))
!                          ;; 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 (setq down-ev (aref key 1)))
!                          (memq 'down (event-modifiers down-ev))))
!                 (if (and (terminal-parameter nil 'xterm-mouse-mode)
!                          (equal (terminal-parameter nil 'xterm-mouse-last-down)
!                                 down-ev))
!                     (aref (read-key-sequence-vector nil) 0)
!                   (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,
  mouse events, and/or menu events.  When calling from a program,
! pass KEY as a string or a vector.
! 
! If non-nil, UNTRANSLATED is a vector of the corresponding untranslated events.
! It can also be a number, in which case the untranslated events from
! the last key sequence entered are used.
! UP-EVENT is the up-event that was discarded by reading KEY, or nil.
  
- 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)))
-      `(,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))
-       ;; Need to do this before erasing *Help* buffer in case event
-       ;; is a mouse click in an existing *Help* buffer.
-       (when up-event
- 	(setq ev-type (event-basic-type up-event))
- 	(let ((sequence (vector up-event)))
- 	  (when (and (eq ev-type 'mouse-1)
- 		     mouse-1-click-follows-link
- 		     (not (eq mouse-1-click-follows-link 'double))
- 		     (setq mouse-1-remapped
- 			   (mouse-on-link-p (event-start up-event))))
- 	    (setq mouse-1-tricky (and (integerp mouse-1-click-follows-link)
- 				      (> mouse-1-click-follows-link 0)))
- 	    (cond ((stringp mouse-1-remapped)
- 		   (setq sequence mouse-1-remapped))
- 		  ((vectorp mouse-1-remapped)
- 		   (setcar up-event (elt mouse-1-remapped 0)))
- 		  (t (setcar up-event 'mouse-2))))
- 	  (setq defn-up (key-binding sequence nil nil (event-start up-event)))
-           (setq key-locus-up (help--binding-locus sequence (event-start up-event)))
- 	  (when mouse-1-tricky
- 	    (setq sequence (vector 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))))))
-       (with-help-window (help-buffer)
          (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)
! 		      (integerp defn-up)
! 		      (equal defn-up 'undefined))
! 	    (princ (format "
! 
! ----------------- up-event %s----------------
! 
! %s%s%s runs the command %S%s, which is "
! 			   (if mouse-1-tricky "(short click) " "")
! 			   (key-description (vector up-event))
! 			   mouse-msg
! 			   (if mouse-1-remapped
!                                " is remapped to <mouse-2>, which" "")
! 			   defn-up (if key-locus-up
!                                        (format " (found in %s)" key-locus-up)
!                                      "")))
! 	    (describe-function-1 defn-up))
! 	  (unless (or (null defn-up-tricky)
! 		      (integerp defn-up-tricky)
! 		      (eq defn-up-tricky 'undefined))
! 	    (princ (format "
! 
! ----------------- up-event (long click) ----------------
! 
! Pressing <%S>%s for longer than %d milli-seconds
! runs the command %S%s, which is "
! 			   ev-type mouse-msg
! 			   mouse-1-click-follows-link
! 			   defn-up-tricky (if key-locus-up-tricky
!                                               (format " (found in %s)" key-locus-up-tricky)
!                                             "")))
! 	    (describe-function-1 defn-up-tricky)))))))
  
  (defun describe-mode (&optional buffer)
    "Display documentation of current major mode and minor modes.
--- 717,815 ----
            nil)))))
  
  (defun help-read-key-sequence (&optional no-mouse-movement)
!   "Read \"a\" key sequence from the user.
! Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
! sequence, and RAW-SEQ is its untranslated form.
  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)
      (unwind-protect
!         (let (last-modifiers key-list)
            ;; 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
!               ;; Read at least one key-sequence.
!               (or (null key-list)
!                   ;; After a down event, also read the (presumably) following
!                   ;; up-event.
!                   (memq 'down last-modifiers)
!                   ;; After a click, see if a double click is on the way.
!                   (and (memq 'click last-modifiers)
!                        (not (sit-for (/ double-click-time 1000.0) t))))
!             (let* ((seq (read-key-sequence "\
  Describe the following key, mouse click, or menu item: "))
!                    (raw-seq (this-single-command-raw-keys))
!                    (key0 (when (> (length seq) 0)
!                            (aref seq 0)))
!                    (base (event-basic-type key0))
!                    (modifiers (event-modifiers key0)))
!               (cond
!                ((and no-mouse-movement (eq base 'mouse-movement)) nil)
!                ((eq base 'help-echo) nil)
!                (t
!                 (setq last-modifiers modifiers)
!                 (push (cons seq raw-seq) key-list)))))
!           (nreverse key-list))
        ;; 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 (key-list)
!   "Display documentation of the function invoked by KEY-LIST.
! KEY-LIST can be any kind of a key sequence; it can include keyboard events,
  mouse events, and/or menu events.  When calling from a program,
! pass KEY-LIST as a list of elements (SEQ . RAW-SEQ) where SEQ is
! a key-sequence and RAW-SEQ is its untranslated form.
! 
! While reading KEY-LIST interactively, this command temporarily enables
! menu items or tool-bar buttons that are disabled to allow getting help
! on them."
!   (interactive (list (help-read-key-sequence)))
!   (let ((buf (current-buffer))
!         (info-list
!          (mapcar (lambda (x)
!                    (pcase-let* ((`(,seq . ,raw-seq) x)
!                                 (`(,brief-desc ,defn ,event ,_mouse-msg)
!                                  (help--analyze-key seq raw-seq))
!                                 (locus
!                                  (help--binding-locus seq (event-start event))))
!                      `(,seq ,raw-seq ,brief-desc ,defn ,event ,locus)))
!                  key-list)))
!     (help-setup-xref (list (lambda (key-list)
!                              (with-current-buffer (if (buffer-live-p buf)
!                                                       buf (current-buffer))
!                                (describe-key key-list)))
!                            key-list)
! 		     (called-interactively-p 'interactive))
!     (with-help-window (help-buffer)
!       (when (> (length info-list) 1)
!         ;; FIXME: Make this into clickable hyperlinks.
!         (princ "There were several key-sequences:\n\n")
!         (princ (mapconcat (lambda (info)
!                              (pcase-let ((`(,seq ,raw-seq
!                                             ,brief-desc ,defn ,event ,locus)
!                                           info))
!                                (concat "  " brief-desc)))
!                            info-list
!                            "\n"))
!         (princ "\n\nThey're all described below."))
!       (pcase-dolist (`(,seq ,raw-seq ,brief-desc ,defn ,event ,locus)
!                      info-list)
!         (when (> (length info-list) 1)
!           (princ (format "\n\n----------------- event `%s' ----------------\n\n"
!                           (key-description seq))))
  
          (princ brief-desc)
!         (when locus
!           (princ (format " (found in %s)" locus)))
          (princ ", which is ")
! 	(describe-function-1 defn)))))
  
  (defun describe-mode (&optional buffer)
    "Display documentation of current major mode and minor modes.

Message #82 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#29478: [Patch 2]: bug#29478: 26.0.90; `C-h k' followed by
 mouse clicks no longer shows down event
Date: Sun, 24 Dec 2017 08:43:48 +0000

Hello, Andreas.

On Sat, Dec 23, 2017 at 15:48:38 +0100, Andreas Schwab wrote:
> On Dez 23 2017, Alan Mackenzie <acm <at> muc.de> wrote:

> > @@ -769,6 +773,28 @@ help-read-key-sequence
> >          (setq yank-menu (copy-sequence saved-yank-menu))
> >          (fset 'yank-menu (cons 'keymap yank-menu))))))
> >  
> > +(defun help-downify-mouse-event-type (base)
> > +  "Add \"down-\" to BASE if it is not already there.
> > +BASE is a symbol, a mouse event type.  If the modification is done,
> > +return the new symbol.  Otherwise return nil."
> > +  (let ((base-s (symbol-name base)))
> > +    ;; Note: the order of the components in the following string is
> > +    ;; determined by `apply_modifiers_uncached' in src/keyboard.c.
> > +    (string-match "\\(A-\\)?\
> > +\\(C-\\)?\
> > +\\(H-\\)?\
> > +\\(M-\\)?\
> > +\\(S-\\)?\
> > +\\(s-\\)?\
> > +\\(double-\\)?\
> > +\\(triple-\\)?\
> > +\\(up-\\)?\
> > +\\(\\(down-\\)?\\)\
> > +\\(\\(drag-\\)?\\)" base-s)
> > +    (when (and (zerop (length (match-string 10 base-s)))  ; "down-"
> > +               (zerop (length (match-string 12 base-s)))) ; "drag-"

> A group that didn't match can be detected by match-string (or
> match-beginning) returning nil.

Yes, it can.  Thanks!  I've amended the function accordingly.

> Andreas.

> -- 
> Andreas Schwab, schwab <at> linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #85 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;`C-h k' followed by mouse clicks no
 longer shows down event
Date: Sun, 24 Dec 2017 11:48:36 +0000

Hello, Stefan.

On Sun, Dec 24, 2017 at 01:52:26 -0500, Stefan Monnier wrote:
> Here's another take on this problem (clearly not intended for emacs-26).

> This is also able to provide help on double/triple mouse clicks
> (tho I had to resort to ztree to test it because such bindings are very
> rare).

> What do you guys think?

It's a long patch.  Any chance of you giving a brief summary of what it
does and what it's for, how it differs from what's already there, and so
on?

It's also refreshing to see a context diff every now and then.  :-)

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #88 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 24 Dec 2017 10:41:55 -0500

>> What do you guys think?
> It's a long patch.  Any chance of you giving a brief summary of what it
> does and what it's for, how it differs from what's already there, and so
> on?

The core part is `help-read-key-sequence` which is changed to:

    Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
    sequence, and RAW-SEQ is its untranslated form.
    If NO-MOUSE-MOVEMENT is non-nil, ignore key sequences starting
    with `mouse-movement' events."

and which just keeps calling read-key-sequence as long as:

              ;; Read at least one key-sequence.
              (or (null key-list)
                  ;; After a down event, also read the (presumably) following
                  ;; up-event.
                  (memq 'down last-modifiers)
                  ;; After a click, see if a double click is on the way.
                  (and (memq 'click last-modifiers)
                       (not (sit-for (/ double-click-time 1000.0) t))))

So when you do `C-h k` followed by a triple click,
help-read-key-sequence will can return a list of 6 elements (3 down
events and 3 up events).

describe-key(-briefly) are changed to just loop over all the key
sequences returned.

> It's also refreshing to see a context diff every now and then.  :-)

The new code share too little with the old code, so I found the unified
diff unreadable.


        Stefan

Message #91 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 30 Dec 2017 12:50:04 +0200

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
>         npostavs <at> users.sourceforge.net
> Date: Sun, 24 Dec 2017 01:52:26 -0500
> 
> Here's another take on this problem (clearly not intended for emacs-26).
> 
> This is also able to provide help on double/triple mouse clicks
> (tho I had to resort to ztree to test it because such bindings are very
> rare).
> 
> What do you guys think?

This patch doesn't apply to the master branch, so I couldn't test it.

Thanks.

Message #94 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29478 <at> debbugs.gnu.org, monnier <at> IRO.UMontreal.CA,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;`C-h k' followed by mouse clicks no
 longer shows down event
Date: Sat, 30 Dec 2017 12:51:15 +0200

> Date: Sat, 23 Dec 2017 21:04:07 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
>   npostavs <at> users.sourceforge.net
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > I must say after reading the whole thread and looking at the code,
> > I still don't really understand what's the idea behind this patch (nor
> > exactly which problems we're trying to solve).
> 
> To make C-h c/k <shift><mouse-1> display "mouse-appearance-menu" rather
> than "key sequence unbound".  The idea behind the code is to return the
> last key sequence which is bound, rather than the very last one, which
> may not be.
> 
> > The way I understand the problem (which may be naive because I haven't
> > tried to play with the code yet, so I'm probably missing some subtlety),
> > the code should "simply" read events until the event read is not a down
> > event (and skipping pseudo events like mouse-movements and help-echo).
> > And then return all those events (it's probably not going to be more
> > than 2, but I don't see any benefit in hard-coding the fact that we
> > return either 1 or 2 events, we could just say "returns a list of
> > events").
> 
> Out of all these key sequences, some code somewhere has to select the
> one for which the help message will be displayed.  Perhaps arbitrarily,
> this is now in help-read-key-sequence.
> 
> > BTW, here are some comments about the current code:
> 
> > >                    (and no-mouse-movement
> > >                         (string-match "mouse-movement" keyname))
> > > +                  (progn (push key keys) nil)
> > >                    (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
> > >                                       keyname)
> 
> > Why string-match on event names rather than use things like
> > event-basic-type, and event-modifiers?
> 
> Lack of knowledge on the part of the person who wrote it (me).
> 
> > >                         (progn
> > > @@ -739,13 +740,31 @@ help-read-key-sequence
> > >                           (sleep-for 0.01)
> > >                           (while (read-event nil nil 0.01))
> 
> > We're blindly throwing away events here, right?  Isn't that a problem?
> 
> No, it's a filter.  Things like <help-echo>, which otherwise triggered
> the sit-for are now discarded, so that only user events are processed.
> I know of nobody who can press mouse buttons as fast as 100 times a
> second.  All this was diagnosed in the early stages of another bug
> report, bug #29272.

Alan, any news on this one?  I'm waiting for this to go in before
asking Nicolas to produce the next pretest of Emacs 26.1.

Thanks.

Message #97 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29478 <at> debbugs.gnu.org, monnier <at> IRO.UMontreal.CA,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;`C-h k' followed by mouse clicks no
 longer shows down event
Date: Sat, 30 Dec 2017 11:32:29 +0000

Hello, Eli.

On Sat, Dec 30, 2017 at 12:51:15 +0200, Eli Zaretskii wrote:
> > Date: Sat, 23 Dec 2017 21:04:07 +0000
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 29478 <at> debbugs.gnu.org,
> >   npostavs <at> users.sourceforge.net
> > From: Alan Mackenzie <acm <at> muc.de>

[ .... ]

> Alan, any news on this one [the amendments to C-h k/c]?  I'm waiting
> for this to go in before asking Nicolas to produce the next pretest of
> Emacs 26.1.

Apologies.  I committed the two changes to C-h k/c last Saturday and
Sunday.  I should have said so to you directly.  Still, the delay has
allowed Zhang to find a bug in the new C-h k (fixed yesterday), that C-h
k <C-up> threw an error.

So, as far as C-h k and C-h c are concerned, I believe the emacs-26 is
ready for the pretest.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #100 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 29478 <at> debbugs.gnu.org, monnier <at> IRO.UMontreal.CA,
 npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;`C-h k' followed by mouse clicks no
 longer shows down event
Date: Sat, 30 Dec 2017 14:49:21 +0200

> Date: Sat, 30 Dec 2017 11:32:29 +0000
> Cc: monnier <at> IRO.UMontreal.CA, 29478 <at> debbugs.gnu.org,
>   npostavs <at> users.sourceforge.net
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > Alan, any news on this one [the amendments to C-h k/c]?  I'm waiting
> > for this to go in before asking Nicolas to produce the next pretest of
> > Emacs 26.1.
> 
> Apologies.  I committed the two changes to C-h k/c last Saturday and
> Sunday.  I should have said so to you directly.  Still, the delay has
> allowed Zhang to find a bug in the new C-h k (fixed yesterday), that C-h
> k <C-up> threw an error.
> 
> So, as far as C-h k and C-h c are concerned, I believe the emacs-26 is
> ready for the pretest.

Great, thanks.

Message #103 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Tue, 02 Jan 2018 22:56:55 -0500

> This patch doesn't apply to the master branch, so I couldn't test it.

It was meant to be read rather than to be applied.
If you want a patch that applies, the one below should work.


        Stefan


diff --git a/lisp/help.el b/lisp/help.el
index 014af5141e..1d985cb4a6 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -593,19 +593,26 @@ help-key-description
 	    string
 	  (format "%s (translated from %s)" string otherstring))))))
 
+(defun help--first-event (keyseq)
+  (when (> (length keyseq) 0)
+    (aref keyseq (if (and (symbolp (aref keyseq 0))
+		          (> (length keyseq) 1)
+		          (consp (aref keyseq 1)))
+                     ;; Look at the second event when the first
+                     ;; is a pseudo-event like `mode-line' of `left-fringe'.
+		     1
+	           0))))
+
 (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)))
+      (error "Missing `untranslated'!"))
+  (let* ((event (help--first-event key))
 	 (modifiers (event-modifiers event))
 	 (mouse-msg (if (or (memq 'click modifiers) (memq 'down modifiers)
-			    (memq 'drag modifiers)) " at that spot" ""))
+			    (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)
@@ -626,22 +633,26 @@ help--analyze-key
          (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.
+(defun describe-key-briefly (key-list &optional insert)
+  "Print the name of the functions KEY-LIST invokes.
+KEY-LIST is a list of pairs (SEQ . RAW-SEQ) of key sequences, where
+RAW-SEQ is the untranslated form of the key sequence SEQ.
 If INSERT (the prefix arg) is non-nil, insert the message in the buffer.
-If non-nil, UNTRANSLATED is a vector of the untranslated events.
-It can also be a number in which case the untranslated events from
-the last key hit are used.
 
-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."
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them."
   (interactive
    ;; 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)))
+   (let ((key-list (help-read-key-sequence 'no-mouse-movement)))
+     `(,key-list ,current-prefix-arg)))
+  (let ((msg (mapconcat (lambda (x)
+                          (pcase-let ((`(,seq . ,raw-seq) x))
+                            (car (help--analyze-key seq raw-seq))))
+                        key-list
+                        "\n")))
+    (if insert (insert msg) (message "%s" msg))))
 
 (defun help--key-binding-keymap (key &optional accept-default no-remap position)
   "Return a keymap holding a binding for KEY within current keymaps.
@@ -688,8 +699,7 @@ help--binding-locus
                                               (format "%s-map" mode)))))
                                        minor-mode-map-alist))
                                 (list 'global-map
-                                      (intern-soft (format "%s-map" major-mode)))))
-              found)
+                                      (intern-soft (format "%s-map" major-mode))))))
           ;; Look into these advertised symbols first.
           (dolist (sym advertised-syms)
             (when (and
@@ -707,224 +717,98 @@ help--binding-locus
           nil)))))
 
 (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.
+  "Read \"a\" key sequence from the user.
+Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
+sequence, and RAW-SEQ is its untranslated form.
 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)
     (unwind-protect
-        (let (key keys down-ev discarded-up)
+        (let (last-modifiers key-list)
           ;; 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 "\
+              ;; Read at least one key-sequence.
+              (or (null key-list)
+                  ;; After a down event, also read the (presumably) following
+                  ;; up-event.
+                  (memq 'down last-modifiers)
+                  ;; After a click, see if a double click is on the way.
+                  (and (memq 'click last-modifiers)
+                       (not (sit-for (/ double-click-time 1000.0) t))))
+            (let* ((seq (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)))
-                 (or
-                  (and no-mouse-movement
-                       (string-match "mouse-movement" keyname))
-                  (progn (push key keys) nil)
-                  (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
-                                     keyname)
-                       (progn
-                         ;; Discard events (e.g. <help-echo>) which might
-                         ;; spuriously trigger the `sit-for'.
-                         (sleep-for 0.01)
-                         (while (read-event nil nil 0.01))
-                         (not (sit-for
-                               (if (numberp double-click-time)
-                                   (/ double-click-time 1000.0)
-                                 3.0)
-                               t))))))))
-          ;; When we have a sequence of mouse events, discard the most
-          ;; recent ones till we find one with a binding.
-          (let ((keys-1 keys))
-            (while (and keys-1
-                        (not (key-binding (car keys-1))))
-              ;; If we discard the last event, and this was a mouse
-              ;; up, remember this.
-              (if (and (eq keys-1 keys)
-                       (vectorp (car keys-1))
-                       (let* ((last-idx (1- (length (car keys-1))))
-                              (last (aref (car keys-1) last-idx)))
-                         (and (eventp last)
-                              (memq 'click (event-modifiers last)))))
-                  (setq discarded-up t))
-              (setq keys-1 (cdr keys-1)))
-            (if keys-1
-                (setq key (car keys-1))))
-          (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 (not discarded-up) ; Don't attempt to ignore the up-event twice.
-                (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 (setq down-ev (aref key 0)))
-                         (memq 'down (event-modifiers down-ev))
-                         ;; 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 (setq down-ev (aref key 1)))
-                         (memq 'down (event-modifiers down-ev))))
-                (if (and (terminal-parameter nil 'xterm-mouse-mode)
-                         (equal (terminal-parameter nil 'xterm-mouse-last-down)
-                                down-ev))
-                    (aref (read-key-sequence-vector nil) 0)
-                  (read-event)))))
+                   (raw-seq (this-single-command-raw-keys))
+                   (key0 (when (> (length seq) 0)
+                           (aref seq 0)))
+                   (base (event-basic-type key0))
+                   (modifiers (event-modifiers key0)))
+              (cond
+               ((and no-mouse-movement (eq base 'mouse-movement)) nil)
+               ((eq base 'help-echo) nil)
+               (t
+                (setq last-modifiers modifiers)
+                (push (cons seq raw-seq) key-list)))))
+          (nreverse key-list))
       ;; 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 help-downify-mouse-event-type (base)
-  "Add \"down-\" to BASE if it is not already there.
-BASE is a symbol, a mouse event type.  If the modification is done,
-return the new symbol.  Otherwise return nil."
-  (let ((base-s (symbol-name base)))
-    ;; Note: the order of the components in the following string is
-    ;; determined by `apply_modifiers_uncached' in src/keyboard.c.
-    (string-match "\\(A-\\)?\
-\\(C-\\)?\
-\\(H-\\)?\
-\\(M-\\)?\
-\\(S-\\)?\
-\\(s-\\)?\
-\\(double-\\)?\
-\\(triple-\\)?\
-\\(up-\\)?\
-\\(\\(down-\\)?\\)\
-\\(drag-\\)?" base-s)
-    (when (and (null (match-beginning 11)) ; "down-"
-               (null (match-beginning 12))) ; "drag-"
-      (intern (replace-match "down-" t t base-s 10)) )))
-
-(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,
+(defun describe-key (key-list)
+  "Display documentation of the function invoked by KEY-LIST.
+KEY-LIST can be any kind of a key sequence; it can include keyboard events,
 mouse events, and/or menu events.  When calling from a program,
-pass KEY as a string or a vector.
-
-If non-nil, UNTRANSLATED is a vector of the corresponding untranslated events.
-It can also be a number, in which case the untranslated events from
-the last key sequence entered are used.
-UP-EVENT is the up-event that was discarded by reading KEY, or nil.
+pass KEY-LIST as a list of elements (SEQ . RAW-SEQ) where SEQ is
+a key-sequence and RAW-SEQ is its untranslated form.
+
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them."
+  (interactive (list (help-read-key-sequence)))
+  (let ((buf (current-buffer))
+        (info-list
+         (mapcar (lambda (x)
+                   (pcase-let* ((`(,seq . ,raw-seq) x)
+                                (`(,brief-desc ,defn ,event ,_mouse-msg)
+                                 (help--analyze-key seq raw-seq))
+                                (locus
+                                 (help--binding-locus seq (event-start event))))
+                     `(,seq ,brief-desc ,defn ,locus)))
+                 key-list)))
+    (help-setup-xref (list (lambda (key-list)
+                             (with-current-buffer (if (buffer-live-p buf)
+                                                      buf (current-buffer))
+                               (describe-key key-list)))
+                           key-list)
+		     (called-interactively-p 'interactive))
+    (with-help-window (help-buffer)
+      (when (> (length info-list) 1)
+        ;; FIXME: Make this into clickable hyperlinks.
+        (princ "There were several key-sequences:\n\n")
+        (princ (mapconcat (lambda (info)
+                             (pcase-let ((`(,_seq ,brief-desc ,_defn ,_locus)
+                                          info))
+                               (concat "  " brief-desc)))
+                           info-list
+                           "\n"))
+        (princ "\n\nThey're all described below."))
+      (pcase-dolist (`(,seq ,brief-desc ,defn ,locus)
+                     info-list)
+        (when (> (length info-list) 1)
+          (princ (format "\n\n----------------- event `%s' ----------------\n\n"
+                          (key-description seq))))
 
-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)))
-     `(,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))
-      ;; Need to do this before erasing *Help* buffer in case event
-      ;; is a mouse click in an existing *Help* buffer.
-      (when up-event
-	(setq ev-type (event-basic-type up-event))
-	(let ((sequence (vector up-event)))
-	  (when (and (eq ev-type 'mouse-1)
-		     mouse-1-click-follows-link
-		     (not (eq mouse-1-click-follows-link 'double))
-		     (setq mouse-1-remapped
-			   (mouse-on-link-p (event-start up-event))))
-	    (setq mouse-1-tricky (and (integerp mouse-1-click-follows-link)
-				      (> mouse-1-click-follows-link 0)))
-	    (cond ((stringp mouse-1-remapped)
-		   (setq sequence mouse-1-remapped))
-		  ((vectorp mouse-1-remapped)
-		   (setcar up-event (elt mouse-1-remapped 0)))
-		  (t (setcar up-event 'mouse-2))))
-	  (setq defn-up (key-binding sequence nil nil (event-start up-event)))
-          (setq key-locus-up (help--binding-locus sequence (event-start up-event)))
-	  (when mouse-1-tricky
-	    (setq sequence (vector 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))))))
-      (with-help-window (help-buffer)
         (princ brief-desc)
-        (let ((key-locus (help--binding-locus key (event-start event))))
-          (when key-locus
-            (princ (format " (found in %s)" key-locus))))
+        (when locus
+          (princ (format " (found in %s)" locus)))
         (princ ", which is ")
-	(describe-function-1 defn)
-        (when (vectorp key)
-          (let* ((last (1- (length key)))
-                 (elt (aref key last))
-                 (elt-1 (if (listp elt) (copy-sequence elt) elt))
-                 key-1 down-event-type)
-            (when (and (listp elt-1)
-                       (symbolp (car elt-1))
-                       (setq down-event-type (help-downify-mouse-event-type
-                                              (car elt-1))))
-              (setcar elt-1 down-event-type)
-              (setq key-1 (vector elt-1))
-              (when (key-binding key-1)
-                (princ (format "
-
-For documentation of the corresponding mouse down event <%s>,
-click and hold the mouse button longer than %s second(s)."
-                               down-event-type (if (numberp double-click-time)
-                                                   (/ double-click-time 1000.0)
-                                                 3)))))))
-	(when up-event
-	  (unless (or (null defn-up)
-		      (integerp defn-up)
-		      (equal defn-up 'undefined))
-	    (princ (format "
-
------------------ up-event %s----------------
-
-%s%s%s runs the command %S%s, which is "
-			   (if mouse-1-tricky "(short click) " "")
-			   (key-description (vector up-event))
-			   mouse-msg
-			   (if mouse-1-remapped
-                               " is remapped to <mouse-2>, which" "")
-			   defn-up (if key-locus-up
-                                       (format " (found in %s)" key-locus-up)
-                                     "")))
-	    (describe-function-1 defn-up))
-	  (unless (or (null defn-up-tricky)
-		      (integerp defn-up-tricky)
-		      (eq defn-up-tricky 'undefined))
-	    (princ (format "
-
------------------ up-event (long click) ----------------
-
-Pressing <%S>%s for longer than %d milli-seconds
-runs the command %S%s, which is "
-			   ev-type mouse-msg
-			   mouse-1-click-follows-link
-			   defn-up-tricky (if key-locus-up-tricky
-                                              (format " (found in %s)" key-locus-up-tricky)
-                                            "")))
-	    (describe-function-1 defn-up-tricky)))))))
+	(describe-function-1 defn)))))
 
 (defun describe-mode (&optional buffer)
   "Display documentation of current major mode and minor modes.

Message #106 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 06 Jan 2018 19:40:57 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: acm <at> muc.de,  29478 <at> debbugs.gnu.org,  npostavs <at> users.sourceforge.net
> Date: Tue, 02 Jan 2018 22:56:55 -0500
> 
> > This patch doesn't apply to the master branch, so I couldn't test it.
> 
> It was meant to be read rather than to be applied.

Sorry, I'm not good at reading such large patches.

> If you want a patch that applies, the one below should work.

Thanks.  It needs some more work.  E.g., "C-h k C-mouse-1" signals an
error:

  help-fns--analyze-function: Symbol’s function definition is void: nil

and "C-h k C-mouse-3" followed by a menu selection asks for another
key or mouse click, although it already has got a full key sequence.

In general, I see the idea is to show both down-mouse-N event and
mouse-N event, both with "C-h c" and "C-h k".  That could be okay, but
why show undefined sequences?  E.g, "C-h c S-mouse-1" shows this in
the echo area:

  <S-down-mouse-1> at that spot runs the command mouse-appearance-menu
  <S-mouse-1> at that spot is undefined

I'd expect the second line not to appear.

Also, if both events are bound, I think we should show the mouse-N
event before the down-mouse-N event, because the former is much more
important and normally is the one that's bound.

Thanks.

Message #109 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: [SUSPECTED SPAM] Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 07 Jan 2018 10:31:38 -0500

>> If you want a patch that applies, the one below should work.
> Thanks.  It needs some more work.

No doubt.

> E.g., "C-h k C-mouse-1" signals an error:
>   help-fns--analyze-function: Symbol’s function definition is void: nil

Thanks.  I'll look into fixing this...

> and "C-h k C-mouse-3" followed by a menu selection asks for another
> key or mouse click, although it already has got a full key sequence.

... and this.

> In general, I see the idea is to show both down-mouse-N event and
> mouse-N event, both with "C-h c" and "C-h k".  That could be okay, but
> why show undefined sequences?  E.g, "C-h c S-mouse-1" shows this in
> the echo area:
>
>   <S-down-mouse-1> at that spot runs the command mouse-appearance-menu
>   <S-mouse-1> at that spot is undefined

I did not make a special effort to do that, but I also didn't feel like
stripping them away because sometimes it's useful to know what is the
event name.  E.g. you want to bind something to "shifted left mouse
click" because you noticed it doesn't do anything, so you already know
it's undefined, but you don't yet know how to spell it.

I think it's also useful to show to the user than there were two events.

> Also, if both events are bound, I think we should show the mouse-N
> event before the down-mouse-N event, because the former is much more
> important and normally is the one that's bound.

If the down event is unbound it's not shown (this is actually not
a feature I actively tried to obtained: it's just the result of
read-key-sequence dropping the down event if it's not bound).

And I think it's important to preserve the order of the events.
As a side-benefit it reduces the amount of ad-hoc code that needs to
know about what is a down event or a mouse event and so on.


        Stefan

Message #112 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [SUSPECTED SPAM] Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 07 Jan 2018 19:46:05 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: acm <at> muc.de,  29478 <at> debbugs.gnu.org,  npostavs <at> users.sourceforge.net
> Date: Sun, 07 Jan 2018 10:31:38 -0500
> 
> > In general, I see the idea is to show both down-mouse-N event and
> > mouse-N event, both with "C-h c" and "C-h k".  That could be okay, but
> > why show undefined sequences?  E.g, "C-h c S-mouse-1" shows this in
> > the echo area:
> >
> >   <S-down-mouse-1> at that spot runs the command mouse-appearance-menu
> >   <S-mouse-1> at that spot is undefined
> 
> I did not make a special effort to do that, but I also didn't feel like
> stripping them away because sometimes it's useful to know what is the
> event name.  E.g. you want to bind something to "shifted left mouse
> click" because you noticed it doesn't do anything, so you already know
> it's undefined, but you don't yet know how to spell it.

We don't show unbound sequences anywhere else, except if they are
explicitly asked about.  Not sure why this case should be different.

> I think it's also useful to show to the user than there were two events.

I don't think I agree.  Moreover, when dragging there are more than 2
events, but we only show 2.

> > Also, if both events are bound, I think we should show the mouse-N
> > event before the down-mouse-N event, because the former is much more
> > important and normally is the one that's bound.
> 
> If the down event is unbound it's not shown (this is actually not
> a feature I actively tried to obtained: it's just the result of
> read-key-sequence dropping the down event if it's not bound).

That's an inconsistency I don't think I like.

> And I think it's important to preserve the order of the events.

The order reversed is still an order ;-)

> As a side-benefit it reduces the amount of ad-hoc code that needs to
> know about what is a down event or a mouse event and so on.

Yes, correct solutions frequently need more messy code ;-)

Message #115 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [SUSPECTED SPAM] Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 07 Jan 2018 13:03:19 -0500

> We don't show unbound sequences anywhere else, except if they are
> explicitly asked about.  Not sure why this case should be different.

Try C-h c C-H-mouse-1 and it will dutifully report that C-H-mouse-1
is undefined.  IOW, this is really not a new behavior in `C-h c`.

>> I think it's also useful to show to the user than there were two events.
> I don't think I agree.  Moreover, when dragging there are more than 2
> events, but we only show 2.

Indeed, there's a whole bunch of options in terms of how much detail we
may want to show.

>> > Also, if both events are bound, I think we should show the mouse-N
>> > event before the down-mouse-N event, because the former is much more
>> > important and normally is the one that's bound.
>> 
>> If the down event is unbound it's not shown (this is actually not
>> a feature I actively tried to obtained: it's just the result of
>> read-key-sequence dropping the down event if it's not bound).
> That's an inconsistency I don't think I like.

Neither do I.  But it would require a fairly nasty hack to try and make
it behave differently, and given all the key remapping we do, there will
always be such inconsistencies, I think.

OTOH, for text-terminals, we add a "(translated from <escape-sequence>)"
and we could do the same here (that's what my patch originally did, by
the way, and that's what I've been using all these years since I think
it's very valuable information), which would say:

   <C-H-mouse-1> (translated from <C-H-down-mouse-1> <C-H-mouse-1>)
   at that spot is undefined

thus exposing the fact that there was also a <C-H-down-mouse-1> event
that got dropped along the way.

>> And I think it's important to preserve the order of the events.
> The order reversed is still an order ;-)

I guess if we reverse them all (i.e. always show events last-to-first),
I could live with it.

>> As a side-benefit it reduces the amount of ad-hoc code that needs to
>> know about what is a down event or a mouse event and so on.
> Yes, correct solutions frequently need more messy code ;-)

In my experience it too often leads to code which is "more correct" in
90% of the cases but inconsistent in the rest.


        Stefan

Message #118 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Fri, 26 Jan 2018 17:00:15 -0500

>> If you want a patch that applies, the one below should work.
> Thanks.  It needs some more work.  E.g., "C-h k C-mouse-1" signals an
> error:
>
>   help-fns--analyze-function: Symbol’s function definition is void: nil

The patch below fixes this...

> and "C-h k C-mouse-3" followed by a menu selection asks for another
> key or mouse click, although it already has got a full key sequence.

...and this.

> In general, I see the idea is to show both down-mouse-N event and
> mouse-N event, both with "C-h c" and "C-h k".  That could be okay, but
> why show undefined sequences?  E.g, "C-h c S-mouse-1" shows this in
> the echo area:
>
>   <S-down-mouse-1> at that spot runs the command mouse-appearance-menu
>   <S-mouse-1> at that spot is undefined
>
> I'd expect the second line not to appear.

About that, I wrote:

    OTOH, for text-terminals, we add a "(translated from <escape-sequence>)"
    and we could do the same here (that's what my patch originally did, by
    the way, and that's what I've been using all these years since I think
    it's very valuable information), which would say:
    
       <C-H-mouse-1> (translated from <C-H-down-mouse-1> <C-H-mouse-1>)
       at that spot is undefined

but I misremembered, the above also appears in vanilla Emacs.  Regarding
your example:

    <S-down-mouse-1> at that spot runs the command mouse-appearance-menu
    <S-mouse-1> at that spot is undefined

it's hard to do much better: the S-mouse-1 event is eaten by
`mouse-appearance-menu` but it's basically impossible to determine
that automatically.

And if you want to "not show undefined sequences", does that mean we
don't say anything at all for `C-h k M-_` rather than "M-_ is undefined"?
How 'bout `C-h c M-S-double-mouse-1` which with my code says (courtesy
copy-next-command-output (!)):

    <M-S-mouse-1> (translated from <M-S-down-mouse-1> <M-S-mouse-1>) at that spot is undefined
    <M-S-double-mouse-1> (translated from <M-S-double-down-mouse-1> <M-S-double-mouse-1>) is undefined

We can probably come up with some heuristic to keep "at least one line
of output" or something, but I think it's more useful for the user to
report all the events and their binding or lack thereof since we don't
really know what the user is looking for.

All in all, the behavior provided in the patch below may not be always
100% ideal for everyone, but the code is simpler, more robust, and gives
more information.

The details of the display can still be improved, but it's already an
improvement over what we have now, so if there's no objection I'll
install it as-is.


        Stefan


diff --git a/lisp/help.el b/lisp/help.el
index 014af5141e..b992e21ad2 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -593,19 +593,26 @@ help-key-description
 	    string
 	  (format "%s (translated from %s)" string otherstring))))))
 
+(defun help--first-event (keyseq)
+  (when (> (length keyseq) 0)
+    (aref keyseq (if (and (symbolp (aref keyseq 0))
+		          (> (length keyseq) 1)
+		          (consp (aref keyseq 1)))
+                     ;; Look at the second event when the first
+                     ;; is a pseudo-event like `mode-line' of `left-fringe'.
+		     1
+	           0))))
+
 (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)))
+      (error "Missing `untranslated'!"))
+  (let* ((event (help--first-event key))
 	 (modifiers (event-modifiers event))
 	 (mouse-msg (if (or (memq 'click modifiers) (memq 'down modifiers)
-			    (memq 'drag modifiers)) " at that spot" ""))
+			    (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)
@@ -626,22 +633,26 @@ help--analyze-key
          (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.
+(defun describe-key-briefly (key-list &optional insert)
+  "Print the name of the functions KEY-LIST invokes.
+KEY-LIST is a list of pairs (SEQ . RAW-SEQ) of key sequences, where
+RAW-SEQ is the untranslated form of the key sequence SEQ.
 If INSERT (the prefix arg) is non-nil, insert the message in the buffer.
-If non-nil, UNTRANSLATED is a vector of the untranslated events.
-It can also be a number in which case the untranslated events from
-the last key hit are used.
 
-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."
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them."
   (interactive
    ;; 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)))
+   (let ((key-list (help-read-key-sequence 'no-mouse-movement)))
+     `(,key-list ,current-prefix-arg)))
+  (let ((msg (mapconcat (lambda (x)
+                          (pcase-let ((`(,seq . ,raw-seq) x))
+                            (car (help--analyze-key seq raw-seq))))
+                        key-list
+                        "\n")))
+    (if insert (insert msg) (message "%s" msg))))
 
 (defun help--key-binding-keymap (key &optional accept-default no-remap position)
   "Return a keymap holding a binding for KEY within current keymaps.
@@ -688,8 +699,7 @@ help--binding-locus
                                               (format "%s-map" mode)))))
                                        minor-mode-map-alist))
                                 (list 'global-map
-                                      (intern-soft (format "%s-map" major-mode)))))
-              found)
+                                      (intern-soft (format "%s-map" major-mode))))))
           ;; Look into these advertised symbols first.
           (dolist (sym advertised-syms)
             (when (and
@@ -707,224 +717,98 @@ help--binding-locus
           nil)))))
 
 (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.
+  "Read \"a\" key sequence from the user.
+Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
+sequence, and RAW-SEQ is its untranslated form.
 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)
     (unwind-protect
-        (let (key keys down-ev discarded-up)
+        (let (last-modifiers key-list)
           ;; 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 "\
+              ;; Read at least one key-sequence.
+              (or (null key-list)
+                  ;; After a down event, also read the (presumably) following
+                  ;; up-event.
+                  (memq 'down last-modifiers)
+                  ;; After a click, see if a double click is on the way.
+                  (and (memq 'click last-modifiers)
+                       (not (sit-for (/ double-click-time 1000.0) t))))
+            (let* ((seq (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)))
-                 (or
-                  (and no-mouse-movement
-                       (string-match "mouse-movement" keyname))
-                  (progn (push key keys) nil)
-                  (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
-                                     keyname)
-                       (progn
-                         ;; Discard events (e.g. <help-echo>) which might
-                         ;; spuriously trigger the `sit-for'.
-                         (sleep-for 0.01)
-                         (while (read-event nil nil 0.01))
-                         (not (sit-for
-                               (if (numberp double-click-time)
-                                   (/ double-click-time 1000.0)
-                                 3.0)
-                               t))))))))
-          ;; When we have a sequence of mouse events, discard the most
-          ;; recent ones till we find one with a binding.
-          (let ((keys-1 keys))
-            (while (and keys-1
-                        (not (key-binding (car keys-1))))
-              ;; If we discard the last event, and this was a mouse
-              ;; up, remember this.
-              (if (and (eq keys-1 keys)
-                       (vectorp (car keys-1))
-                       (let* ((last-idx (1- (length (car keys-1))))
-                              (last (aref (car keys-1) last-idx)))
-                         (and (eventp last)
-                              (memq 'click (event-modifiers last)))))
-                  (setq discarded-up t))
-              (setq keys-1 (cdr keys-1)))
-            (if keys-1
-                (setq key (car keys-1))))
-          (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 (not discarded-up) ; Don't attempt to ignore the up-event twice.
-                (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 (setq down-ev (aref key 0)))
-                         (memq 'down (event-modifiers down-ev))
-                         ;; 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 (setq down-ev (aref key 1)))
-                         (memq 'down (event-modifiers down-ev))))
-                (if (and (terminal-parameter nil 'xterm-mouse-mode)
-                         (equal (terminal-parameter nil 'xterm-mouse-last-down)
-                                down-ev))
-                    (aref (read-key-sequence-vector nil) 0)
-                  (read-event)))))
+                   (raw-seq (this-single-command-raw-keys))
+                   (key0 (when (> (length seq) 0)
+                           (aref seq 0)))
+                   (base (event-basic-type key0))
+                   (modifiers (event-modifiers key0)))
+              (cond
+               ((and no-mouse-movement (eq base 'mouse-movement)) nil)
+               ((eq base 'help-echo) nil)
+               (t
+                (setq last-modifiers modifiers)
+                (push (cons seq raw-seq) key-list)))))
+          (nreverse key-list))
       ;; 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 help-downify-mouse-event-type (base)
-  "Add \"down-\" to BASE if it is not already there.
-BASE is a symbol, a mouse event type.  If the modification is done,
-return the new symbol.  Otherwise return nil."
-  (let ((base-s (symbol-name base)))
-    ;; Note: the order of the components in the following string is
-    ;; determined by `apply_modifiers_uncached' in src/keyboard.c.
-    (string-match "\\(A-\\)?\
-\\(C-\\)?\
-\\(H-\\)?\
-\\(M-\\)?\
-\\(S-\\)?\
-\\(s-\\)?\
-\\(double-\\)?\
-\\(triple-\\)?\
-\\(up-\\)?\
-\\(\\(down-\\)?\\)\
-\\(drag-\\)?" base-s)
-    (when (and (null (match-beginning 11)) ; "down-"
-               (null (match-beginning 12))) ; "drag-"
-      (intern (replace-match "down-" t t base-s 10)) )))
-
-(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,
+(defun describe-key (key-list)
+  "Display documentation of the function invoked by KEY-LIST.
+KEY-LIST can be any kind of a key sequence; it can include keyboard events,
 mouse events, and/or menu events.  When calling from a program,
-pass KEY as a string or a vector.
-
-If non-nil, UNTRANSLATED is a vector of the corresponding untranslated events.
-It can also be a number, in which case the untranslated events from
-the last key sequence entered are used.
-UP-EVENT is the up-event that was discarded by reading KEY, or nil.
+pass KEY-LIST as a list of elements (SEQ . RAW-SEQ) where SEQ is
+a key-sequence and RAW-SEQ is its untranslated form.
+
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them."
+  (interactive (list (help-read-key-sequence)))
+  (let ((buf (current-buffer))
+        (info-list
+         (mapcar (lambda (x)
+                   (pcase-let* ((`(,seq . ,raw-seq) x)
+                                (`(,brief-desc ,defn ,event ,_mouse-msg)
+                                 (help--analyze-key seq raw-seq))
+                                (locus
+                                 (help--binding-locus seq (event-start event))))
+                     `(,seq ,brief-desc ,defn ,locus)))
+                 key-list)))
+    (help-setup-xref (list (lambda (key-list)
+                             (with-current-buffer (if (buffer-live-p buf)
+                                                      buf (current-buffer))
+                               (describe-key key-list)))
+                           key-list)
+		     (called-interactively-p 'interactive))
+    (with-help-window (help-buffer)
+      (when (> (length info-list) 1)
+        ;; FIXME: Make this into clickable hyperlinks.
+        (princ "There were several key-sequences:\n\n")
+        (princ (mapconcat (lambda (info)
+                             (pcase-let ((`(,_seq ,brief-desc ,_defn ,_locus)
+                                          info))
+                               (concat "  " brief-desc)))
+                           info-list
+                           "\n"))
+        (princ "\n\nThey're all described below."))
+      (pcase-dolist (`(,seq ,brief-desc ,defn ,locus)
+                     info-list)
+        (when (> (length info-list) 1)
+          (princ (format "\n\n----------------- event `%s' ----------------\n\n"
+                          (key-description seq))))
 
-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)))
-     `(,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))
-      ;; Need to do this before erasing *Help* buffer in case event
-      ;; is a mouse click in an existing *Help* buffer.
-      (when up-event
-	(setq ev-type (event-basic-type up-event))
-	(let ((sequence (vector up-event)))
-	  (when (and (eq ev-type 'mouse-1)
-		     mouse-1-click-follows-link
-		     (not (eq mouse-1-click-follows-link 'double))
-		     (setq mouse-1-remapped
-			   (mouse-on-link-p (event-start up-event))))
-	    (setq mouse-1-tricky (and (integerp mouse-1-click-follows-link)
-				      (> mouse-1-click-follows-link 0)))
-	    (cond ((stringp mouse-1-remapped)
-		   (setq sequence mouse-1-remapped))
-		  ((vectorp mouse-1-remapped)
-		   (setcar up-event (elt mouse-1-remapped 0)))
-		  (t (setcar up-event 'mouse-2))))
-	  (setq defn-up (key-binding sequence nil nil (event-start up-event)))
-          (setq key-locus-up (help--binding-locus sequence (event-start up-event)))
-	  (when mouse-1-tricky
-	    (setq sequence (vector 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))))))
-      (with-help-window (help-buffer)
         (princ brief-desc)
-        (let ((key-locus (help--binding-locus key (event-start event))))
-          (when key-locus
-            (princ (format " (found in %s)" key-locus))))
+        (when locus
+          (princ (format " (found in %s)" locus)))
         (princ ", which is ")
-	(describe-function-1 defn)
-        (when (vectorp key)
-          (let* ((last (1- (length key)))
-                 (elt (aref key last))
-                 (elt-1 (if (listp elt) (copy-sequence elt) elt))
-                 key-1 down-event-type)
-            (when (and (listp elt-1)
-                       (symbolp (car elt-1))
-                       (setq down-event-type (help-downify-mouse-event-type
-                                              (car elt-1))))
-              (setcar elt-1 down-event-type)
-              (setq key-1 (vector elt-1))
-              (when (key-binding key-1)
-                (princ (format "
-
-For documentation of the corresponding mouse down event <%s>,
-click and hold the mouse button longer than %s second(s)."
-                               down-event-type (if (numberp double-click-time)
-                                                   (/ double-click-time 1000.0)
-                                                 3)))))))
-	(when up-event
-	  (unless (or (null defn-up)
-		      (integerp defn-up)
-		      (equal defn-up 'undefined))
-	    (princ (format "
-
------------------ up-event %s----------------
-
-%s%s%s runs the command %S%s, which is "
-			   (if mouse-1-tricky "(short click) " "")
-			   (key-description (vector up-event))
-			   mouse-msg
-			   (if mouse-1-remapped
-                               " is remapped to <mouse-2>, which" "")
-			   defn-up (if key-locus-up
-                                       (format " (found in %s)" key-locus-up)
-                                     "")))
-	    (describe-function-1 defn-up))
-	  (unless (or (null defn-up-tricky)
-		      (integerp defn-up-tricky)
-		      (eq defn-up-tricky 'undefined))
-	    (princ (format "
-
------------------ up-event (long click) ----------------
-
-Pressing <%S>%s for longer than %d milli-seconds
-runs the command %S%s, which is "
-			   ev-type mouse-msg
-			   mouse-1-click-follows-link
-			   defn-up-tricky (if key-locus-up-tricky
-                                              (format " (found in %s)" key-locus-up-tricky)
-                                            "")))
-	    (describe-function-1 defn-up-tricky)))))))
+	(describe-function-1 defn)))))
 
 (defun describe-mode (&optional buffer)
   "Display documentation of current major mode and minor modes.

Message #121 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Fri, 26 Jan 2018 17:37:22 -0500

>>> If you want a patch that applies, the one below should work.
>> Thanks.  It needs some more work.  E.g., "C-h k C-mouse-1" signals an
>> error:
>>   help-fns--analyze-function: Symbol’s function definition is void: nil
> The patch below fixes this...

Sorry, wrong patch!

I meant this one,


        Stefan


diff --git a/lisp/help.el b/lisp/help.el
index 014af5141e..1f92c38927 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1,4 +1,4 @@
-;;; help.el --- help commands for Emacs
+;;; help.el --- help commands for Emacs  -*- lexical-binding:t -*-
 
 ;; Copyright (C) 1985-1986, 1993-1994, 1998-2018 Free Software
 ;; Foundation, Inc.
@@ -593,19 +593,26 @@ help-key-description
 	    string
 	  (format "%s (translated from %s)" string otherstring))))))
 
+(defun help--first-event (keyseq)
+  (when (> (length keyseq) 0)
+    (aref keyseq (if (and (symbolp (aref keyseq 0))
+		          (> (length keyseq) 1)
+		          (consp (aref keyseq 1)))
+                     ;; Look at the second event when the first
+                     ;; is a pseudo-event like `mode-line' of `left-fringe'.
+		     1
+	           0))))
+
 (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)))
+      (error "Missing `untranslated'!"))
+  (let* ((event (help--first-event key))
 	 (modifiers (event-modifiers event))
 	 (mouse-msg (if (or (memq 'click modifiers) (memq 'down modifiers)
-			    (memq 'drag modifiers)) " at that spot" ""))
+			    (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)
@@ -626,22 +633,26 @@ help--analyze-key
          (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.
+(defun describe-key-briefly (key-list &optional insert)
+  "Print the name of the functions KEY-LIST invokes.
+KEY-LIST is a list of pairs (SEQ . RAW-SEQ) of key sequences, where
+RAW-SEQ is the untranslated form of the key sequence SEQ.
 If INSERT (the prefix arg) is non-nil, insert the message in the buffer.
-If non-nil, UNTRANSLATED is a vector of the untranslated events.
-It can also be a number in which case the untranslated events from
-the last key hit are used.
 
-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."
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them."
   (interactive
    ;; 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)))
+   (let ((key-list (help-read-key-sequence 'no-mouse-movement)))
+     `(,key-list ,current-prefix-arg)))
+  (let ((msg (mapconcat (lambda (x)
+                          (pcase-let ((`(,seq . ,raw-seq) x))
+                            (car (help--analyze-key seq raw-seq))))
+                        key-list
+                        "\n")))
+    (if insert (insert msg) (message "%s" msg))))
 
 (defun help--key-binding-keymap (key &optional accept-default no-remap position)
   "Return a keymap holding a binding for KEY within current keymaps.
@@ -688,8 +699,7 @@ help--binding-locus
                                               (format "%s-map" mode)))))
                                        minor-mode-map-alist))
                                 (list 'global-map
-                                      (intern-soft (format "%s-map" major-mode)))))
-              found)
+                                      (intern-soft (format "%s-map" major-mode))))))
           ;; Look into these advertised symbols first.
           (dolist (sym advertised-syms)
             (when (and
@@ -707,224 +717,105 @@ help--binding-locus
           nil)))))
 
 (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.
+  "Read \"a\" key sequence from the user.
+Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
+sequence, and RAW-SEQ is its untranslated form.
 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)
     (unwind-protect
-        (let (key keys down-ev discarded-up)
+        (let (last-modifiers key-list)
           ;; 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 "\
+              ;; Read at least one key-sequence.
+              (or (null key-list)
+                  ;; After a down event, also read the (presumably) following
+                  ;; up-event.
+                  (memq 'down last-modifiers)
+                  ;; After a click, see if a double click is on the way.
+                  (and (memq 'click last-modifiers)
+                       (not (sit-for (/ double-click-time 1000.0) t))))
+            (let* ((seq (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)))
-                 (or
-                  (and no-mouse-movement
-                       (string-match "mouse-movement" keyname))
-                  (progn (push key keys) nil)
-                  (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
-                                     keyname)
-                       (progn
-                         ;; Discard events (e.g. <help-echo>) which might
-                         ;; spuriously trigger the `sit-for'.
-                         (sleep-for 0.01)
-                         (while (read-event nil nil 0.01))
-                         (not (sit-for
-                               (if (numberp double-click-time)
-                                   (/ double-click-time 1000.0)
-                                 3.0)
-                               t))))))))
-          ;; When we have a sequence of mouse events, discard the most
-          ;; recent ones till we find one with a binding.
-          (let ((keys-1 keys))
-            (while (and keys-1
-                        (not (key-binding (car keys-1))))
-              ;; If we discard the last event, and this was a mouse
-              ;; up, remember this.
-              (if (and (eq keys-1 keys)
-                       (vectorp (car keys-1))
-                       (let* ((last-idx (1- (length (car keys-1))))
-                              (last (aref (car keys-1) last-idx)))
-                         (and (eventp last)
-                              (memq 'click (event-modifiers last)))))
-                  (setq discarded-up t))
-              (setq keys-1 (cdr keys-1)))
-            (if keys-1
-                (setq key (car keys-1))))
-          (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 (not discarded-up) ; Don't attempt to ignore the up-event twice.
-                (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 (setq down-ev (aref key 0)))
-                         (memq 'down (event-modifiers down-ev))
-                         ;; 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 (setq down-ev (aref key 1)))
-                         (memq 'down (event-modifiers down-ev))))
-                (if (and (terminal-parameter nil 'xterm-mouse-mode)
-                         (equal (terminal-parameter nil 'xterm-mouse-last-down)
-                                down-ev))
-                    (aref (read-key-sequence-vector nil) 0)
-                  (read-event)))))
+                   (raw-seq (this-single-command-raw-keys))
+                   (keyn (when (> (length seq) 0)
+                           (aref seq (1- (length seq)))))
+                   (base (event-basic-type keyn))
+                   (modifiers (event-modifiers keyn)))
+              (cond
+               ((zerop (length seq)))   ;FIXME: Can this happen?
+               ((and no-mouse-movement (eq base 'mouse-movement)) nil)
+               ((eq base 'help-echo) nil)
+               (t
+                (setq last-modifiers modifiers)
+                (push (cons seq raw-seq) key-list)))))
+          (nreverse key-list))
       ;; 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 help-downify-mouse-event-type (base)
-  "Add \"down-\" to BASE if it is not already there.
-BASE is a symbol, a mouse event type.  If the modification is done,
-return the new symbol.  Otherwise return nil."
-  (let ((base-s (symbol-name base)))
-    ;; Note: the order of the components in the following string is
-    ;; determined by `apply_modifiers_uncached' in src/keyboard.c.
-    (string-match "\\(A-\\)?\
-\\(C-\\)?\
-\\(H-\\)?\
-\\(M-\\)?\
-\\(S-\\)?\
-\\(s-\\)?\
-\\(double-\\)?\
-\\(triple-\\)?\
-\\(up-\\)?\
-\\(\\(down-\\)?\\)\
-\\(drag-\\)?" base-s)
-    (when (and (null (match-beginning 11)) ; "down-"
-               (null (match-beginning 12))) ; "drag-"
-      (intern (replace-match "down-" t t base-s 10)) )))
-
-(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,
+(defun describe-key (key-list &optional buffer)
+  "Display documentation of the function invoked by KEY-LIST.
+KEY-LIST can be any kind of a key sequence; it can include keyboard events,
 mouse events, and/or menu events.  When calling from a program,
-pass KEY as a string or a vector.
-
-If non-nil, UNTRANSLATED is a vector of the corresponding untranslated events.
-It can also be a number, in which case the untranslated events from
-the last key sequence entered are used.
-UP-EVENT is the up-event that was discarded by reading KEY, or nil.
-
-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)))
-     `(,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))
-      ;; Need to do this before erasing *Help* buffer in case event
-      ;; is a mouse click in an existing *Help* buffer.
-      (when up-event
-	(setq ev-type (event-basic-type up-event))
-	(let ((sequence (vector up-event)))
-	  (when (and (eq ev-type 'mouse-1)
-		     mouse-1-click-follows-link
-		     (not (eq mouse-1-click-follows-link 'double))
-		     (setq mouse-1-remapped
-			   (mouse-on-link-p (event-start up-event))))
-	    (setq mouse-1-tricky (and (integerp mouse-1-click-follows-link)
-				      (> mouse-1-click-follows-link 0)))
-	    (cond ((stringp mouse-1-remapped)
-		   (setq sequence mouse-1-remapped))
-		  ((vectorp mouse-1-remapped)
-		   (setcar up-event (elt mouse-1-remapped 0)))
-		  (t (setcar up-event 'mouse-2))))
-	  (setq defn-up (key-binding sequence nil nil (event-start up-event)))
-          (setq key-locus-up (help--binding-locus sequence (event-start up-event)))
-	  (when mouse-1-tricky
-	    (setq sequence (vector 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))))))
-      (with-help-window (help-buffer)
-        (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 (vectorp key)
-          (let* ((last (1- (length key)))
-                 (elt (aref key last))
-                 (elt-1 (if (listp elt) (copy-sequence elt) elt))
-                 key-1 down-event-type)
-            (when (and (listp elt-1)
-                       (symbolp (car elt-1))
-                       (setq down-event-type (help-downify-mouse-event-type
-                                              (car elt-1))))
-              (setcar elt-1 down-event-type)
-              (setq key-1 (vector elt-1))
-              (when (key-binding key-1)
-                (princ (format "
-
-For documentation of the corresponding mouse down event <%s>,
-click and hold the mouse button longer than %s second(s)."
-                               down-event-type (if (numberp double-click-time)
-                                                   (/ double-click-time 1000.0)
-                                                 3)))))))
-	(when up-event
-	  (unless (or (null defn-up)
-		      (integerp defn-up)
-		      (equal defn-up 'undefined))
-	    (princ (format "
-
------------------ up-event %s----------------
-
-%s%s%s runs the command %S%s, which is "
-			   (if mouse-1-tricky "(short click) " "")
-			   (key-description (vector up-event))
-			   mouse-msg
-			   (if mouse-1-remapped
-                               " is remapped to <mouse-2>, which" "")
-			   defn-up (if key-locus-up
-                                       (format " (found in %s)" key-locus-up)
-                                     "")))
-	    (describe-function-1 defn-up))
-	  (unless (or (null defn-up-tricky)
-		      (integerp defn-up-tricky)
-		      (eq defn-up-tricky 'undefined))
-	    (princ (format "
-
------------------ up-event (long click) ----------------
-
-Pressing <%S>%s for longer than %d milli-seconds
-runs the command %S%s, which is "
-			   ev-type mouse-msg
-			   mouse-1-click-follows-link
-			   defn-up-tricky (if key-locus-up-tricky
-                                              (format " (found in %s)" key-locus-up-tricky)
-                                            "")))
-	    (describe-function-1 defn-up-tricky)))))))
+pass KEY-LIST as a list of elements (SEQ . RAW-SEQ) where SEQ is
+a key-sequence and RAW-SEQ is its untranslated form.
+
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them."
+  (interactive (list (help-read-key-sequence)))
+  (let* ((buf (or buffer (current-buffer)))
+         (info-list
+          (with-current-buffer buf
+            (mapcar (lambda (x)
+                      (pcase-let* ((`(,seq . ,raw-seq) x)
+                                   (`(,brief-desc ,defn ,event ,_mouse-msg)
+                                    (help--analyze-key seq raw-seq))
+                                   (locus
+                                    (help--binding-locus
+                                     seq (event-start event))))
+                        `(,seq ,brief-desc ,defn ,locus)))
+                    key-list))))
+    (help-setup-xref (list (lambda (key-list)
+                             (describe-key key-list
+                                           (if (buffer-live-p buf) buf)))
+                           key-list)
+		     (called-interactively-p 'interactive))
+    (with-help-window (help-buffer)
+      (when (> (length info-list) 1)
+        ;; FIXME: Make this into clickable hyperlinks.
+        (princ "There were several key-sequences:\n\n")
+        (princ (mapconcat (lambda (info)
+                             (pcase-let ((`(,_seq ,brief-desc ,_defn ,_locus)
+                                          info))
+                               (concat "  " brief-desc)))
+                           info-list
+                           "\n"))
+        (princ "\n\nThey're all described below."))
+      (pcase-dolist (`(,_seq ,brief-desc ,defn ,locus)
+                     info-list)
+        (when defn
+          (when (> (length info-list) 1)
+            (with-current-buffer standard-output
+              (insert "\n\n"
+                      ;; FIXME: Can't eval-when-compile, because string
+                      ;; constant in purespace can't have properties!
+                      (propertize "\n" 'face '(:height 0.1 :inverse-video t))
+                      "\n")))
+
+          (princ brief-desc)
+          (when locus
+            (princ (format " (found in %s)" locus)))
+          (princ ", which is ")
+	  (describe-function-1 defn))))))
 
 (defun describe-mode (&optional buffer)
   "Display documentation of current major mode and minor modes.
@@ -1120,7 +1011,7 @@ lookup-minor-mode-from-indicator
 
 ;;; Automatic resizing of temporary buffers.
 (defcustom temp-buffer-max-height
-  (lambda (buffer)
+  (lambda (_buffer)
     (if (and (display-graphic-p) (eq (selected-window) (frame-root-window)))
 	(/ (x-display-pixel-height) (frame-char-height) 2)
       (/ (- (frame-height) 2) 2)))
@@ -1137,7 +1028,7 @@ temp-buffer-max-height
   :version "24.3")
 
 (defcustom temp-buffer-max-width
-  (lambda (buffer)
+  (lambda (_buffer)
     (if (and (display-graphic-p) (eq (selected-window) (frame-root-window)))
 	(/ (x-display-pixel-width) (frame-char-width) 2)
       (/ (- (frame-width) 2) 2)))

Message #124 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 27 Jan 2018 09:59:15 +0200

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
> Date: Fri, 26 Jan 2018 17:00:15 -0500
> 
> >   <S-down-mouse-1> at that spot runs the command mouse-appearance-menu
> >   <S-mouse-1> at that spot is undefined
> >
> > I'd expect the second line not to appear.
> 
> About that, I wrote:
> 
>     OTOH, for text-terminals, we add a "(translated from <escape-sequence>)"
>     and we could do the same here (that's what my patch originally did, by
>     the way, and that's what I've been using all these years since I think
>     it's very valuable information), which would say:
>     
>        <C-H-mouse-1> (translated from <C-H-down-mouse-1> <C-H-mouse-1>)
>        at that spot is undefined
> 
> but I misremembered, the above also appears in vanilla Emacs.  Regarding
> your example:
> 
>     <S-down-mouse-1> at that spot runs the command mouse-appearance-menu
>     <S-mouse-1> at that spot is undefined
> 
> it's hard to do much better: the S-mouse-1 event is eaten by
> `mouse-appearance-menu` but it's basically impossible to determine
> that automatically.
> 
> And if you want to "not show undefined sequences", does that mean we
> don't say anything at all for `C-h k M-_` rather than "M-_ is undefined"?
> How 'bout `C-h c M-S-double-mouse-1` which with my code says (courtesy
> copy-next-command-output (!)):
> 
>     <M-S-mouse-1> (translated from <M-S-down-mouse-1> <M-S-mouse-1>) at that spot is undefined
>     <M-S-double-mouse-1> (translated from <M-S-double-down-mouse-1> <M-S-double-mouse-1>) is undefined
> 
> We can probably come up with some heuristic to keep "at least one line
> of output" or something, but I think it's more useful for the user to
> report all the events and their binding or lack thereof since we don't
> really know what the user is looking for.

I'd prefer the "undefined" lines to appear only if none of the
possible interpretations is bound to a command, yes.

Message #127 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sat, 27 Jan 2018 10:28:30 +0200

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
> Date: Fri, 26 Jan 2018 17:37:22 -0500
> 
> >>> If you want a patch that applies, the one below should work.
> >> Thanks.  It needs some more work.  E.g., "C-h k C-mouse-1" signals an
> >> error:
> >>   help-fns--analyze-function: Symbol’s function definition is void: nil
> > The patch below fixes this...
> 
> Sorry, wrong patch!
> 
> I meant this one,
> 
> 
>         Stefan
> 
> 
> diff --git a/lisp/help.el b/lisp/help.el
> index 014af5141e..1f92c38927 100644
> --- a/lisp/help.el
> +++ b/lisp/help.el
> @@ -1,4 +1,4 @@
> -;;; help.el --- help commands for Emacs
> +;;; help.el --- help commands for Emacs  -*- lexical-binding:t -*-
>  
>  ;; Copyright (C) 1985-1986, 1993-1994, 1998-2018 Free Software
>  ;; Foundation, Inc.
> @@ -593,19 +593,26 @@ help-key-description
>  	    string
>  	  (format "%s (translated from %s)" string otherstring))))))
>  
> +(defun help--first-event (keyseq)
> +  (when (> (length keyseq) 0)
> +    (aref keyseq (if (and (symbolp (aref keyseq 0))
> +		          (> (length keyseq) 1)
> +		          (consp (aref keyseq 1)))
> +                     ;; Look at the second event when the first
> +                     ;; is a pseudo-event like `mode-line' of `left-fringe'.
> +		     1
> +	           0))))

Should we perhaps test explicitly for pseudo-events, in case some day
we have events we don't want to skip?  Or maybe have an assertion
here, for that future day?

> -(defun describe-key-briefly (&optional key insert untranslated)
> -  "Print the name of the function KEY invokes.  KEY is a string.
> +(defun describe-key-briefly (key-list &optional insert)
> +  "Print the name of the functions KEY-LIST invokes.

Is it a good idea to change the signature of this function?  I'm sure
it's used in many places, including outside Emacs.

>  (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.
> +  "Read \"a\" key sequence from the user.

Why "a" in quotes?

> +Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
> +sequence, and RAW-SEQ is its untranslated form.

This also changes the format of the return value in incompatible way,
doesn't it?  Is this incompatible change justified?

> -(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,
> +(defun describe-key (key-list &optional buffer)
> +  "Display documentation of the function invoked by KEY-LIST.
> +KEY-LIST can be any kind of a key sequence; it can include keyboard events,
>  mouse events, and/or menu events.  When calling from a program,

Same issue here.

Thanks.

Message #130 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 28 Jan 2018 11:02:26 -0500

>> +(defun help--first-event (keyseq)
>> +  (when (> (length keyseq) 0)
>> +    (aref keyseq (if (and (symbolp (aref keyseq 0))
>> +		          (> (length keyseq) 1)
>> +		          (consp (aref keyseq 1)))
>> +                     ;; Look at the second event when the first
>> +                     ;; is a pseudo-event like `mode-line' of `left-fringe'.
>> +		     1
>> +	           0))))
>
> Should we perhaps test explicitly for pseudo-events, in case some day
> we have events we don't want to skip?  Or maybe have an assertion
> here, for that future day?

[ This code was moved from help--analyze-key, so improving it is
  orthogonal to the patch under discussion.  Especially now that I see
  that the reason why I extracted this function (to use it in
  help-read-key-sequence) doesn't apply any more.  ]

You mean we should check that (aref keyseq 0) is within
(mode-line left-fringe right-fringe ...)?

I guess we could but what could we do with the result of the check?
Signaling an error would not be very useful to the user.

Note that the output of this function is only used to extract the
modifiers of the event to then decide whether we add the text "at that
spot" or not, so it's really not important.

>> -(defun describe-key-briefly (&optional key insert untranslated)
>> -  "Print the name of the function KEY invokes.  KEY is a string.
>> +(defun describe-key-briefly (key-list &optional insert)
>> +  "Print the name of the functions KEY-LIST invokes.
>
> Is it a good idea to change the signature of this function?

It's indeed the main issue I see with the patch: it completely changes
the signature of several functions.

> I'm sure it's used in many places, including outside Emacs.

I couldn't find any use of those functions outside of Emacs (I did find
uses as commands, but not as functions).  But indeed I now see that
there are some uses within Emacs, which I'd need to fix.

I'll change the new code to accept the old calling convention.

>>  (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.
>> +  "Read \"a\" key sequence from the user.
>
> Why "a" in quotes?

Because it can also return several key sequences.  But now that you
mention it, it's better not to put those quotes and simply explain it
better in the subsequent text.

>> +Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
>> +sequence, and RAW-SEQ is its untranslated form.
> This also changes the format of the return value in incompatible way,
> doesn't it?

Yes.

> Is this incompatible change justified?

Yes: we can't recover all the RAW-SEQs after the call (only the
RAW-SEQ of the last element which can still be obtained from
this-single-command-raw-keys), so the old return value format throws
away information we need to emit complete information.

In any case, this function is (or at least should be) internal, so I now
renamed it with a "help--" prefix.  If external callers show up, we may
have to revisit this, but I couldn't find any.

>> -(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,
>> +(defun describe-key (key-list &optional buffer)
>> +  "Display documentation of the function invoked by KEY-LIST.
>> +KEY-LIST can be any kind of a key sequence; it can include keyboard events,
>>  mouse events, and/or menu events.  When calling from a program,
> Same issue here.

I'll also add compatibility to the old calling convention.

> > We can probably come up with some heuristic to keep "at least one line
> > of output" or something, but I think it's more useful for the user to
> > report all the events and their binding or lack thereof since we don't
> > really know what the user is looking for.

> I'd prefer the "undefined" lines to appear only if none of the
> possible interpretations is bound to a command, yes.

Not sure what this means.


        Stefan

Message #133 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Sun, 28 Jan 2018 19:18:57 +0200

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
> Date: Sun, 28 Jan 2018 11:02:26 -0500
> 
> > > We can probably come up with some heuristic to keep "at least one line
> > > of output" or something, but I think it's more useful for the user to
> > > report all the events and their binding or lack thereof since we don't
> > > really know what the user is looking for.
> 
> > I'd prefer the "undefined" lines to appear only if none of the
> > possible interpretations is bound to a command, yes.
> 
> Not sure what this means.

It means I'm okay with this:

  FOO is undefined

but prefer not to see the second line in this:

  FOO at that spot runs BAR
  down-FOO at that spot is undefined

Message #136 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Mon, 29 Jan 2018 16:54:09 -0500

> It means I'm okay with this:
>
>   FOO is undefined
>
> but prefer not to see the second line in this:
>
>   FOO at that spot runs BAR
>   down-FOO at that spot is undefined

OK, in help--filter-info-list in the patch below I follow the rule of
getting rid of all `undefined` keys except when there are only undefined
keys in which case I keep the last one (e.g. the triple-mouse-N).


        Stefan


diff --git a/lisp/help.el b/lisp/help.el
index 014af5141e..ccac854d8b 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1,4 +1,4 @@
-;;; help.el --- help commands for Emacs
+;;; help.el --- help commands for Emacs  -*- lexical-binding:t -*-
 
 ;; Copyright (C) 1985-1986, 1993-1994, 1998-2018 Free Software
 ;; Foundation, Inc.
@@ -593,19 +592,27 @@
 	    string
 	  (format "%s (translated from %s)" string otherstring))))))
 
+(defun help--binding-undefined-p (defn)
+  (or (null defn) (integerp defn) (equal defn 'undefined)))
+
 (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)))
+      (error "Missing `untranslated'!"))
+  (let* ((event (when (> (length key) 0)
+                  (aref key (if (and (symbolp (aref key 0))
+		                     (> (length key) 1)
+		                     (consp (aref key 1)))
+                                ;; Look at the second event when the first
+                                ;; is a pseudo-event like `mode-line' or
+                                ;; `left-fringe'.
+		                1
+	                      0))))
 	 (modifiers (event-modifiers event))
 	 (mouse-msg (if (or (memq 'click modifiers) (memq 'down modifiers)
-			    (memq 'drag modifiers)) " at that spot" ""))
+			    (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)
@@ -621,27 +628,47 @@ help--analyze-key
     (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))
+       (if (help--binding-undefined-p defn)
            (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.
+(defun help--filter-info-list (info-list i)
+  "Drop the undefined keys."
+  (or
+   ;; Remove all `undefined' keys.
+   (delq nil (mapcar (lambda (x)
+                       (unless (help--binding-undefined-p (nth i x)) x))
+                     info-list))
+   ;; If nothing left, then keep one (the last one).
+   (last info-list)))
+
+(defun describe-key-briefly (&optional key-list insert untranslated)
+  "Print the name of the functions KEY-LIST invokes.
+KEY-LIST is a list of pairs (SEQ . RAW-SEQ) of key sequences, where
+RAW-SEQ is the untranslated form of the key sequence SEQ.
 If INSERT (the prefix arg) is non-nil, insert the message in the buffer.
-If non-nil, UNTRANSLATED is a vector of the untranslated events.
-It can also be a number in which case the untranslated events from
-the last key hit are used.
 
-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."
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them."
+  (declare (advertised-calling-convention (key-list &optional insert) "27.1"))
   (interactive
    ;; 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)))
+   (let ((key-list (help--read-key-sequence 'no-mouse-movement)))
+     `(,key-list ,current-prefix-arg)))
+  (when (arrayp key-list)
+    ;; Old calling convention, changed
+    (setq key-list (list (cons key-list
+                               (if (numberp untranslated)
+                                   (this-single-command-raw-keys)
+                                 untranslated)))))
+  (let* ((info-list (mapcar (lambda (kr)
+                              (help--analyze-key (car kr) (cdr kr)))
+                            key-list))
+         (msg (mapconcat #'car (help--filter-info-list info-list 1) "\n")))
+    (if insert (insert msg) (message "%s" msg))))
 
 (defun help--key-binding-keymap (key &optional accept-default no-remap position)
   "Return a keymap holding a binding for KEY within current keymaps.
@@ -688,8 +715,7 @@ help--binding-locus
                                               (format "%s-map" mode)))))
                                        minor-mode-map-alist))
                                 (list 'global-map
-                                      (intern-soft (format "%s-map" major-mode)))))
-              found)
+                                      (intern-soft (format "%s-map" major-mode))))))
           ;; Look into these advertised symbols first.
           (dolist (sym advertised-syms)
             (when (and
@@ -706,225 +732,136 @@ help--binding-locus
                (throw 'found x))))
           nil)))))
 
-(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.
+(defun help--read-key-sequence (&optional no-mouse-movement)
+  "Read a key sequence from the user.
+Usually reads a single key sequence, except when that sequence might
+hide another one (e.g. a down event, where the user is interested
+in getting info about the up event, or a click event, where the user
+wants to get info about the double click).
+Return a list of elements of the form (SEQ . RAW-SEQ), where SEQ is a key
+sequence, and RAW-SEQ is its untranslated form.
 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)
     (unwind-protect
-        (let (key keys down-ev discarded-up)
+        (let (last-modifiers key-list)
           ;; 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 "\
+              ;; Read at least one key-sequence.
+              (or (null key-list)
+                  ;; After a down event, also read the (presumably) following
+                  ;; up-event.
+                  (memq 'down last-modifiers)
+                  ;; After a click, see if a double click is on the way.
+                  (and (memq 'click last-modifiers)
+                       (not (sit-for (/ double-click-time 1000.0) t))))
+            (let* ((seq (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)))
-                 (or
-                  (and no-mouse-movement
-                       (string-match "mouse-movement" keyname))
-                  (progn (push key keys) nil)
-                  (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
-                                     keyname)
-                       (progn
-                         ;; Discard events (e.g. <help-echo>) which might
-                         ;; spuriously trigger the `sit-for'.
-                         (sleep-for 0.01)
-                         (while (read-event nil nil 0.01))
-                         (not (sit-for
-                               (if (numberp double-click-time)
-                                   (/ double-click-time 1000.0)
-                                 3.0)
-                               t))))))))
-          ;; When we have a sequence of mouse events, discard the most
-          ;; recent ones till we find one with a binding.
-          (let ((keys-1 keys))
-            (while (and keys-1
-                        (not (key-binding (car keys-1))))
-              ;; If we discard the last event, and this was a mouse
-              ;; up, remember this.
-              (if (and (eq keys-1 keys)
-                       (vectorp (car keys-1))
-                       (let* ((last-idx (1- (length (car keys-1))))
-                              (last (aref (car keys-1) last-idx)))
-                         (and (eventp last)
-                              (memq 'click (event-modifiers last)))))
-                  (setq discarded-up t))
-              (setq keys-1 (cdr keys-1)))
-            (if keys-1
-                (setq key (car keys-1))))
-          (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 (not discarded-up) ; Don't attempt to ignore the up-event twice.
-                (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 (setq down-ev (aref key 0)))
-                         (memq 'down (event-modifiers down-ev))
-                         ;; 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 (setq down-ev (aref key 1)))
-                         (memq 'down (event-modifiers down-ev))))
-                (if (and (terminal-parameter nil 'xterm-mouse-mode)
-                         (equal (terminal-parameter nil 'xterm-mouse-last-down)
-                                down-ev))
-                    (aref (read-key-sequence-vector nil) 0)
-                  (read-event)))))
+                   (raw-seq (this-single-command-raw-keys))
+                   (keyn (when (> (length seq) 0)
+                           (aref seq (1- (length seq)))))
+                   (base (event-basic-type keyn))
+                   (modifiers (event-modifiers keyn)))
+              (cond
+               ((zerop (length seq)))   ;FIXME: Can this happen?
+               ((and no-mouse-movement (eq base 'mouse-movement)) nil)
+               ((eq base 'help-echo) nil)
+               (t
+                (setq last-modifiers modifiers)
+                (push (cons seq raw-seq) key-list)))))
+          (nreverse key-list))
       ;; 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 help-downify-mouse-event-type (base)
-  "Add \"down-\" to BASE if it is not already there.
-BASE is a symbol, a mouse event type.  If the modification is done,
-return the new symbol.  Otherwise return nil."
-  (let ((base-s (symbol-name base)))
-    ;; Note: the order of the components in the following string is
-    ;; determined by `apply_modifiers_uncached' in src/keyboard.c.
-    (string-match "\\(A-\\)?\
-\\(C-\\)?\
-\\(H-\\)?\
-\\(M-\\)?\
-\\(S-\\)?\
-\\(s-\\)?\
-\\(double-\\)?\
-\\(triple-\\)?\
-\\(up-\\)?\
-\\(\\(down-\\)?\\)\
-\\(drag-\\)?" base-s)
-    (when (and (null (match-beginning 11)) ; "down-"
-               (null (match-beginning 12))) ; "drag-"
-      (intern (replace-match "down-" t t base-s 10)) )))
-
-(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,
+(defun describe-key (&optional key-list buffer up-event)
+  "Display documentation of the function invoked by KEY-LIST.
+KEY-LIST can be any kind of a key sequence; it can include keyboard events,
 mouse events, and/or menu events.  When calling from a program,
-pass KEY as a string or a vector.
-
-If non-nil, UNTRANSLATED is a vector of the corresponding untranslated events.
-It can also be a number, in which case the untranslated events from
-the last key sequence entered are used.
-UP-EVENT is the up-event that was discarded by reading KEY, or nil.
-
-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)))
-     `(,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))
-      ;; Need to do this before erasing *Help* buffer in case event
-      ;; is a mouse click in an existing *Help* buffer.
-      (when up-event
-	(setq ev-type (event-basic-type up-event))
-	(let ((sequence (vector up-event)))
-	  (when (and (eq ev-type 'mouse-1)
-		     mouse-1-click-follows-link
-		     (not (eq mouse-1-click-follows-link 'double))
-		     (setq mouse-1-remapped
-			   (mouse-on-link-p (event-start up-event))))
-	    (setq mouse-1-tricky (and (integerp mouse-1-click-follows-link)
-				      (> mouse-1-click-follows-link 0)))
-	    (cond ((stringp mouse-1-remapped)
-		   (setq sequence mouse-1-remapped))
-		  ((vectorp mouse-1-remapped)
-		   (setcar up-event (elt mouse-1-remapped 0)))
-		  (t (setcar up-event 'mouse-2))))
-	  (setq defn-up (key-binding sequence nil nil (event-start up-event)))
-          (setq key-locus-up (help--binding-locus sequence (event-start up-event)))
-	  (when mouse-1-tricky
-	    (setq sequence (vector 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))))))
+pass KEY-LIST as a list of elements (SEQ . RAW-SEQ) where SEQ is
+a key-sequence and RAW-SEQ is its untranslated form.
+
+While reading KEY-LIST interactively, this command temporarily enables
+menu items or tool-bar buttons that are disabled to allow getting help
+on them.
+
+BUFFER is the buffer in which to lookup those keys; it defaults to the
+current buffer."
+  (declare (advertised-calling-convention (key-list &optional buffer) "27.1"))
+  (interactive (list (help--read-key-sequence)))
+  (when (arrayp key-list)
+    ;; Compatibility with old calling convention.
+    (setq key-list (cons (list key-list) (if up-event (list up-event))))
+    (when buffer
+      (let ((raw (if (numberp buffer) (this-single-command-raw-keys) buffer)))
+        (setf (cdar (last key-list)) raw)))
+    (setq buffer nil))
+  (let* ((buf (or buffer (current-buffer)))
+         (on-link
+          (mapcar (lambda (kr)
+                    (let ((raw (cdr kr)))
+                      (and (not (memq mouse-1-click-follows-link '(nil double)))
+                           (> (length raw) 0)
+                           (eq (car-safe (aref raw 0)) 'mouse-1)
+                           (with-current-buffer buf
+                             (mouse-on-link-p (event-start (aref raw 0)))))))
+                  key-list))
+         (info-list
+          (help--filter-info-list
+           (with-current-buffer buf
+             (mapcar (lambda (x)
+                       (pcase-let* ((`(,seq . ,raw-seq) x)
+                                    (`(,brief-desc ,defn ,event ,_mouse-msg)
+                                     (help--analyze-key seq raw-seq))
+                                    (locus
+                                     (help--binding-locus
+                                      seq (event-start event))))
+                         `(,seq ,brief-desc ,defn ,locus)))
+                     key-list))
+           2)))
+    (help-setup-xref (list (lambda (key-list buf)
+                             (describe-key key-list
+                                           (if (buffer-live-p buf) buf)))
+                           key-list buf)
+		     (called-interactively-p 'interactive))
+    (if (and (<= (length info-list) 1)
+             (help--binding-undefined-p (nth 2 (car info-list))))
+        (message "%s" (nth 1 (car info-list)))
       (with-help-window (help-buffer)
-        (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 (vectorp key)
-          (let* ((last (1- (length key)))
-                 (elt (aref key last))
-                 (elt-1 (if (listp elt) (copy-sequence elt) elt))
-                 key-1 down-event-type)
-            (when (and (listp elt-1)
-                       (symbolp (car elt-1))
-                       (setq down-event-type (help-downify-mouse-event-type
-                                              (car elt-1))))
-              (setcar elt-1 down-event-type)
-              (setq key-1 (vector elt-1))
-              (when (key-binding key-1)
-                (princ (format "
-
-For documentation of the corresponding mouse down event <%s>,
-click and hold the mouse button longer than %s second(s)."
-                               down-event-type (if (numberp double-click-time)
-                                                   (/ double-click-time 1000.0)
-                                                 3)))))))
-	(when up-event
-	  (unless (or (null defn-up)
-		      (integerp defn-up)
-		      (equal defn-up 'undefined))
-	    (princ (format "
-
------------------ up-event %s----------------
-
-%s%s%s runs the command %S%s, which is "
-			   (if mouse-1-tricky "(short click) " "")
-			   (key-description (vector up-event))
-			   mouse-msg
-			   (if mouse-1-remapped
-                               " is remapped to <mouse-2>, which" "")
-			   defn-up (if key-locus-up
-                                       (format " (found in %s)" key-locus-up)
-                                     "")))
-	    (describe-function-1 defn-up))
-	  (unless (or (null defn-up-tricky)
-		      (integerp defn-up-tricky)
-		      (eq defn-up-tricky 'undefined))
-	    (princ (format "
-
------------------ up-event (long click) ----------------
-
-Pressing <%S>%s for longer than %d milli-seconds
-runs the command %S%s, which is "
-			   ev-type mouse-msg
-			   mouse-1-click-follows-link
-			   defn-up-tricky (if key-locus-up-tricky
-                                              (format " (found in %s)" key-locus-up-tricky)
-                                            "")))
-	    (describe-function-1 defn-up-tricky)))))))
+        (when (> (length info-list) 1)
+          ;; FIXME: Make this into clickable hyperlinks.
+          (princ "There were several key-sequences:\n\n")
+          (princ (mapconcat (lambda (info)
+                              (pcase-let ((`(,_seq ,brief-desc ,_defn ,_locus)
+                                           info))
+                                (concat "  " brief-desc)))
+                            info-list
+                            "\n"))
+          (when (delq nil on-link)
+            (princ "\n\nThose are influenced by `mouse-1-click-follows-link'"))
+          (princ "\n\nThey're all described below."))
+        (pcase-dolist (`(,_seq ,brief-desc ,defn ,locus)
+                       info-list)
+          (when defn
+            (when (> (length info-list) 1)
+              (with-current-buffer standard-output
+                (insert "\n\n"
+                        (eval-when-compile
+                          (propertize "\n" 'face '(:height 0.1 :inverse-video t)))
+                        "\n")))
+
+            (princ brief-desc)
+            (when locus
+              (princ (format " (found in %s)" locus)))
+            (princ ", which is ")
+	    (describe-function-1 defn)))))))
 
 (defun describe-mode (&optional buffer)
   "Display documentation of current major mode and minor modes.
@@ -1120,7 +1058,7 @@ lookup-minor-mode-from-indicator
 
 ;;; Automatic resizing of temporary buffers.
 (defcustom temp-buffer-max-height
-  (lambda (buffer)
+  (lambda (_buffer)
     (if (and (display-graphic-p) (eq (selected-window) (frame-root-window)))
 	(/ (x-display-pixel-height) (frame-char-height) 2)
       (/ (- (frame-height) 2) 2)))
@@ -1137,7 +1075,7 @@ temp-buffer-max-height
   :version "24.3")
 
 (defcustom temp-buffer-max-width
-  (lambda (buffer)
+  (lambda (_buffer)
     (if (and (display-graphic-p) (eq (selected-window) (frame-root-window)))
 	(/ (x-display-pixel-width) (frame-char-width) 2)
       (/ (- (frame-width) 2) 2)))

Message #139 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Tue, 30 Jan 2018 05:23:57 +0200

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
> Date: Mon, 29 Jan 2018 16:54:09 -0500
> 
> > It means I'm okay with this:
> >
> >   FOO is undefined
> >
> > but prefer not to see the second line in this:
> >
> >   FOO at that spot runs BAR
> >   down-FOO at that spot is undefined
> 
> OK, in help--filter-info-list in the patch below I follow the rule of
> getting rid of all `undefined` keys except when there are only undefined
> keys in which case I keep the last one (e.g. the triple-mouse-N).

Thanks.

Message #142 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Tue, 30 Jan 2018 09:18:04 -0500

>> OK, in help--filter-info-list in the patch below I follow the rule of
>> getting rid of all `undefined` keys except when there are only undefined
>> keys in which case I keep the last one (e.g. the triple-mouse-N).
> Thanks.

Should I push it to master?


        Stefan

Message #145 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Tue, 30 Jan 2018 17:40:10 +0200

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
> Date: Tue, 30 Jan 2018 09:18:04 -0500
> 
> >> OK, in help--filter-info-list in the patch below I follow the rule of
> >> getting rid of all `undefined` keys except when there are only undefined
> >> keys in which case I keep the last one (e.g. the triple-mouse-N).
> > Thanks.
> 
> Should I push it to master?

I think so, yes.

Message #148 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acm <at> muc.de, 29478 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: [Patch] bug#29478: 26.0.90;
 `C-h k' followed by mouse clicks no longer shows down event
Date: Tue, 30 Jan 2018 11:59:18 -0500

>> Should I push it to master?
> I think so, yes.

Thanks, done,


        Stefan

Message #151 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29478 <at> debbugs.gnu.org
Subject: Re: bug#29478: 26.0.90; `C-h k' followed by mouse clicks no longer
 shows down event
Date: Wed, 26 Aug 2020 15:43:31 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> The down mouse-button event is no longer listed along with the up
> event when you use `C-h k' with a click event.  Dunno whether this was
> by design or is an oversight.

I tried this now in Emacs 28, and it says:

---

There were several key-sequences:

  <down-mouse-1> at that spot runs the command ignore
  <mouse-1> at that spot runs the command view-echo-area-messages

They're all described below.

---

I just skimmed the rest of this long thread, but at least one patch was
applied, and since this seems to work for me now, I'm going to go ahead
and guess that the bug described here was fixed, and I'm closing this
bug report.  If there's more to be done here, please reopen, or file a
new bug report about any remaining issues.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #156 received at 29478 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 29478 <at> debbugs.gnu.org
Subject: RE: bug#29478: 26.0.90; `C-h k' followed by mouse clicks no longer
 shows down event
Date: Wed, 26 Aug 2020 10:12:20 -0700 (PDT)

> I tried this now in Emacs 28, and it says:
> 
> ---
> 
> There were several key-sequences:
> 
>   <down-mouse-1> at that spot runs the command ignore
>   <mouse-1> at that spot runs the command view-echo-area-messages
> 
> They're all described below.
> 
> ---
> 
> I just skimmed the rest of this long thread, but at least one patch was
> applied, and since this seems to work for me now, I'm going to go ahead
> and guess that the bug described here was fixed, and I'm closing this
> bug report.  If there's more to be done here, please reopen, or file a
> new bug report about any remaining issues.

Me too.  I just reread the thread, which is long and is mostly long inline patches (ugh).

I'm just assuming at this point that those working on it made a useful improvement.

In terms of what I read, I think the main point is for `C-h k' to let you know (1) that there are multiple events (when there are), (2) show you the info for each of those events, and (3) present them in order of their occurrence.

I think Stefan argued for each of (1), (2), and (3), as did I.

I also think that, if it presents some difficulty, in terms of providing help, to deal with both `C-h c' and `C-h k' in this context, then my concern was with `C-h k'.  In principle, we should be able to provide good help for each of those help keys.  If we can't do that for both using the same or similar code, then we should find some way to handle them separately.  What I really care about here is `C-h k' help.

If I ever get around to using Emacs 28 (I can't use Emacs 27.1 at all - breaks everything I have), and if at that point I have something to say about the new `C-h k' doc for this, then I'll send another bug report.

Thanks for everyone's attention to this bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.