Making TTY menus more visual

Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

[-- Attachment #1.1: Type: text/plain, Size: 733 bytes --]

Hi lovely developers, I'm a long time Emacs user, first time Emacs patch
submitter. :) 

Right now in Emacs on a TTY, clicking on the menu bar with the mouse
pops up menu navigation through tmm.el.  I think it would make more
sense for mouse clicks to pop up visual menus via menu-bar-open-mouse by
default. 

But I get why that's not the current default. These visual menus
currently behave very poorly with a mouse on a Linux TTY: Clicking
<mouse-1> anywhere selects the currently selected item, not the item you
clicked on. I have a prototype fix for this that I've been working on.
If you think it makes sense, I can finish it up. I've attached the
changes in case you want to take a look. 

Let me know your thoughts. 

  -- MJF

[-- Attachment #1.2: Type: text/html, Size: 965 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adding-mouse-controls-to-menu-bar.el.patch --]
[-- Type: text/x-diff; name=0001-Adding-mouse-controls-to-menu-bar.el.patch, Size: 4392 bytes --]

From 9972ca4995e2e43cfaff9b550f2b73c72ae76bb6 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 19 Sep 2020 00:43:29 -0700
Subject: [PATCH 1/2] Adding mouse controls to menu-bar.el.

---
 lisp/menu-bar.el | 12 +++++++++
 lisp/tmm.el      | 64 ++++++++++++++++++++++++++++--------------------
 2 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index d3e434aec9..9021be8eff 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2660,6 +2660,18 @@ menu-bar-open
 
 (global-set-key [f10] 'menu-bar-open)
 
+(defun menu-bar-open-mouse (event)
+  "Mosue-triggered version of `menu-bar-open'.
+This command is to be used when you click the mouse in the menubar."
+  (interactive "e")
+  (require 'tmm)       ; Possibly have tmm depend on menu-bar instead?
+  (let* ((x-position (car (posn-x-y (event-start event))))
+         (menu-bar-item-cons (tmm-menubar-item-at-x x-position)))
+    (menu-bar-open nil
+                   (if menu-bar-item-cons
+                       (cdr menu-bar-item-cons)
+                     0))))
+
 (defun buffer-menu-open ()
   "Start key navigation of the buffer menu.
 This is the keyboard interface to \\[mouse-buffer-menu]."
diff --git a/lisp/tmm.el b/lisp/tmm.el
index 1e18c8b4ae..710295c776 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -59,6 +59,40 @@ tmm-menubar-keymap
      (tmm-get-keybind [menu-bar]))
     `(keymap ,@(nreverse menu-bar) ,@menu-end)))
 
+(defun tmm-menubar-item-at-x (x-position)
+  "Return a cons of the form (KEY . X) for the item clicked on.
+
+If nothing is clicked on, returns nil."
+  (let ((column 0)
+        (menu-bar (tmm-menubar-keymap))
+        prev-key
+        prev-column
+        found)
+    (catch 'done
+      (map-keymap
+       (lambda (key binding)
+         (when (> column x-position)
+           (setq found t)
+           (throw 'done nil))
+         (setq prev-key key)
+         (pcase binding
+           ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
+                `(menu-item ,name ,_cmd            ;Extended menu item.
+                            . ,(and props
+                                    (guard (let ((visible
+                                                  (plist-get props :visible)))
+                                             (or (null visible)
+                                                 (eval visible)))))))
+            (setq prev-column column
+                  column (+ column (length name) 1)))))
+       menu-bar)
+      ;; Check the last menu item.
+      (when (> column x-position)
+        (setq found t)))
+    (if found
+        (cons prev-key prev-column)
+      nil)))
+
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
 ;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
 
@@ -74,33 +108,11 @@ tmm-menubar
 `tty-menu-open-use-tmm' to a non-nil value."
   (interactive)
   (run-hooks 'menu-bar-update-hook)
-  ;; Obey menu-bar-final-items; put those items last.
   (let ((menu-bar (tmm-menubar-keymap))
-	menu-bar-item)
-    (if x-position
-	(let ((column 0)
-              prev-key)
-          (catch 'done
-            (map-keymap
-             (lambda (key binding)
-               (when (> column x-position)
-                 (setq menu-bar-item prev-key)
-                 (throw 'done nil))
-               (setq prev-key key)
-               (pcase binding
-                 ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
-                      `(menu-item ,name ,_cmd            ;Extended menu item.
-                        . ,(and props
-                                (guard (let ((visible
-                                              (plist-get props :visible)))
-                                         (or (null visible)
-                                             (eval visible)))))))
-                  (setq column (+ column (length name) 1)))))
-             menu-bar)
-            ;; Check the last menu item.
-            (when (> column x-position)
-              (setq menu-bar-item prev-key)))))
-    (tmm-prompt menu-bar nil menu-bar-item)))
+        (menu-bar-item-cons (tmm-menubar-item-at-x x-position)))
+    (tmm-prompt menu-bar
+                nil
+                (and menu-bar-item-cons (car menu-bar-item-cons)))))
 
 ;;;###autoload
 (defun tmm-menubar-mouse (event)
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-WIP-Adding-mouse-navigation-of-tty-menus.patch --]
[-- Type: text/x-diff; name=0002-WIP-Adding-mouse-navigation-of-tty-menus.patch, Size: 2408 bytes --]

From 756c297535ed10534973e84387419ea7d728938e Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Thu, 1 Oct 2020 00:48:19 -0700
Subject: [PATCH 2/2] WIP: Adding mouse navigation of tty menus.

---
 src/term.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/term.c b/src/term.c
index 3677644845..8167763e84 100644
--- a/src/term.c
+++ b/src/term.c
@@ -3101,7 +3101,27 @@ read_menu_input (struct frame *sf, int *x, int *y, int min_y, int max_y,
 	    st = MI_SCROLL_BACK;
 	}
       else if (EQ (cmd, Qtty_menu_select))
-	st = MI_ITEM_SELECTED;
+        {
+          // Remaining to do:
+          // * Add new symbol "tty-menu-mouse-select-or-dismiss", bind to mouse buttons.
+          // * If click is in visible menu act as if tty-menu-select, otherwise act as if tty-menu-exit.
+          // * Ensure logic works with any menu popup position.
+          Lisp_Object posn_x_y = CALLN (Ffuncall, intern ("posn-x-y"),
+                                        CALLN (Ffuncall, intern ("event-start"), last_nonmenu_event));
+          int mouse_x = XFIXNUM (Fcar (posn_x_y));
+          int mouse_y = XFIXNUM (Fcdr (posn_x_y));
+          message ("In read_menu_input: tty_menu_select w/ mouse_x=%d, mouse_y=%d, min_y=%d, max_y=%d",
+                   mouse_x, mouse_y, min_y, max_y);
+
+          // Unclear offset -- need to figure this out. Perhaps due to menu-bar?
+          mouse_y++;
+          if (mouse_y >= min_y && mouse_y <= max_y)
+            {
+              *y = mouse_y;
+            }
+
+          st = MI_ITEM_SELECTED;
+        }
       else if (!EQ (cmd, Qtty_menu_ignore))
 	usable_input = 0;
       if (usable_input)
@@ -3224,6 +3244,9 @@ tty_menu_activate (tty_menu *menu, int *pane, int *selidx,
       int max_y = min (min_y + state[0].menu->count, FRAME_TOTAL_LINES (sf) - 1) - 1;
 
       input_status = read_menu_input (sf, &x, &y, min_y, max_y, &first_time);
+
+      message("In tty_menu_activate: read_menu_input --> input_status=%d, x=%d, y=%d, first_time=%d",
+              input_status, x, y, first_time);
       if (input_status)
 	{
 	  leave = 1;
@@ -3369,6 +3392,7 @@ tty_menu_activate (tty_menu *menu, int *pane, int *selidx,
     clear_input_pending ();
   SAFE_FREE ();
   Vinhibit_redisplay = prev_inhibit_redisplay;
+  message ("Returning result=%d", result);
   return result;
 }
 
-- 
2.20.1

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Thu, 01 Oct 2020 23:16:49 -0700
> From: Jared Finder via "Emacs development discussions." <emacs-devel@gnu.org>
> 
> Right now in Emacs on a TTY, clicking on the menu bar with the mouse pops up menu navigation through
> tmm.el.  I think it would make more sense for mouse clicks to pop up visual menus via
> menu-bar-open-mouse by default.

Which mouse support do you have on that TTY?  Is it GPM or is it
something else?

If Emacs opens tmm-menubar when you click on the menu bar, it means
Emacs doesn't understand that a mouse click on the menu bar was
received.  Can you tell why this happens in your case?

> But I get why that's not the current default. These visual menus currently behave very poorly with a mouse
> on a Linux TTY: Clicking <mouse-1> anywhere selects the currently selected item, not the item you clicked
> on.

What do you mean by "currently selected item"?

> I have a prototype fix for this that I've been working on. If you think it makes sense, I can finish it up. I've
> attached the changes in case you want to take a look.

I don't understand why you need all this.  TTY menus work perfectly on
MS-Windows console: the place where you click on the menu bar
determines the menu that is dropped down.  For example, if you click
on Options, you get the Options menu.

It is supposed to work the same on Linux terminals with a mouse: when
you click the mouse on the menu bar, the function x_popup_menu_1 is
supposed to be called with the argument POSITION that specifies the
coordinates of the click, and then tty_menu_show, called by
x_popup_menu_1, uses those coordinates to decide which menu to drop
down.  Which of these doesn't work for you, and why?

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

On 2020-10-02 12:31 am, Eli Zaretskii wrote:
>> Date: Thu, 01 Oct 2020 23:16:49 -0700
>> From: Jared Finder via "Emacs development discussions." 
>> <emacs-devel@gnu.org>
>> 
>> Right now in Emacs on a TTY, clicking on the menu bar with the mouse 
>> pops up menu navigation through
>> tmm.el.  I think it would make more sense for mouse clicks to pop up 
>> visual menus via
>> menu-bar-open-mouse by default.
> 
> Which mouse support do you have on that TTY?  Is it GPM or is it
> something else?
> 
> If Emacs opens tmm-menubar when you click on the menu bar, it means
> Emacs doesn't understand that a mouse click on the menu bar was
> received.  Can you tell why this happens in your case?

This is for xterm-mouse support. From what I can tell, <menu-bar> 
<mouse-1> is always bound to tmm-menubar. The following holds on Mac, 
Windows, and Linux, and appears to be independent of if you start with 
or without --no-window-system:

ELISP> (lookup-key global-map (kbd "<menu-bar> <mouse-1>"))
tmm-menubar-mouse

>> But I get why that's not the current default. These visual menus 
>> currently behave very poorly with a mouse
>> on a Linux TTY: Clicking <mouse-1> anywhere selects the currently 
>> selected item, not the item you clicked
>> on.
> 
> What do you mean by "currently selected item"?

Perhaps "currently focused menu item" is the correct term? I'm referring 
to the highlighted menu item that you can change with arrow keys.

>> I have a prototype fix for this that I've been working on. If you 
>> think it makes sense, I can finish it up. I've
>> attached the changes in case you want to take a look.
> 
> I don't understand why you need all this.  TTY menus work perfectly on
> MS-Windows console: the place where you click on the menu bar
> determines the menu that is dropped down.  For example, if you click
> on Options, you get the Options menu.
> 
> It is supposed to work the same on Linux terminals with a mouse: when
> you click the mouse on the menu bar, the function x_popup_menu_1 is
> supposed to be called with the argument POSITION that specifies the
> coordinates of the click, and then tty_menu_show, called by
> x_popup_menu_1, uses those coordinates to decide which menu to drop
> down.  Which of these doesn't work for you, and why?

This addition is for TTY menus triggered via xterm-mouse-mode. If this 
makes sense to add as a feature, I can finish up my patches.

I see the following things that need to change:

1. Add a new command that calls menu-bar-open with the right value for 
initial-x. (this is the patch 001 in the root of the thread)

2. In read_menu_input in term.c, additional logic needs to be added to 
handle xterm mouse clicks since the focused menu item will not be 
necessarily the one clicked on. Also, a user may click outside of the 
popped up menu, which they would expect to dismiss the menu. (this is 
patch 002 in the root of the thread, not yet complete)

3. The new command should replace the current binding of <menu-bar> 
<mouse-1>. (no patch yet, I wanted to see if you had any backward compat 
concens)

   -- MJF

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Fri, 02 Oct 2020 17:16:55 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> This is for xterm-mouse support. From what I can tell, <menu-bar> 
> <mouse-1> is always bound to tmm-menubar. The following holds on Mac, 
> Windows, and Linux, and appears to be independent of if you start with 
> or without --no-window-system:
> 
> ELISP> (lookup-key global-map (kbd "<menu-bar> <mouse-1>"))
> tmm-menubar-mouse

That is true, but when mouse clicks on the menu bar are supported,
this binding is never used.  However, maybe this is not so for
xterm-mouse.

> This addition is for TTY menus triggered via xterm-mouse-mode. If this 
> makes sense to add as a feature, I can finish up my patches.

Yes, having the TTY drop-down menus supported with xterm-mouse is
something we would welcome very much.

> I see the following things that need to change:
> 
> 1. Add a new command that calls menu-bar-open with the right value for 
> initial-x. (this is the patch 001 in the root of the thread)

But patch 001 also includes unrelated parts, the tmm-menubar-item-at-x
function etc., right?

> 2. In read_menu_input in term.c, additional logic needs to be added to 
> handle xterm mouse clicks since the focused menu item will not be 
> necessarily the one clicked on.

I'm not sure I understand how this can happen.  Can you describe the
sequence of events that could lead to this, assuming tmm-menubar is
NOT involved?  When TTY menus drop down from the menu bar, the focused
item and the one clicked on should always coincide.  Any click on
another item is a click "outside of the popped up menu", and that
should dismiss the menu.

> Also, a user may click outside of the popped up menu, which they
> would expect to dismiss the menu. (this is patch 002 in the root of
> the thread, not yet complete)

This should already work; it does in the MS-Windows build when Emacs
is invoked with -nw.  Please tell more why you think any changes there
are needed.  Perhaps you could take me through the code there and
explain what is missing and why.  (And why do you call posn-x-y in the
patch when X and Y are already known and used by that code? is that
because mouse_get_xy does not yet support xterm-mouse? if so, that
support should be added via the terminal's mouse_position_hook.)

> 3. The new command should replace the current binding of <menu-bar> 
> <mouse-1>.

Yes, eventually this should be done as part of turning on
xterm-mouse-mode.

Thanks.

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

On 2020-10-03 1:50 am, Eli Zaretskii wrote:
>> Date: Fri, 02 Oct 2020 17:16:55 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> I see the following things that need to change:
>> 
>> 1. Add a new command that calls menu-bar-open with the right value for
>> initial-x. (this is the patch 001 in the root of the thread)
> 
> But patch 001 also includes unrelated parts, the tmm-menubar-item-at-x
> function etc., right?

tmm-menubar-item-at-x is a new function to share the conversion of a 
mouse X to a specific menu item between tmm-menubar and the newly added 
command, menu-bar-open-mouse.

There's no new code there, it's the existing logic in tmm-menubar.

>> 2. In read_menu_input in term.c, additional logic needs to be added to
>> handle xterm mouse clicks since the focused menu item will not be
>> necessarily the one clicked on.
> 
> I'm not sure I understand how this can happen.  Can you describe the
> sequence of events that could lead to this, assuming tmm-menubar is
> NOT involved?  When TTY menus drop down from the menu bar, the focused
> item and the one clicked on should always coincide.  Any click on
> another item is a click "outside of the popped up menu", and that
> should dismiss the menu.
> 
>> Also, a user may click outside of the popped up menu, which they
>> would expect to dismiss the menu. (this is patch 002 in the root of
>> the thread, not yet complete)
> 
> This should already work; it does in the MS-Windows build when Emacs
> is invoked with -nw.  Please tell more why you think any changes there
> are needed.  Perhaps you could take me through the code there and
> explain what is missing and why.  (And why do you call posn-x-y in the
> patch when X and Y are already known and used by that code? is that
> because mouse_get_xy does not yet support xterm-mouse? if so, that
> support should be added via the terminal's mouse_position_hook.)

 From injecting debug logs into read_menu_input, I can observe that 
tty-menu-mouse-movement is never received so the highlighted item never 
changes except due to keyboard input. And from tracing 
xterm-mouse--read-event-sequence, it appears that Emacs normally does 
not receive xterm mouse motion events unless a button is pressed.

This appears to be due to xt-mouse sending event code 1002 instead of 
1003 (see 
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking).

Just sending 1003 instead doesn't just work, but I do see mouse events 
now coming through. Let me do some more investigation here and I will 
get back to you.

   -- MJF

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

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

On 2020-10-03 12:26 pm, Jared Finder wrote:
> On 2020-10-03 1:50 am, Eli Zaretskii wrote:
>>> Date: Fri, 02 Oct 2020 17:16:55 -0700
>>> From: Jared Finder <jared@finder.org>
>>> Cc: emacs-devel@gnu.org
>>> 
>>> Also, a user may click outside of the popped up menu, which they
>>> would expect to dismiss the menu. (this is patch 002 in the root of
>>> the thread, not yet complete)
>> 
>> This should already work; it does in the MS-Windows build when Emacs
>> is invoked with -nw.  Please tell more why you think any changes there
>> are needed.  Perhaps you could take me through the code there and
>> explain what is missing and why.  (And why do you call posn-x-y in the
>> patch when X and Y are already known and used by that code? is that
>> because mouse_get_xy does not yet support xterm-mouse? if so, that
>> support should be added via the terminal's mouse_position_hook.)
> 
> From injecting debug logs into read_menu_input, I can observe that
> tty-menu-mouse-movement is never received so the highlighted item
> never changes except due to keyboard input. And from tracing
> xterm-mouse--read-event-sequence, it appears that Emacs normally does
> not receive xterm mouse motion events unless a button is pressed.
> 
> This appears to be due to xt-mouse sending event code 1002 instead of
> 1003 (see
> https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking).
> 
> Just sending 1003 instead doesn't just work, but I do see mouse events
> now coming through. Let me do some more investigation here and I will
> get back to you.

It wasn't that much more work to get xterm-mouse to work. I've attached 
an updated patch.

I have just one question, corresponding to the remaining TODO:

Now there are newly emitted events for mouse-movement that are not 
handled such as "<mode-line> <mouse-movement>" or "<vertical-line> 
<mouse-movement>". It'd be easy enough to bind all of these to ignore 
and further update tty-menu-navigation-map to have more cases, but is 
that the right solution? I'm surprised that only xterm-mouse would run 
into this case.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch --]
[-- Type: text/x-diff; name=0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch, Size: 5208 bytes --]

From 2b71048614be65eb9c4267224a0606bfcd86cc8a Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 3 Oct 2020 14:46:30 -0700
Subject: [PATCH 2/2] Making TTY menus work with xterm-mouse-mode.

* Update xt-mouse.el to use SET_ANY_EVENT_MOUSE (1003) so mouse movement is
  always reported.
* Hook up mouse_get_xy to mouse-position so it works with xterm mouse.
* (WIP) Properly handle mouse-movement events in all the possible
  prefixes, such as menu-bar or mode-line.
---
 lisp/menu-bar.el | 10 ++++++++++
 lisp/xt-mouse.el | 10 +++++-----
 src/term.c       | 28 ++++++++++++++++++----------
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 9021be8eff..44de6e7c32 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2758,6 +2758,16 @@ tty-menu-navigation-map
     (define-key map [C-down-mouse-2] 'tty-menu-ignore)
     (define-key map [C-down-mouse-3] 'tty-menu-ignore)
     (define-key map [mouse-movement] 'tty-menu-mouse-movement)
+
+    ;; These are needed because xterm-mouse always sends events with
+    ;; special prefixes.
+    ;;
+    ;; TODO -- either make xterm-mouse not use the special prefixes or
+    ;; fix all "prefix-thingy mouse-movement is undefined" with
+    ;; special prefixes.
+    (define-key map [header-line mouse-movement] 'tty-menu-mouse-movement)
+    (define-key map [menu-bar mouse-movement] 'tty-menu-mouse-movement)
+    (define-key map [mode-line mouse-movement] 'tty-menu-mouse-movement)
     map)
   "Keymap used while processing TTY menus.")
 
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 362d29b943..ae6f85f1f7 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -339,7 +339,7 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
             motion events during dragging operations.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an
@@ -360,7 +360,7 @@ xterm-mouse-tracking-enable-sequence
   (apply #'concat (xterm-mouse--tracking-sequence ?h)))
 
 (defconst xterm-mouse-tracking-enable-sequence
-  "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"
+  "\e[?1000h\e[?1003h\e[?1005h\e[?1006h"
   "Control sequence to enable xterm mouse tracking.
 Enables basic mouse tracking, mouse motion events and finally
 extended tracking on terminals that support it. The following
@@ -371,7 +371,7 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
             motion events during dragging operations.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an extension
@@ -400,7 +400,7 @@ xterm-mouse-tracking-disable-sequence
   (apply #'concat (nreverse (xterm-mouse--tracking-sequence ?l))))
 
 (defconst xterm-mouse-tracking-disable-sequence
-  "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"
+  "\e[?1006l\e[?1005l\e[?1003l\e[?1000l"
   "Reset the modes set by `xterm-mouse-tracking-enable-sequence'.")
 
 (make-obsolete-variable
@@ -414,7 +414,7 @@ xterm-mouse--tracking-sequence
 enable, ?l to disable)."
   (mapcar
    (lambda (code) (format "\e[?%d%c" code suffix))
-   `(1000 1002 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
+   `(1000 1003 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
 
 (defun turn-on-xterm-mouse-tracking-on-terminal (&optional terminal)
   "Enable xterm mouse tracking on TERMINAL."
diff --git a/src/term.c b/src/term.c
index 3677644845..1196609047 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2481,6 +2481,16 @@ term_mouse_position (struct frame **fp, int insist, Lisp_Object *bar_window,
   *bar_window = Qnil;
   *part = scroll_bar_above_handle;
 
+  // xterm-mouse always stores the mouse positions in terminal
+  // parameters. Existing code path (GPM?) does not.
+  //
+  // TODO: Figure out the right way to unify these code paths.
+  if (true)
+    {
+      last_mouse_x = XFIXNUM (Fterminal_parameter (Qnil, intern ("xterm-mouse-x")));
+      last_mouse_y = XFIXNUM (Fterminal_parameter (Qnil, intern ("xterm-mouse-y")));
+    }
+
   XSETINT (*x, last_mouse_x);
   XSETINT (*y, last_mouse_y);
   *timeptr = current_Time ();
@@ -2804,16 +2814,14 @@ tty_menu_calc_size (tty_menu *menu, int *width, int *height)
 static void
 mouse_get_xy (int *x, int *y)
 {
-  struct frame *sf = SELECTED_FRAME ();
-  Lisp_Object lmx = Qnil, lmy = Qnil, lisp_dummy;
-  enum scroll_bar_part part_dummy;
-  Time time_dummy;
-
-  if (FRAME_TERMINAL (sf)->mouse_position_hook)
-    (*FRAME_TERMINAL (sf)->mouse_position_hook) (&sf, -1,
-                                                 &lisp_dummy, &part_dummy,
-						 &lmx, &lmy,
-						 &time_dummy);
+  Lisp_Object lmx = Qnil, lmy = Qnil, mouse_position = Fmouse_position ();
+
+  if (EQ (selected_frame, XCAR(mouse_position)))
+    {
+      lmx = XCAR (XCDR (mouse_position));
+      lmy = XCDR (XCDR (mouse_position));
+    }
+
   if (!NILP (lmx))
     {
       *x = XFIXNUM (lmx);
-- 
2.20.1

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

On 2020-10-03 3:28 pm, Jared Finder wrote:
> On 2020-10-03 12:26 pm, Jared Finder wrote:
>> On 2020-10-03 1:50 am, Eli Zaretskii wrote:
>>>> Date: Fri, 02 Oct 2020 17:16:55 -0700
>>>> From: Jared Finder <jared@finder.org>
>>>> Cc: emacs-devel@gnu.org
>>>> 
>>>> Also, a user may click outside of the popped up menu, which they
>>>> would expect to dismiss the menu. (this is patch 002 in the root of
>>>> the thread, not yet complete)
>>> 
>>> This should already work; it does in the MS-Windows build when Emacs
>>> is invoked with -nw.  Please tell more why you think any changes 
>>> there
>>> are needed.  Perhaps you could take me through the code there and
>>> explain what is missing and why.  (And why do you call posn-x-y in 
>>> the
>>> patch when X and Y are already known and used by that code? is that
>>> because mouse_get_xy does not yet support xterm-mouse? if so, that
>>> support should be added via the terminal's mouse_position_hook.)
>> 
>> From injecting debug logs into read_menu_input, I can observe that
>> tty-menu-mouse-movement is never received so the highlighted item
>> never changes except due to keyboard input. And from tracing
>> xterm-mouse--read-event-sequence, it appears that Emacs normally does
>> not receive xterm mouse motion events unless a button is pressed.
>> 
>> This appears to be due to xt-mouse sending event code 1002 instead of
>> 1003 (see
>> https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking).
>> 
>> Just sending 1003 instead doesn't just work, but I do see mouse events
>> now coming through. Let me do some more investigation here and I will
>> get back to you.
> 
> It wasn't that much more work to get xterm-mouse to work. I've
> attached an updated patch.
> 
> I have just one question, corresponding to the remaining TODO:
> 
> Now there are newly emitted events for mouse-movement that are not
> handled such as "<mode-line> <mouse-movement>" or "<vertical-line>
> <mouse-movement>". It'd be easy enough to bind all of these to ignore
> and further update tty-menu-navigation-map to have more cases, but is
> that the right solution? I'm surprised that only xterm-mouse would run
> into this case.

Apologies, I mistakenly included some exploratory code in my previous 
patch. The changes in term_mouse_position are unneeded.

   -- MJF

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sat, 03 Oct 2020 12:26:49 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> >> 1. Add a new command that calls menu-bar-open with the right value for
> >> initial-x. (this is the patch 001 in the root of the thread)
> > 
> > But patch 001 also includes unrelated parts, the tmm-menubar-item-at-x
> > function etc., right?
> 
> tmm-menubar-item-at-x is a new function to share the conversion of a 
> mouse X to a specific menu item between tmm-menubar and the newly added 
> command, menu-bar-open-mouse.
> 
> There's no new code there, it's the existing logic in tmm-menubar.

OK, but then this function should be in menu-bar.el, and its name
shouldn't begin with "tmm-".  tmm-menubar.el will then use it.

> > This should already work; it does in the MS-Windows build when Emacs
> > is invoked with -nw.  Please tell more why you think any changes there
> > are needed.  Perhaps you could take me through the code there and
> > explain what is missing and why.  (And why do you call posn-x-y in the
> > patch when X and Y are already known and used by that code? is that
> > because mouse_get_xy does not yet support xterm-mouse? if so, that
> > support should be added via the terminal's mouse_position_hook.)
> 
>  From injecting debug logs into read_menu_input, I can observe that 
> tty-menu-mouse-movement is never received so the highlighted item never 
> changes except due to keyboard input. And from tracing 
> xterm-mouse--read-event-sequence, it appears that Emacs normally does 
> not receive xterm mouse motion events unless a button is pressed.
> 
> This appears to be due to xt-mouse sending event code 1002 instead of 
> 1003 (see 
> https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking).
> 
> Just sending 1003 instead doesn't just work, but I do see mouse events 
> now coming through. Let me do some more investigation here and I will 
> get back to you.

Thanks.

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sat, 03 Oct 2020 12:26:49 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
>  From injecting debug logs into read_menu_input, I can observe that 
> tty-menu-mouse-movement is never received so the highlighted item never 
> changes except due to keyboard input. And from tracing 
> xterm-mouse--read-event-sequence, it appears that Emacs normally does 
> not receive xterm mouse motion events unless a button is pressed.
> 
> This appears to be due to xt-mouse sending event code 1002 instead of 
> 1003 (see 
> https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking).

Hmm... maybe I'm misunderstanding what that page says, but it seems
to say 1002 is for button click event and 1003 is for any mouse event:

     #define SET_BTN_EVENT_MOUSE         1002
     #define SET_ANY_EVENT_MOUSE         1003

What am I missing?

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sat, 03 Oct 2020 15:28:23 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> It wasn't that much more work to get xterm-mouse to work. I've attached 
> an updated patch.

See some comments below.

> I have just one question, corresponding to the remaining TODO:
> 
> Now there are newly emitted events for mouse-movement that are not 
> handled such as "<mode-line> <mouse-movement>" or "<vertical-line> 
> <mouse-movement>". It'd be easy enough to bind all of these to ignore 
> and further update tty-menu-navigation-map to have more cases, but is 
> that the right solution? I'm surprised that only xterm-mouse would run 
> into this case.

Emacs normally doesn't receive mouse-movement events unless we
explicitly enable that, see the macro track-mouse.  And the
menu-processing code for other TTYs doesn't enable mouse-movement
events because it isn't needed: with other mice the mouse coordinates
can be read even if the button wasn't clicked.  Isn't there a similar
feature of xterm, whereby we can send some command to the mouse driver
to respond with the current mouse coordinates?  If so, it would be a
better match for what other mice do.  But if such a command doesn't
exist for xterm, we will need to enable mouse-movement events for menu
processing by xterm-mouse.  If that requires binding more key
combinations to 'ignore', I see no problem with that, as other mouse
interfaces don't care.

> diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
> index 362d29b943..ae6f85f1f7 100644
> --- a/lisp/xt-mouse.el
> +++ b/lisp/xt-mouse.el
> @@ -339,7 +339,7 @@ xterm-mouse-tracking-enable-sequence
>              position (<= 223), which can be reported in this
>              basic mode.
>  
> -\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
> +\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
>              motion events during dragging operations.

I think we should only switch to 0x1003 when the track-mouse variable
is non-nil, see the macro track-mouse and its internal part
internal--track-mouse.

> @@ -2804,16 +2814,14 @@ tty_menu_calc_size (tty_menu *menu, int *width, int *height)
>  static void
>  mouse_get_xy (int *x, int *y)
>  {
> -  struct frame *sf = SELECTED_FRAME ();
> -  Lisp_Object lmx = Qnil, lmy = Qnil, lisp_dummy;
> -  enum scroll_bar_part part_dummy;
> -  Time time_dummy;
> -
> -  if (FRAME_TERMINAL (sf)->mouse_position_hook)
> -    (*FRAME_TERMINAL (sf)->mouse_position_hook) (&sf, -1,
> -                                                 &lisp_dummy, &part_dummy,
> -						 &lmx, &lmy,
> -						 &time_dummy);
> +  Lisp_Object lmx = Qnil, lmy = Qnil, mouse_position = Fmouse_position ();
> +
> +  if (EQ (selected_frame, XCAR(mouse_position)))
> +    {
> +      lmx = XCAR (XCDR (mouse_position));
> +      lmy = XCDR (XCDR (mouse_position));
> +    }
> +

This is okay as a general idea, but it would be more clean to make
Fmouse_position call a new C function (extracted from the first part
of Fmouse_position's code) that just computes the mouse coordinates.
Then Fmouse_position's would call mouse-position-function after the
new C function returns.  Then in term.c we could call just that new C
function.  This is because it is inappropriate for mouse_get_xy to
call mouse-position-function when a menu is being processed.

Thanks.

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sun, 04 Oct 2020 09:43:12 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > Now there are newly emitted events for mouse-movement that are not 
> > handled such as "<mode-line> <mouse-movement>" or "<vertical-line> 
> > <mouse-movement>". It'd be easy enough to bind all of these to ignore 
> > and further update tty-menu-navigation-map to have more cases, but is 
> > that the right solution? I'm surprised that only xterm-mouse would run 
> > into this case.
> 
> Emacs normally doesn't receive mouse-movement events unless we
> explicitly enable that, see the macro track-mouse.  And the
> menu-processing code for other TTYs doesn't enable mouse-movement
> events because it isn't needed: with other mice the mouse coordinates
> can be read even if the button wasn't clicked.

Actually, I see that my memory was faulty, sorry: we do enable
mouse-movement events internally in read_menu_input.  And the map used
by TTY menus, defined in menu-bar.el, does have this binding:

    (define-key map [mouse-movement] 'tty-menu-mouse-movement)

So I'm not sure why you'd need to explicitly ignore mouse-movement
events on the mode line.  Can you tell more about the problems you saw
with that?

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

On 2020-10-03 11:24 pm, Eli Zaretskii wrote:
>> Date: Sat, 03 Oct 2020 12:26:49 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> This appears to be due to xt-mouse sending event code 1002 instead of
>> 1003 (see
>> https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking).
> 
> Hmm... maybe I'm misunderstanding what that page says, but it seems
> to say 1002 is for button click event and 1003 is for any mouse event:
> 
>      #define SET_BTN_EVENT_MOUSE         1002
>      #define SET_ANY_EVENT_MOUSE         1003
> 
> What am I missing?

A summary of my understanding of 
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking:

SET_VT200_MOUSE (1000) is for button press and release events.
SET_BTN_EVENT_MOUSE (1002) is for mouse move events while a button is 
pressed.
SET_ANY_EVENT_MOUSE (1003) is for all mouse move events.

Specifically, the section "Any-event tracking" makes the distinction 
between 1002 and 1003 clear. A direct quote:

Any-event tracking

Any-event mode is the same as button-event mode, except that all motion
events are reported, even if no mouse button is down.  It is enabled by
specifying 1003 to DECSET.

   -- MJF

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

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

>> > Now there are newly emitted events for mouse-movement that are not
>> > handled such as "<mode-line> <mouse-movement>" or "<vertical-line>
>> > <mouse-movement>". It'd be easy enough to bind all of these to ignore
>> > and further update tty-menu-navigation-map to have more cases, but is
>> > that the right solution? I'm surprised that only xterm-mouse would run
>> > into this case.
>> 
>> Emacs normally doesn't receive mouse-movement events unless we
>> explicitly enable that, see the macro track-mouse.  And the
>> menu-processing code for other TTYs doesn't enable mouse-movement
>> events because it isn't needed: with other mice the mouse coordinates
>> can be read even if the button wasn't clicked.
> 
> Actually, I see that my memory was faulty, sorry: we do enable
> mouse-movement events internally in read_menu_input.  And the map used
> by TTY menus, defined in menu-bar.el, does have this binding:
> 
>     (define-key map [mouse-movement] 'tty-menu-mouse-movement)
> 
> So I'm not sure why you'd need to explicitly ignore mouse-movement
> events on the mode line.  Can you tell more about the problems you saw
> with that?

Two things combined fixed all issues I saw: respecting track-mouse and 
adding all possible prefix bindings to tty-menu-navigation-map.

Without track-mouse being respected, I would see messages such as 
"<mode-line> <mouse-movement> is undefined" whenever I moved the mouse 
over those special areas of the window.

Without adding the prefix bindings to tty-menu-navigation-map, TTY menu 
interaction with the mouse would only work when the mouse was over the 
normal parts of the window and not any of the special areas such as 
mode-line.

With both of these changes, the menu bars menus work fine as well as 
usual mouse drag operations such as highlighting text or dragging the 
mode-line to resize windows.

>> diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
>> index 362d29b943..ae6f85f1f7 100644
>> --- a/lisp/xt-mouse.el
>> +++ b/lisp/xt-mouse.el
>> @@ -339,7 +339,7 @@ xterm-mouse-tracking-enable-sequence
>>              position (<= 223), which can be reported in this
>>              basic mode.
>> 
>> -\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
>> +\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
>>              motion events during dragging operations.
> 
> I think we should only switch to 0x1003 when the track-mouse variable
> is non-nil, see the macro track-mouse and its internal part
> internal--track-mouse.

I did this slightly differently as I did not find any way to be notified 
that track-mouse has changed. Instead I suppress mouse events from ever 
being emitted if track-mouse is nil. See xterm-mouse-translate-1 in my 
patch.

>> @@ -2804,16 +2814,14 @@ tty_menu_calc_size (tty_menu *menu, int 
>> *width, int *height)
>>  static void
>>  mouse_get_xy (int *x, int *y)
>>  {
>> -  struct frame *sf = SELECTED_FRAME ();
>> -  Lisp_Object lmx = Qnil, lmy = Qnil, lisp_dummy;
>> -  enum scroll_bar_part part_dummy;
>> -  Time time_dummy;
>> -
>> -  if (FRAME_TERMINAL (sf)->mouse_position_hook)
>> -    (*FRAME_TERMINAL (sf)->mouse_position_hook) (&sf, -1,
>> -                                                 &lisp_dummy, 
>> &part_dummy,
>> -						 &lmx, &lmy,
>> -						 &time_dummy);
>> +  Lisp_Object lmx = Qnil, lmy = Qnil, mouse_position = 
>> Fmouse_position ();
>> +
>> +  if (EQ (selected_frame, XCAR(mouse_position)))
>> +    {
>> +      lmx = XCAR (XCDR (mouse_position));
>> +      lmy = XCDR (XCDR (mouse_position));
>> +    }
>> +
> 
> This is okay as a general idea, but it would be more clean to make
> Fmouse_position call a new C function (extracted from the first part
> of Fmouse_position's code) that just computes the mouse coordinates.
> Then Fmouse_position's would call mouse-position-function after the
> new C function returns.  Then in term.c we could call just that new C
> function.  This is because it is inappropriate for mouse_get_xy to
> call mouse-position-function when a menu is being processed.
> 
> Thanks.

That won't work. Making mouse_get_xy call mouse-position-function is the 
purpose of this change. mouse-position-function is how xterm-mouse 
injects its calculated mouse positions to the TTY menus. From searching 
the Emacs codebase for usage of mouse-position-function, it appears that 
xterm-mouse is the only client. Of course external libraries could use 
it too, but I assume they would have the same goal.

New patches attached. BTW, is it helpful to have this split up into two 
patches (three if you count my bug fix)? I did it this way to make 
reviewing and accepting easier, but if you would rather have one bigger 
patch, I'm fine doing that too.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adding-mouse-controls-to-menu-bar.el.patch --]
[-- Type: text/x-diff; name=0001-Adding-mouse-controls-to-menu-bar.el.patch, Size: 8036 bytes --]

From 3366c93ea0c46bc5f95018fe23889f6b4bba267d Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 19 Sep 2020 00:43:29 -0700
Subject: [PATCH 1/2] Adding mouse controls to menu-bar.el.

---
 lisp/isearch.el  |  4 +--
 lisp/menu-bar.el | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 lisp/tmm.el      | 64 ++++++-----------------------------------
 3 files changed, 84 insertions(+), 58 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 781a8c5a93..1d7321f0e3 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -54,7 +54,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl-lib))
-(declare-function tmm-menubar-keymap "tmm.el")
+(declare-function menu-bar-keymap "menu-bar.el")
 \f
 ;; Some additional options and constants.
 
@@ -501,7 +501,7 @@ isearch-tmm-menubar
   (require 'tmm)
   (run-hooks 'menu-bar-update-hook)
   (let ((command nil))
-    (let ((menu-bar (tmm-menubar-keymap)))
+    (let ((menu-bar (menu-bar-keymap)))
       (with-isearch-suspended
        (setq command (let ((isearch-mode t)) ; Show bindings from
                                              ; `isearch-mode-map' in
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index d3e434aec9..4a0a27415b 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2660,6 +2660,80 @@ menu-bar-open
 
 (global-set-key [f10] 'menu-bar-open)
 
+(defun menu-bar-open-mouse (event)
+  "Mosue-triggered version of `menu-bar-open'.
+This command is to be used when you click the mouse in the menubar."
+  (interactive "e")
+  (require 'tmm)       ; Possibly have tmm depend on menu-bar instead?
+  (let* ((x-position (car (posn-x-y (event-start event))))
+         (menu-bar-item-cons (menu-bar-item-at-x x-position)))
+    (menu-bar-open nil
+                   (if menu-bar-item-cons
+                       (cdr menu-bar-item-cons)
+                     0))))
+
+(defun menu-bar-keymap ()
+  "Return the current menu-bar keymap.
+
+The ordering of the return value respects `menu-bar-final-items'."
+  (let ((menu-bar '())
+        (menu-end '()))
+    (map-keymap
+     (lambda (key binding)
+       (let ((pos (seq-position menu-bar-final-items key))
+             (menu-item (cons key binding)))
+         (if pos
+             ;; If KEY is the name of an item that we want to put
+             ;; last, store it separately with explicit ordering for
+             ;; sorting.
+             (push (cons pos menu-item) menu-end)
+           (push menu-item menu-bar))))
+     (menu-bar-get-keybind [menu-bar]))
+    `(keymap ,@(nreverse menu-bar)
+             ,@(mapcar 'cdr (sort menu-end
+                                  (lambda (a b) (< (car a) (car b))))))))
+(defun menu-bar-get-keybind (keyseq)
+  "Return the current binding of KEYSEQ, merging prefix definitions.
+If KEYSEQ is a prefix key that has local and global bindings,
+we merge them into a single keymap which shows the proper order of the menu.
+However, for the menu bar itself, the value does not take account
+of `menu-bar-final-items'."
+  (lookup-key (cons 'keymap (nreverse (current-active-maps))) keyseq))
+
+(defun menu-bar-item-at-x (x-position)
+  "Return a cons of the form (KEY . X) for the item clicked on.
+
+If nothing is clicked on, returns nil."
+  (let ((column 0)
+        (menu-bar (menu-bar-keymap))
+        prev-key
+        prev-column
+        found)
+    (catch 'done
+      (map-keymap
+       (lambda (key binding)
+         (when (> column x-position)
+           (setq found t)
+           (throw 'done nil))
+         (setq prev-key key)
+         (pcase binding
+           ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
+                `(menu-item ,name ,_cmd            ;Extended menu item.
+                            . ,(and props
+                                    (guard (let ((visible
+                                                  (plist-get props :visible)))
+                                             (or (null visible)
+                                                 (eval visible)))))))
+            (setq prev-column column
+                  column (+ column (length name) 1)))))
+       menu-bar)
+      ;; Check the last menu item.
+      (when (> column x-position)
+        (setq found t)))
+    (if found
+        (cons prev-key prev-column)
+      nil)))
+
 (defun buffer-menu-open ()
   "Start key navigation of the buffer menu.
 This is the keyboard interface to \\[mouse-buffer-menu]."
diff --git a/lisp/tmm.el b/lisp/tmm.el
index 074ee7593f..b73af9cda3 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -28,6 +28,8 @@
 ;;; Code:
 
 (require 'electric)
+(declare-function menu-bar-keymap "menu-bar.el")
+(declare-function menu-bar-item-at-x "menu-bar.el")
 
 (defgroup tmm nil
   "Text mode access to menu-bar."
@@ -42,27 +44,6 @@ tmm-km-list
 (defvar tmm-next-shortcut-digit)
 (defvar tmm-table-undef)
 
-(defun tmm-menubar-keymap ()
-  "Return the current menu-bar keymap.
-
-The ordering of the return value respects `menu-bar-final-items'."
-  (let ((menu-bar '())
-        (menu-end '()))
-    (map-keymap
-     (lambda (key binding)
-       (let ((pos (seq-position menu-bar-final-items key))
-             (menu-item (cons key binding)))
-         (if pos
-             ;; If KEY is the name of an item that we want to put
-             ;; last, store it separately with explicit ordering for
-             ;; sorting.
-             (push (cons pos menu-item) menu-end)
-           (push menu-item menu-bar))))
-     (tmm-get-keybind [menu-bar]))
-    `(keymap ,@(nreverse menu-bar)
-             ,@(mapcar 'cdr (sort menu-end
-                                  (lambda (a b) (< (car a) (car b))))))))
-
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
 ;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
 
@@ -78,33 +59,12 @@ tmm-menubar
 `tty-menu-open-use-tmm' to a non-nil value."
   (interactive)
   (run-hooks 'menu-bar-update-hook)
-  ;; Obey menu-bar-final-items; put those items last.
-  (let ((menu-bar (tmm-menubar-keymap))
-	menu-bar-item)
-    (if x-position
-	(let ((column 0)
-              prev-key)
-          (catch 'done
-            (map-keymap
-             (lambda (key binding)
-               (when (> column x-position)
-                 (setq menu-bar-item prev-key)
-                 (throw 'done nil))
-               (setq prev-key key)
-               (pcase binding
-                 ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
-                      `(menu-item ,name ,_cmd            ;Extended menu item.
-                        . ,(and props
-                                (guard (let ((visible
-                                              (plist-get props :visible)))
-                                         (or (null visible)
-                                             (eval visible)))))))
-                  (setq column (+ column (length name) 1)))))
-             menu-bar)
-            ;; Check the last menu item.
-            (when (> column x-position)
-              (setq menu-bar-item prev-key)))))
-    (tmm-prompt menu-bar nil menu-bar-item)))
+  (let ((menu-bar (menu-bar-keymap))
+        (menu-bar-item-cons (and x-position
+                                 (menu-bar-item-at-x x-position))))
+    (tmm-prompt menu-bar
+                nil
+                (and menu-bar-item-cons (car menu-bar-item-cons)))))
 
 ;;;###autoload
 (defun tmm-menubar-mouse (event)
@@ -524,14 +484,6 @@ tmm-get-keymap
 	   (or (assoc str tmm-km-list)
 	       (push (cons str (cons event km)) tmm-km-list))))))
 
-(defun tmm-get-keybind (keyseq)
-  "Return the current binding of KEYSEQ, merging prefix definitions.
-If KEYSEQ is a prefix key that has local and global bindings,
-we merge them into a single keymap which shows the proper order of the menu.
-However, for the menu bar itself, the value does not take account
-of `menu-bar-final-items'."
-  (lookup-key (cons 'keymap (nreverse (current-active-maps))) keyseq))
-
 (provide 'tmm)
 
 ;;; tmm.el ends here
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch --]
[-- Type: text/x-diff; name=0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch, Size: 9141 bytes --]

From a8831a847059a448dda1d4502d5b8f486b6bf0e5 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 3 Oct 2020 14:46:30 -0700
Subject: [PATCH 2/2] Making TTY menus work with xterm-mouse-mode.

* xt-mouse.el uses SET_ANY_EVENT_MOUSE (1003) so mouse movement can be
  reported even if no buttons are pressed.
* xt-mouse.el now respects track-mouse. It previously did not need to
  since mouse movement was only reported if a mouse button was pressed.
* Hook up mouse_get_xy to mouse-position so it works with xterm mouse.
* tty-navigation-map handles all the different prefixes mouse events can
  have, such as menu-bar or mode-line.
---
 lisp/menu-bar.el | 64 +++++++++++++++++++++++++-----------------------
 lisp/xt-mouse.el | 20 +++++++++------
 src/term.c       | 18 ++++++--------
 3 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 4a0a27415b..df27b6d456 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2753,6 +2753,16 @@ mouse-buffer-menu-keymap
                 (menu-bar-buffer-vector item)))))
     km))
 
+(defun menu-bar-define-mouse-key (map key def)
+  "Like `define-key', but adds all possible prefixes for the mouse."
+  (define-key map (vector key) def)
+  (mapc (lambda (prefix) (define-key map (vector prefix key) def))
+        ;; This list only needs to contain special window areas that
+        ;; are rendered in TTYs. No need for *-scroll-bar, *-fringe, or
+        ;; *-divider.
+        '(tab-line header-line menu-bar tab-bar mode-line vertical-line
+          left-margin right-margin)))
+
 (defvar tty-menu-navigation-map
   (let ((map (make-sparse-keymap)))
     ;; The next line is disabled because it breaks interpretation of
@@ -2787,39 +2797,33 @@ tty-menu-navigation-map
     (define-key map [?\C-j] 'tty-menu-select)
     (define-key map [return] 'tty-menu-select)
     (define-key map [linefeed] 'tty-menu-select)
-    (define-key map [mouse-1] 'tty-menu-select)
-    (define-key map [drag-mouse-1] 'tty-menu-select)
-    (define-key map [mouse-2] 'tty-menu-select)
-    (define-key map [drag-mouse-2] 'tty-menu-select)
-    (define-key map [mouse-3] 'tty-menu-select)
-    (define-key map [drag-mouse-3] 'tty-menu-select)
-    (define-key map [wheel-down] 'tty-menu-next-item)
-    (define-key map [wheel-up] 'tty-menu-prev-item)
-    (define-key map [wheel-left] 'tty-menu-prev-menu)
-    (define-key map [wheel-right] 'tty-menu-next-menu)
-    ;; The following 4 bindings are for those whose text-mode mouse
+    (menu-bar-define-mouse-key map 'mouse-1 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-1 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'mouse-2 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-2 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'mouse-3 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-3 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'wheel-down 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'wheel-up 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'wheel-left 'tty-menu-prev-menu)
+    (menu-bar-define-mouse-key map 'wheel-right 'tty-menu-next-menu)
+    ;; The following 6 bindings are for those whose text-mode mouse
     ;; lack the wheel.
-    (define-key map [S-mouse-1] 'tty-menu-next-item)
-    (define-key map [S-drag-mouse-1] 'tty-menu-next-item)
-    (define-key map [S-mouse-2] 'tty-menu-prev-item)
-    (define-key map [S-drag-mouse-2] 'tty-menu-prev-item)
-    (define-key map [S-mouse-3] 'tty-menu-prev-item)
-    (define-key map [S-drag-mouse-3] 'tty-menu-prev-item)
-    (define-key map [header-line mouse-1] 'tty-menu-select)
-    (define-key map [header-line drag-mouse-1] 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'S-mouse-1 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-1 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'S-mouse-2 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-2 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-mouse-3 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-3 'tty-menu-prev-item)
     ;; The down-mouse events must be bound to tty-menu-ignore, so that
     ;; only releasing the mouse button pops up the menu.
-    (define-key map [mode-line down-mouse-1] 'tty-menu-ignore)
-    (define-key map [mode-line down-mouse-2] 'tty-menu-ignore)
-    (define-key map [mode-line down-mouse-3] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-1] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-2] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-3] 'tty-menu-ignore)
-    (define-key map [down-mouse-1] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-1] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-2] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-3] 'tty-menu-ignore)
-    (define-key map [mouse-movement] 'tty-menu-mouse-movement)
+    (menu-bar-define-mouse-key map 'down-mouse-1 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'down-mouse-2 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'down-mouse-3 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-1 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-2 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-3 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'mouse-movement 'tty-menu-mouse-movement)
     map)
   "Keymap used while processing TTY menus.")
 
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 362d29b943..5410aafb3b 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -76,7 +76,11 @@ xterm-mouse-translate-1
               ;; to guard against that.
               (copy-sequence event))
 	vec)
-       (is-move vec)
+       (is-move
+        (if track-mouse vec
+          ;; Mouse movement events are currently supposed to be
+          ;; suppressed. Return no event.
+          []))
        (t
 	(let* ((down (terminal-parameter nil 'xterm-mouse-last-down))
 	       (down-data (nth 1 down))
@@ -339,8 +343,8 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
-            motion events during dragging operations.
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an
             extension to the basic mouse mode, which uses UTF-8
@@ -360,7 +364,7 @@ xterm-mouse-tracking-enable-sequence
   (apply #'concat (xterm-mouse--tracking-sequence ?h)))
 
 (defconst xterm-mouse-tracking-enable-sequence
-  "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"
+  "\e[?1000h\e[?1003h\e[?1005h\e[?1006h"
   "Control sequence to enable xterm mouse tracking.
 Enables basic mouse tracking, mouse motion events and finally
 extended tracking on terminals that support it. The following
@@ -371,8 +375,8 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
-            motion events during dragging operations.
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an extension
             to the basic mouse mode, which uses UTF-8
@@ -400,7 +404,7 @@ xterm-mouse-tracking-disable-sequence
   (apply #'concat (nreverse (xterm-mouse--tracking-sequence ?l))))
 
 (defconst xterm-mouse-tracking-disable-sequence
-  "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"
+  "\e[?1006l\e[?1005l\e[?1003l\e[?1000l"
   "Reset the modes set by `xterm-mouse-tracking-enable-sequence'.")
 
 (make-obsolete-variable
@@ -414,7 +418,7 @@ xterm-mouse--tracking-sequence
 enable, ?l to disable)."
   (mapcar
    (lambda (code) (format "\e[?%d%c" code suffix))
-   `(1000 1002 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
+   `(1000 1003 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
 
 (defun turn-on-xterm-mouse-tracking-on-terminal (&optional terminal)
   "Enable xterm mouse tracking on TERMINAL."
diff --git a/src/term.c b/src/term.c
index 3677644845..564d55be5b 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2804,16 +2804,14 @@ tty_menu_calc_size (tty_menu *menu, int *width, int *height)
 static void
 mouse_get_xy (int *x, int *y)
 {
-  struct frame *sf = SELECTED_FRAME ();
-  Lisp_Object lmx = Qnil, lmy = Qnil, lisp_dummy;
-  enum scroll_bar_part part_dummy;
-  Time time_dummy;
-
-  if (FRAME_TERMINAL (sf)->mouse_position_hook)
-    (*FRAME_TERMINAL (sf)->mouse_position_hook) (&sf, -1,
-                                                 &lisp_dummy, &part_dummy,
-						 &lmx, &lmy,
-						 &time_dummy);
+  Lisp_Object lmx = Qnil, lmy = Qnil, mouse_position = Fmouse_position ();
+
+  if (EQ (selected_frame, XCAR(mouse_position)))
+    {
+      lmx = XCAR (XCDR (mouse_position));
+      lmy = XCDR (XCDR (mouse_position));
+    }
+
   if (!NILP (lmx))
     {
       *x = XFIXNUM (lmx);
-- 
2.20.1

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sun, 04 Oct 2020 22:36:21 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> > This is okay as a general idea, but it would be more clean to make
> > Fmouse_position call a new C function (extracted from the first part
> > of Fmouse_position's code) that just computes the mouse coordinates.
> > Then Fmouse_position's would call mouse-position-function after the
> > new C function returns.  Then in term.c we could call just that new C
> > function.  This is because it is inappropriate for mouse_get_xy to
> > call mouse-position-function when a menu is being processed.
> 
> That won't work. Making mouse_get_xy call mouse-position-function is the 
> purpose of this change. mouse-position-function is how xterm-mouse 
> injects its calculated mouse positions to the TTY menus. From searching 
> the Emacs codebase for usage of mouse-position-function, it appears that 
> xterm-mouse is the only client. Of course external libraries could use 
> it too, but I assume they would have the same goal.

Okay, but mouse-position-function can be used for any number of
purposes.  That it's currently used only by xterm-mouse in the Emacs
sources doesn't mean it isn't used elsewhere.  The functions in that
variable are not necessarily meant to be called in the context of TTY
menus; that would be an incompatible change.  So I still think we
shouldn't call mouse-position-function in general in the TTY menu
case, only when xterm-mouse is active.  Please change the code to call
mouse-position-function only in that single case.

Perhaps a slightly more general way of doing that would be to
introduce another variable that a package must set for
mouse-position-function to be called in the context of TTY menus;
xterm-mouse will then bind that variable (or the TTY menu code in
menu-bar.el could do that for xterm-mouse).  This way, we don't
hard-code xterm-mouse in the C sources.

> New patches attached. BTW, is it helpful to have this split up into two 
> patches (three if you count my bug fix)? I did it this way to make 
> reviewing and accepting easier, but if you would rather have one bigger 
> patch, I'm fine doing that too.

It doesn't matter much.  I personally prefer a single patch for each
changeset that makes sense on its own right.

A few minor comments below.

> diff --git a/lisp/isearch.el b/lisp/isearch.el
> index 781a8c5a93..1d7321f0e3 100644
> --- a/lisp/isearch.el
> +++ b/lisp/isearch.el
> @@ -54,7 +54,7 @@
>  ;;; Code:
>  
>  (eval-when-compile (require 'cl-lib))
> -(declare-function tmm-menubar-keymap "tmm.el")
> +(declare-function menu-bar-keymap "menu-bar.el")

Why did you need this part?  menu-bar.el is preloaded, so the function
menu-bar-keymap should be known to Emacs.

> +(defun menu-bar-open-mouse (event)
> +  "Mosue-triggered version of `menu-bar-open'.
> +This command is to be used when you click the mouse in the menubar."
> +  (interactive "e")
> +  (require 'tmm)       ; Possibly have tmm depend on menu-bar instead?
     ^^^^^^^^^^^^^^
I'd like to avoid this.  menu-bar.el is preloaded, so the above will
make tmm.el preloaded, for no good reason from my POV.

Can you explain why you needed to require tmm?  (Doing this the other
way around is definitely okay, since menu-bar.el is preloaded.)

> +(defun menu-bar-get-keybind (keyseq)
                       ^^^^^^^
"keybinding".  Or just "binding".

> diff --git a/lisp/tmm.el b/lisp/tmm.el
> index 074ee7593f..b73af9cda3 100644
> --- a/lisp/tmm.el
> +++ b/lisp/tmm.el
> @@ -28,6 +28,8 @@
>  ;;; Code:
>  
>  (require 'electric)
> +(declare-function menu-bar-keymap "menu-bar.el")
> +(declare-function menu-bar-item-at-x "menu-bar.el")

Again, these two shouldn't be needed, since menu-bar.el is preloaded.

Last, but not least: this needs a NEWS entry, announcing TTY menus
support by xterm-mouse.

Thanks.

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

On 2020-10-04 11:45 pm, Eli Zaretskii wrote:
>> Date: Sun, 04 Oct 2020 22:36:21 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > This is okay as a general idea, but it would be more clean to make
>> > Fmouse_position call a new C function (extracted from the first part
>> > of Fmouse_position's code) that just computes the mouse coordinates.
>> > Then Fmouse_position's would call mouse-position-function after the
>> > new C function returns.  Then in term.c we could call just that new C
>> > function.  This is because it is inappropriate for mouse_get_xy to
>> > call mouse-position-function when a menu is being processed.
>> 
>> That won't work. Making mouse_get_xy call mouse-position-function is 
>> the
>> purpose of this change. mouse-position-function is how xterm-mouse
>> injects its calculated mouse positions to the TTY menus. From 
>> searching
>> the Emacs codebase for usage of mouse-position-function, it appears 
>> that
>> xterm-mouse is the only client. Of course external libraries could use
>> it too, but I assume they would have the same goal.
> 
> Okay, but mouse-position-function can be used for any number of
> purposes.  That it's currently used only by xterm-mouse in the Emacs
> sources doesn't mean it isn't used elsewhere.  The functions in that
> variable are not necessarily meant to be called in the context of TTY
> menus; that would be an incompatible change.  So I still think we
> shouldn't call mouse-position-function in general in the TTY menu
> case, only when xterm-mouse is active.  Please change the code to call
> mouse-position-function only in that single case.
> 
> Perhaps a slightly more general way of doing that would be to
> introduce another variable that a package must set for
> mouse-position-function to be called in the context of TTY menus;
> xterm-mouse will then bind that variable (or the TTY menu code in
> menu-bar.el could do that for xterm-mouse).  This way, we don't
> hard-code xterm-mouse in the C sources.

I'm going to push back here one more time. You have more expertise here 
than I, so if this isn't convincing, I can implement such a variable 
(it's easy enough).

I'm pushing back because I can not envision a use case for 
mouse-position-function that would break for TTY menus but otherwise 
work for every other type of normal mouse input. Adding this extra step 
that must be done by all clients seems like adding needless complexity.

I did a much more exhaustive search of Emacs code and remain convinced 
that the only use of mouse-position-function is for cases just like 
xt-mouse.el:

I did a search of MELPA to see if there was any use of 
mouse-position-function. I found nothing.

I searched GitHub for any usage and only found usages aligned with 
xt-mouse, specifically:

* xt-mouse.el, loadhist.el from Gnu Emacs repo forks.
* t-mouse.el from (old) Gnu Emacs repo forks.
* ext-mouse.el, a modification of an old version of xt-mouse.el that 
adds dragging support.
* hyperbole.el from Gnu Elpa, which uses this to fix a bug in old 
versions of Gnu Emacs.
* A handful of other files that do not actually set 
mouse-position-function, they instead deal with other aspects (e.g. 
syntax highlighting built-in symbols)

I realize this isn't an exhaustive list of all possible code, but I 
suspect it is a very representative list.

Let me know your thoughts. As I said, I think you have more expertise 
here so I will defer to your call.

   -- MJF

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Wed, 07 Oct 2020 23:39:07 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> > Okay, but mouse-position-function can be used for any number of
> > purposes.  That it's currently used only by xterm-mouse in the Emacs
> > sources doesn't mean it isn't used elsewhere.  The functions in that
> > variable are not necessarily meant to be called in the context of TTY
> > menus; that would be an incompatible change.  So I still think we
> > shouldn't call mouse-position-function in general in the TTY menu
> > case, only when xterm-mouse is active.  Please change the code to call
> > mouse-position-function only in that single case.
> > 
> > Perhaps a slightly more general way of doing that would be to
> > introduce another variable that a package must set for
> > mouse-position-function to be called in the context of TTY menus;
> > xterm-mouse will then bind that variable (or the TTY menu code in
> > menu-bar.el could do that for xterm-mouse).  This way, we don't
> > hard-code xterm-mouse in the C sources.
> 
> I'm going to push back here one more time. You have more expertise here 
> than I, so if this isn't convincing, I can implement such a variable 
> (it's easy enough).
> 
> I'm pushing back because I can not envision a use case for 
> mouse-position-function that would break for TTY menus but otherwise 
> work for every other type of normal mouse input. Adding this extra step 
> that must be done by all clients seems like adding needless complexity.

The added complexity is minor, won't you agree?  And it only affects
xt-mouse itself and its workalikes, and then only in the context of
TTY menus, something that never worked until now.  So any client that
wants to support menus will need to make changes anyway.  Any other
uses of mouse-position-function are unaffected.  So I don't quite see
where we put any tangible burden on other clients.  Am I missing
something?

> I did a much more exhaustive search of Emacs code and remain convinced 
> that the only use of mouse-position-function is for cases just like 
> xt-mouse.el:

I don't think we can decide on a backward-incompatible change based on
any reasonable search of ELisp code out there.  Emacs is stable, which
means users and programmers rightfully expect us not to break backward
compatibility unless it is really necessary and justified.  In this
case, the price for staying backward compatible is small, so I think
the balance is clearly in favor of not calling the hook inside TTY
menus.  The implementation of TTY menus is very delicate, so I'd very
much prefer to avoid any unnecessary Lisp calls while a menu is
displayed, because calling Lisp there is in effect a permission for
user programs to do whatever they like -- we cannot control what they
do.  For example, if the Lisp this hook calls causes redisplay, the
menu display could potentially be screwed.

> I realize this isn't an exhaustive list of all possible code, but I 
> suspect it is a very representative list.
> 
> Let me know your thoughts. As I said, I think you have more expertise 
> here so I will defer to your call.

Based on the analysis above, I think the price for staying compatible
is very small, while the advantage of that is significant, as that is
what our users expect from us.

Thanks.

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

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

On 2020-10-08 1:15 am, Eli Zaretskii wrote:
>> Date: Wed, 07 Oct 2020 23:39:07 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> I realize this isn't an exhaustive list of all possible code, but I
>> suspect it is a very representative list.
>> 
>> Let me know your thoughts. As I said, I think you have more expertise
>> here so I will defer to your call.
> 
> Based on the analysis above, I think the price for staying compatible
> is very small, while the advantage of that is significant, as that is
> what our users expect from us.

Thanks for the detailed reasoning. I have updated my patches to only 
conditionally call mouse-position-function, as requested. I've also 
added one additional patch that rebinds [menu-bar mouse-1] to get TTY 
menus by default.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adding-mouse-controls-to-menu-bar.el.patch --]
[-- Type: text/x-diff; name=0001-Adding-mouse-controls-to-menu-bar.el.patch, Size: 7783 bytes --]

From 701d927312353c249c4ae616606ec846b42dc66d Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 19 Sep 2020 00:43:29 -0700
Subject: [PATCH 1/3] Adding mouse controls to menu-bar.el.

---
 lisp/isearch.el  |  3 +-
 lisp/menu-bar.el | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
 lisp/tmm.el      | 63 ++++------------------------------------
 3 files changed, 82 insertions(+), 59 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 781a8c5a93..01d1509a36 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -54,7 +54,6 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl-lib))
-(declare-function tmm-menubar-keymap "tmm.el")
 \f
 ;; Some additional options and constants.
 
@@ -501,7 +500,7 @@ isearch-tmm-menubar
   (require 'tmm)
   (run-hooks 'menu-bar-update-hook)
   (let ((command nil))
-    (let ((menu-bar (tmm-menubar-keymap)))
+    (let ((menu-bar (menu-bar-keymap)))
       (with-isearch-suspended
        (setq command (let ((isearch-mode t)) ; Show bindings from
                                              ; `isearch-mode-map' in
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 1b3e102cfa..6ab006809b 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2660,6 +2660,81 @@ menu-bar-open
 
 (global-set-key [f10] 'menu-bar-open)
 
+(defun menu-bar-open-mouse (event)
+  "Mosue-triggered version of `menu-bar-open'.
+This command is to be used when you click the mouse in the menubar."
+  (interactive "e")
+  (let* ((x-position (car (posn-x-y (event-start event))))
+         (menu-bar-item-cons (menu-bar-item-at-x x-position)))
+    (menu-bar-open nil
+                   (if menu-bar-item-cons
+                       (cdr menu-bar-item-cons)
+                     0))))
+
+(defun menu-bar-keymap ()
+  "Return the current menu-bar keymap.
+
+The ordering of the return value respects `menu-bar-final-items'."
+  (let ((menu-bar '())
+        (menu-end '()))
+    (map-keymap
+     (lambda (key binding)
+       (let ((pos (seq-position menu-bar-final-items key))
+             (menu-item (cons key binding)))
+         (if pos
+             ;; If KEY is the name of an item that we want to put
+             ;; last, store it separately with explicit ordering for
+             ;; sorting.
+             (push (cons pos menu-item) menu-end)
+           (push menu-item menu-bar))))
+     (menu-bar-get-binding [menu-bar]))
+    `(keymap ,@(nreverse menu-bar)
+             ,@(mapcar #'cdr (sort menu-end
+                                   (lambda (a b)
+                                     (< (car a) (car b))))))))
+
+(defun menu-bar-get-binding (keyseq)
+  "Return the current binding of KEYSEQ, merging prefix definitions.
+If KEYSEQ is a prefix key that has local and global bindings,
+we merge them into a single keymap which shows the proper order of the menu.
+However, for the menu bar itself, the value does not take account
+of `menu-bar-final-items'."
+  (lookup-key (cons 'keymap (nreverse (current-active-maps))) keyseq))
+
+(defun menu-bar-item-at-x (x-position)
+  "Return a cons of the form (KEY . X) for the item clicked on.
+
+If nothing is clicked on, returns nil."
+  (let ((column 0)
+        (menu-bar (menu-bar-keymap))
+        prev-key
+        prev-column
+        found)
+    (catch 'done
+      (map-keymap
+       (lambda (key binding)
+         (when (> column x-position)
+           (setq found t)
+           (throw 'done nil))
+         (setq prev-key key)
+         (pcase binding
+           ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
+                `(menu-item ,name ,_cmd            ;Extended menu item.
+                            . ,(and props
+                                    (guard (let ((visible
+                                                  (plist-get props :visible)))
+                                             (or (null visible)
+                                                 (eval visible)))))))
+            (setq prev-column column
+                  column (+ column (length name) 1)))))
+       menu-bar)
+      ;; Check the last menu item.
+      (when (> column x-position)
+        (setq found t)))
+    (if found
+        (cons prev-key prev-column)
+      nil)))
+
 (defun buffer-menu-open ()
   "Start key navigation of the buffer menu.
 This is the keyboard interface to \\[mouse-buffer-menu]."
diff --git a/lisp/tmm.el b/lisp/tmm.el
index 0e83f427f5..fc02fd5790 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -42,28 +42,6 @@ tmm-km-list
 (defvar tmm-next-shortcut-digit)
 (defvar tmm-table-undef)
 
-(defun tmm-menubar-keymap ()
-  "Return the current menu-bar keymap.
-
-The ordering of the return value respects `menu-bar-final-items'."
-  (let ((menu-bar '())
-        (menu-end '()))
-    (map-keymap
-     (lambda (key binding)
-       (let ((pos (seq-position menu-bar-final-items key))
-             (menu-item (cons key binding)))
-         (if pos
-             ;; If KEY is the name of an item that we want to put
-             ;; last, store it separately with explicit ordering for
-             ;; sorting.
-             (push (cons pos menu-item) menu-end)
-           (push menu-item menu-bar))))
-     (tmm-get-keybind [menu-bar]))
-    `(keymap ,@(nreverse menu-bar)
-             ,@(mapcar #'cdr (sort menu-end
-                                   (lambda (a b)
-                                     (< (car a) (car b))))))))
-
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
 ;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
 
@@ -79,33 +57,12 @@ tmm-menubar
 `tty-menu-open-use-tmm' to a non-nil value."
   (interactive)
   (run-hooks 'menu-bar-update-hook)
-  ;; Obey menu-bar-final-items; put those items last.
-  (let ((menu-bar (tmm-menubar-keymap))
-	menu-bar-item)
-    (if x-position
-	(let ((column 0)
-              prev-key)
-          (catch 'done
-            (map-keymap
-             (lambda (key binding)
-               (when (> column x-position)
-                 (setq menu-bar-item prev-key)
-                 (throw 'done nil))
-               (setq prev-key key)
-               (pcase binding
-                 ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
-                      `(menu-item ,name ,_cmd            ;Extended menu item.
-                        . ,(and props
-                                (guard (let ((visible
-                                              (plist-get props :visible)))
-                                         (or (null visible)
-                                             (eval visible)))))))
-                  (setq column (+ column (length name) 1)))))
-             menu-bar)
-            ;; Check the last menu item.
-            (when (> column x-position)
-              (setq menu-bar-item prev-key)))))
-    (tmm-prompt menu-bar nil menu-bar-item)))
+  (let ((menu-bar (menu-bar-keymap))
+        (menu-bar-item-cons (and x-position
+                                 (menu-bar-item-at-x x-position))))
+    (tmm-prompt menu-bar
+                nil
+                (and menu-bar-item-cons (car menu-bar-item-cons)))))
 
 ;;;###autoload
 (defun tmm-menubar-mouse (event)
@@ -525,14 +482,6 @@ tmm-get-keymap
 	   (or (assoc str tmm-km-list)
 	       (push (cons str (cons event km)) tmm-km-list))))))
 
-(defun tmm-get-keybind (keyseq)
-  "Return the current binding of KEYSEQ, merging prefix definitions.
-If KEYSEQ is a prefix key that has local and global bindings,
-we merge them into a single keymap which shows the proper order of the menu.
-However, for the menu bar itself, the value does not take account
-of `menu-bar-final-items'."
-  (lookup-key (cons 'keymap (nreverse (current-active-maps))) keyseq))
-
 (provide 'tmm)
 
 ;;; tmm.el ends here
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch --]
[-- Type: text/x-diff; name=0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch, Size: 12373 bytes --]

From 1de85b6c325501daa7adbea5330ddb2e3701fe28 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 3 Oct 2020 14:46:30 -0700
Subject: [PATCH 2/3] Making TTY menus work with xterm-mouse-mode.

* xt-mouse.el uses SET_ANY_EVENT_MOUSE (1003) so mouse movement can be
  reported even if no buttons are pressed.
* xt-mouse.el now respects track-mouse. It previously did not need to
  since mouse movement was only reported if a mouse button was pressed.
* Conditionally hook up mouse_get_xy to mouse-position-function so it
  works with xterm mouse. This is controlled by a new variable,
  tty-menu-calls-mouse-position-function.
* tty-navigation-map handles all the different prefixes mouse events can
  have, such as menu-bar or mode-line.
---
 etc/NEWS         |  5 ++++
 lisp/menu-bar.el | 64 +++++++++++++++++++++++++-----------------------
 lisp/xt-mouse.el | 23 ++++++++++-------
 src/frame.c      |  8 +++++-
 src/frame.h      |  1 +
 src/term.c       | 26 ++++++++++++--------
 6 files changed, 77 insertions(+), 50 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d05b706ea3..7a776b3d56 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1621,6 +1621,11 @@ convert them to a list '(R G B)' of primary color values.
 This user option can be one of the predefined styles or a function to
 personalize the uniquified buffer name.
 
++++
+** New variable 'tty-menu-calls-mouse-position-function'.
+This controls if TTY menu logic calls mouse-position-function. Libraries that
+set mouse-position-function should also set this to non-nil if they can confirm
+they are compatible with the tty menu logic.
 
 \f
 * Changes in Emacs 28.1 on Non-Free Operating Systems
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 6ab006809b..518d0cc024 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2754,6 +2754,16 @@ mouse-buffer-menu-keymap
                 (menu-bar-buffer-vector item)))))
     km))
 
+(defun menu-bar-define-mouse-key (map key def)
+  "Like `define-key', but adds all possible prefixes for the mouse."
+  (define-key map (vector key) def)
+  (mapc (lambda (prefix) (define-key map (vector prefix key) def))
+        ;; This list only needs to contain special window areas that
+        ;; are rendered in TTYs. No need for *-scroll-bar, *-fringe, or
+        ;; *-divider.
+        '(tab-line header-line menu-bar tab-bar mode-line vertical-line
+          left-margin right-margin)))
+
 (defvar tty-menu-navigation-map
   (let ((map (make-sparse-keymap)))
     ;; The next line is disabled because it breaks interpretation of
@@ -2788,39 +2798,33 @@ tty-menu-navigation-map
     (define-key map [?\C-j] 'tty-menu-select)
     (define-key map [return] 'tty-menu-select)
     (define-key map [linefeed] 'tty-menu-select)
-    (define-key map [mouse-1] 'tty-menu-select)
-    (define-key map [drag-mouse-1] 'tty-menu-select)
-    (define-key map [mouse-2] 'tty-menu-select)
-    (define-key map [drag-mouse-2] 'tty-menu-select)
-    (define-key map [mouse-3] 'tty-menu-select)
-    (define-key map [drag-mouse-3] 'tty-menu-select)
-    (define-key map [wheel-down] 'tty-menu-next-item)
-    (define-key map [wheel-up] 'tty-menu-prev-item)
-    (define-key map [wheel-left] 'tty-menu-prev-menu)
-    (define-key map [wheel-right] 'tty-menu-next-menu)
-    ;; The following 4 bindings are for those whose text-mode mouse
+    (menu-bar-define-mouse-key map 'mouse-1 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-1 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'mouse-2 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-2 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'mouse-3 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-3 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'wheel-down 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'wheel-up 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'wheel-left 'tty-menu-prev-menu)
+    (menu-bar-define-mouse-key map 'wheel-right 'tty-menu-next-menu)
+    ;; The following 6 bindings are for those whose text-mode mouse
     ;; lack the wheel.
-    (define-key map [S-mouse-1] 'tty-menu-next-item)
-    (define-key map [S-drag-mouse-1] 'tty-menu-next-item)
-    (define-key map [S-mouse-2] 'tty-menu-prev-item)
-    (define-key map [S-drag-mouse-2] 'tty-menu-prev-item)
-    (define-key map [S-mouse-3] 'tty-menu-prev-item)
-    (define-key map [S-drag-mouse-3] 'tty-menu-prev-item)
-    (define-key map [header-line mouse-1] 'tty-menu-select)
-    (define-key map [header-line drag-mouse-1] 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'S-mouse-1 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-1 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'S-mouse-2 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-2 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-mouse-3 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-3 'tty-menu-prev-item)
     ;; The down-mouse events must be bound to tty-menu-ignore, so that
     ;; only releasing the mouse button pops up the menu.
-    (define-key map [mode-line down-mouse-1] 'tty-menu-ignore)
-    (define-key map [mode-line down-mouse-2] 'tty-menu-ignore)
-    (define-key map [mode-line down-mouse-3] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-1] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-2] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-3] 'tty-menu-ignore)
-    (define-key map [down-mouse-1] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-1] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-2] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-3] 'tty-menu-ignore)
-    (define-key map [mouse-movement] 'tty-menu-mouse-movement)
+    (menu-bar-define-mouse-key map 'down-mouse-1 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'down-mouse-2 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'down-mouse-3 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-1 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-2 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-3 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'mouse-movement 'tty-menu-mouse-movement)
     map)
   "Keymap used while processing TTY menus.")
 
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 362d29b943..954cf23ee6 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -76,7 +76,11 @@ xterm-mouse-translate-1
               ;; to guard against that.
               (copy-sequence event))
 	vec)
-       (is-move vec)
+       (is-move
+        (if track-mouse vec
+          ;; Mouse movement events are currently supposed to be
+          ;; suppressed. Return no event.
+          []))
        (t
 	(let* ((down (terminal-parameter nil 'xterm-mouse-last-down))
 	       (down-data (nth 1 down))
@@ -321,7 +325,8 @@ xterm-mouse-mode
   (if xterm-mouse-mode
       ;; Turn it on
       (progn
-	(setq mouse-position-function #'xterm-mouse-position-function)
+        (setq mouse-position-function #'xterm-mouse-position-function
+              tty-menu-calls-mouse-position-function t)
         (mapc #'turn-on-xterm-mouse-tracking-on-terminal (terminal-list)))
     ;; Turn it off
     (mapc #'turn-off-xterm-mouse-tracking-on-terminal (terminal-list))
@@ -339,8 +344,8 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
-            motion events during dragging operations.
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an
             extension to the basic mouse mode, which uses UTF-8
@@ -360,7 +365,7 @@ xterm-mouse-tracking-enable-sequence
   (apply #'concat (xterm-mouse--tracking-sequence ?h)))
 
 (defconst xterm-mouse-tracking-enable-sequence
-  "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"
+  "\e[?1000h\e[?1003h\e[?1005h\e[?1006h"
   "Control sequence to enable xterm mouse tracking.
 Enables basic mouse tracking, mouse motion events and finally
 extended tracking on terminals that support it. The following
@@ -371,8 +376,8 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
-            motion events during dragging operations.
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an extension
             to the basic mouse mode, which uses UTF-8
@@ -400,7 +405,7 @@ xterm-mouse-tracking-disable-sequence
   (apply #'concat (nreverse (xterm-mouse--tracking-sequence ?l))))
 
 (defconst xterm-mouse-tracking-disable-sequence
-  "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"
+  "\e[?1006l\e[?1005l\e[?1003l\e[?1000l"
   "Reset the modes set by `xterm-mouse-tracking-enable-sequence'.")
 
 (make-obsolete-variable
@@ -414,7 +419,7 @@ xterm-mouse--tracking-sequence
 enable, ?l to disable)."
   (mapcar
    (lambda (code) (format "\e[?%d%c" code suffix))
-   `(1000 1002 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
+   `(1000 1003 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
 
 (defun turn-on-xterm-mouse-tracking-on-terminal (&optional terminal)
   "Enable xterm mouse tracking on TERMINAL."
diff --git a/src/frame.c b/src/frame.c
index 0b707c2af8..5d967a59ce 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2433,6 +2433,12 @@ DEFUN ("mouse-position", Fmouse_position, Smouse_position, 0, 0, 0,
 passing the normal return value to that function as an argument,
 and returns whatever that function returns.  */)
   (void)
+{
+  return mouse_position (true);
+}
+
+Lisp_Object
+mouse_position (bool call_mouse_position_function)
 {
   struct frame *f;
   Lisp_Object lispy_dummy;
@@ -2462,7 +2468,7 @@ DEFUN ("mouse-position", Fmouse_position, Smouse_position, 0, 0, 0,
     }
   XSETFRAME (lispy_dummy, f);
   retval = Fcons (lispy_dummy, Fcons (x, y));
-  if (!NILP (Vmouse_position_function))
+  if (call_mouse_position_function && !NILP (Vmouse_position_function))
     retval = call1 (Vmouse_position_function, retval);
   return retval;
 }
diff --git a/src/frame.h b/src/frame.h
index 476bac67fa..002ee6ebb6 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1361,6 +1361,7 @@ window_system_available (struct frame *f)
 extern void adjust_frame_size (struct frame *, int, int, int, bool, Lisp_Object);
 extern void frame_size_history_add (struct frame *f, Lisp_Object fun_symbol,
 				    int width, int height, Lisp_Object rest);
+extern Lisp_Object mouse_position (bool call_mouse_position_function);
 
 extern Lisp_Object Vframe_list;
 
diff --git a/src/term.c b/src/term.c
index 3677644845..9a3b4857f8 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2804,16 +2804,15 @@ tty_menu_calc_size (tty_menu *menu, int *width, int *height)
 static void
 mouse_get_xy (int *x, int *y)
 {
-  struct frame *sf = SELECTED_FRAME ();
-  Lisp_Object lmx = Qnil, lmy = Qnil, lisp_dummy;
-  enum scroll_bar_part part_dummy;
-  Time time_dummy;
-
-  if (FRAME_TERMINAL (sf)->mouse_position_hook)
-    (*FRAME_TERMINAL (sf)->mouse_position_hook) (&sf, -1,
-                                                 &lisp_dummy, &part_dummy,
-						 &lmx, &lmy,
-						 &time_dummy);
+  Lisp_Object lmx = Qnil, lmy = Qnil;
+  Lisp_Object mouse = mouse_position (tty_menu_calls_mouse_position_function);
+
+  if (EQ (selected_frame, XCAR(mouse)))
+    {
+      lmx = XCAR (XCDR (mouse));
+      lmy = XCDR (XCDR (mouse));
+    }
+
   if (!NILP (lmx))
     {
       *x = XFIXNUM (lmx);
@@ -4552,6 +4551,13 @@ syms_of_term (void)
 bigger, or it may make it blink, or it may do nothing at all.  */);
   visible_cursor = 1;
 
+  DEFVAR_BOOL ("tty-menu-calls-mouse-position-function",
+               tty_menu_calls_mouse_position_function,
+    doc: /* Non-nil means that TTY menus will call `mouse-position-function'.
+This should be set if the function set does just straight logic and does not
+trigger redisplay.  */);
+  tty_menu_calls_mouse_position_function = 0;
+
   defsubr (&Stty_display_color_p);
   defsubr (&Stty_display_color_cells);
   defsubr (&Stty_no_underline);
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Enabling-TTY-menus-with-xterm-mouse-mode.patch --]
[-- Type: text/x-diff; name=0003-Enabling-TTY-menus-with-xterm-mouse-mode.patch, Size: 2017 bytes --]

From d1964f70df253ace00f9fbc038f6aacca05bd1ce Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Tue, 6 Oct 2020 20:04:12 -0700
Subject: [PATCH 3/3] Enabling TTY menus with xterm-mouse-mode.

---
 etc/NEWS         | 11 +++++++++++
 lisp/menu-bar.el |  2 ++
 lisp/tmm.el      |  1 -
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 7a776b3d56..e38bd913f8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1211,6 +1211,17 @@ never be narrower than 19 characters.
 When the bookmark.el library is loaded, a customize choice is added
 to 'tab-bar-new-tab-choice' for new tabs to show the bookmark list.
 
+** xterm-mouse mode
+
+---
+*** TTY menu navigation is now supported in 'xterm-mouse-mode'.
+
+TTY menus support mouse navigation and selection when xterm-mouse-mode
+is active. When run in a terminal, clicking on the menu bar with the
+mouse now pops up a TTY menu by default instead of running the command
+'tmm-menubar'. To restore the old behavior, set the variable
+'tty-menu-open-use-tmm' to non-nil.
+
 ** xwidget-webkit mode
 
 *** New xwidget commands.
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 518d0cc024..4385a09542 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2085,6 +2085,8 @@ global-map
 (bindings--define-key global-map [menu-bar help-menu]
   (cons (purecopy "Help") menu-bar-help-menu))
 
+(define-key global-map [menu-bar mouse-1] 'menu-bar-open-mouse)
+
 (defun menu-bar-menu-frame-live-and-visible-p ()
   "Return non-nil if the menu frame is alive and visible.
 The menu frame is the frame for which we are updating the menu."
diff --git a/lisp/tmm.el b/lisp/tmm.el
index fc02fd5790..4c2855751c 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -43,7 +43,6 @@ tmm-next-shortcut-digit
 (defvar tmm-table-undef)
 
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
-;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
 
 ;;;###autoload
 (defun tmm-menubar (&optional x-position)
-- 
2.20.1

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Thu, 08 Oct 2020 22:17:41 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> Thanks for the detailed reasoning. I have updated my patches to only 
> conditionally call mouse-position-function, as requested.

Thanks.

> I've also added one additional patch that rebinds [menu-bar mouse-1]
> to get TTY menus by default.

I'm not sure I understand the need.  E.g., the mouse-equipped w32
console doesn't need this, and neither should the GPM mouse.  Can you
explain?  Does xterm-mouse alone need it?  Also, could this affect the
GUI frames in any way?

A few minor comments below, and then we will wait for your legal
paperwork to come through.

> +(defun menu-bar-open-mouse (event)
> +  "Mosue-triggered version of `menu-bar-open'.
      ^^^^^
"Mouse".  And the first line of any doc string should be a summary of
what the function does, and should reference the arguments.

> +This command is to be used when you click the mouse in the menubar."
> +  (interactive "e")
> +  (let* ((x-position (car (posn-x-y (event-start event))))
> +         (menu-bar-item-cons (menu-bar-item-at-x x-position)))
> +    (menu-bar-open nil
> +                   (if menu-bar-item-cons
> +                       (cdr menu-bar-item-cons)
> +                     0))))
> +
> +(defun menu-bar-keymap ()
> +  "Return the current menu-bar keymap.
> +
> +The ordering of the return value respects `menu-bar-final-items'."
> +  (let ((menu-bar '())
> +        (menu-end '()))
> +    (map-keymap
> +     (lambda (key binding)
> +       (let ((pos (seq-position menu-bar-final-items key))
> +             (menu-item (cons key binding)))
> +         (if pos
> +             ;; If KEY is the name of an item that we want to put
> +             ;; last, store it separately with explicit ordering for
> +             ;; sorting.
> +             (push (cons pos menu-item) menu-end)
> +           (push menu-item menu-bar))))
> +     (menu-bar-get-binding [menu-bar]))
> +    `(keymap ,@(nreverse menu-bar)
> +             ,@(mapcar #'cdr (sort menu-end
> +                                   (lambda (a b)
> +                                     (< (car a) (car b))))))))
> +
> +(defun menu-bar-get-binding (keyseq)
> +  "Return the current binding of KEYSEQ, merging prefix definitions.

This should somehow mention the menu bar, otherwise it's too general.

> +However, for the menu bar itself, the value does not take account
> +of `menu-bar-final-items'."           ^^^^^^^^^^^^^^^^^^^^^^^^^^^

"take into account" instead of "take account of", I presume?

> +(defun menu-bar-item-at-x (x-position)
> +  "Return a cons of the form (KEY . X) for the item clicked on.

Can we say that X here is the X coordinate of the click?

> ++++
> +** New variable 'tty-menu-calls-mouse-position-function'.
> +This controls if TTY menu logic calls mouse-position-function. Libraries that
                                                                ^^
In documentation, doc strings, and comments we use US English
convention of leaving 2 spaces between sentences.

> @@ -321,7 +325,8 @@ xterm-mouse-mode
>    (if xterm-mouse-mode
>        ;; Turn it on
>        (progn
> -	(setq mouse-position-function #'xterm-mouse-position-function)
> +        (setq mouse-position-function #'xterm-mouse-position-function
> +              tty-menu-calls-mouse-position-function t)

Can we bind tty-menu-calls-mouse-position-function only around the
code that uses the menus?  If that's not feasible, we should at least
reset it to nil when the mode is switched off.

> +  if (EQ (selected_frame, XCAR(mouse)))
                                 ^
Our style is to leave one space between the function/macro name and
the opening parenthesis that follows.

> +TTY menus support mouse navigation and selection when xterm-mouse-mode
> +is active. When run in a terminal, clicking on the menu bar with the
> +mouse now pops up a TTY menu by default instead of running the command
> +'tmm-menubar'. To restore the old behavior, set the variable
> +'tty-menu-open-use-tmm' to non-nil.

Same issue with 2 spaces between sentences.

Thanks again for working on this.

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

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

On 2020-10-09 8:02 am, Eli Zaretskii wrote:
>> Date: Thu, 08 Oct 2020 22:17:41 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> I've also added one additional patch that rebinds [menu-bar mouse-1]
>> to get TTY menus by default.
> 
> I'm not sure I understand the need.  E.g., the mouse-equipped w32
> console doesn't need this, and neither should the GPM mouse.  Can you
> explain?  Does xterm-mouse alone need it?  Also, could this affect the
> GUI frames in any way?

xterm-mouse alone needs it. [menu-bar mouse-1] was already bound before 
my changes, so I believe it is safe. It was previously bound to 
tmm-menubar-mouse. This change makes it use TTY menus or tmm menus based 
on tty-menu-open-use-tmm, like menu-bar-open.

>> +(defun menu-bar-get-binding (keyseq)
>> +  "Return the current binding of KEYSEQ, merging prefix definitions.
> 
> This should somehow mention the menu bar, otherwise it's too general.

(For context: This is the docstring unchanged from when this function 
was in tmm.el. That said, I'm happy to improve it.)

I made a different change here to address. The binding lookup wasn't the 
interesting part, it was the reversing of the order of 
current-active-maps. I pulled the lookup to the caller and renamed the 
function entirely.

>> @@ -321,7 +325,8 @@ xterm-mouse-mode
>>    (if xterm-mouse-mode
>>        ;; Turn it on
>>        (progn
>> -	(setq mouse-position-function #'xterm-mouse-position-function)
>> +        (setq mouse-position-function #'xterm-mouse-position-function
>> +              tty-menu-calls-mouse-position-function t)
> 
> Can we bind tty-menu-calls-mouse-position-function only around the
> code that uses the menus?  If that's not feasible, we should at least
> reset it to nil when the mode is switched off.

I did the latter. It is not feasible as the menu is opened via 
menu-bar-open-mouse, which should not have knowledge about xterm-mouse 
specifically.


All other items also addressed. Updated patches attached.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adding-mouse-controls-to-menu-bar.el.patch --]
[-- Type: text/x-diff; name=0001-Adding-mouse-controls-to-menu-bar.el.patch, Size: 7982 bytes --]

From 0f9c4ac21a92a368162cd134252045b6c3362433 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 19 Sep 2020 00:43:29 -0700
Subject: [PATCH 1/3] Adding mouse controls to menu-bar.el.

---
 lisp/isearch.el  |  3 +-
 lisp/menu-bar.el | 80 ++++++++++++++++++++++++++++++++++++++++++++++++
 lisp/tmm.el      | 63 ++++----------------------------------
 3 files changed, 87 insertions(+), 59 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 781a8c5a93..01d1509a36 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -54,7 +54,6 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl-lib))
-(declare-function tmm-menubar-keymap "tmm.el")
 \f
 ;; Some additional options and constants.
 
@@ -501,7 +500,7 @@ isearch-tmm-menubar
   (require 'tmm)
   (run-hooks 'menu-bar-update-hook)
   (let ((command nil))
-    (let ((menu-bar (tmm-menubar-keymap)))
+    (let ((menu-bar (menu-bar-keymap)))
       (with-isearch-suspended
        (setq command (let ((isearch-mode t)) ; Show bindings from
                                              ; `isearch-mode-map' in
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 1b3e102cfa..8fad8f36bb 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2660,6 +2660,86 @@ menu-bar-open
 
 (global-set-key [f10] 'menu-bar-open)
 
+(defun menu-bar-open-mouse (event)
+  "Open the menu bar for the menu item clicked on by the mouse.
+EVENT should be a mouse down or click event.
+
+Also see `menu-bar-open', which this calls.
+This command is to be used when you click the mouse in the menubar."
+  (interactive "e")
+  (let* ((x-position (car (posn-x-y (event-start event))))
+         (menu-bar-item-cons (menu-bar-item-at-x x-position)))
+    (menu-bar-open nil
+                   (if menu-bar-item-cons
+                       (cdr menu-bar-item-cons)
+                     0))))
+
+(defun menu-bar-keymap ()
+  "Return the current menu-bar keymap.
+
+The ordering of the return value respects `menu-bar-final-items'."
+  (let ((menu-bar '())
+        (menu-end '()))
+    (map-keymap
+     (lambda (key binding)
+       (let ((pos (seq-position menu-bar-final-items key))
+             (menu-item (cons key binding)))
+         (if pos
+             ;; If KEY is the name of an item that we want to put
+             ;; last, store it separately with explicit ordering for
+             ;; sorting.
+             (push (cons pos menu-item) menu-end)
+           (push menu-item menu-bar))))
+     (lookup-key (menu-bar-current-active-maps) [menu-bar]))
+    `(keymap ,@(nreverse menu-bar)
+             ,@(mapcar #'cdr (sort menu-end
+                                   (lambda (a b)
+                                     (< (car a) (car b))))))))
+
+(defun menu-bar-current-active-maps ()
+  "Return the current active maps in the order the menu bar displays them.
+This value does not take into account `menu-bar-final-items' as that applies
+per-item."
+  ;; current-active-maps returns maps in the order local then
+  ;; global. The menu bar displays items in the opposite order.
+  (cons 'keymap (nreverse (current-active-maps))))
+
+(defun menu-bar-item-at-x (x-position)
+  "Return a cons of the form (KEY . X) for a menu item.
+The returned X is the left X coordinate for that menu item.
+
+X-POSITION is the X coordinate being queried.  If nothing is clicked on,
+returns nil."
+  (let ((column 0)
+        (menu-bar (menu-bar-keymap))
+        prev-key
+        prev-column
+        found)
+    (catch 'done
+      (map-keymap
+       (lambda (key binding)
+         (when (> column x-position)
+           (setq found t)
+           (throw 'done nil))
+         (setq prev-key key)
+         (pcase binding
+           ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
+                `(menu-item ,name ,_cmd            ;Extended menu item.
+                            . ,(and props
+                                    (guard (let ((visible
+                                                  (plist-get props :visible)))
+                                             (or (null visible)
+                                                 (eval visible)))))))
+            (setq prev-column column
+                  column (+ column (length name) 1)))))
+       menu-bar)
+      ;; Check the last menu item.
+      (when (> column x-position)
+        (setq found t)))
+    (if found
+        (cons prev-key prev-column)
+      nil)))
+
 (defun buffer-menu-open ()
   "Start key navigation of the buffer menu.
 This is the keyboard interface to \\[mouse-buffer-menu]."
diff --git a/lisp/tmm.el b/lisp/tmm.el
index 0e83f427f5..fc02fd5790 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -42,28 +42,6 @@ tmm-km-list
 (defvar tmm-next-shortcut-digit)
 (defvar tmm-table-undef)
 
-(defun tmm-menubar-keymap ()
-  "Return the current menu-bar keymap.
-
-The ordering of the return value respects `menu-bar-final-items'."
-  (let ((menu-bar '())
-        (menu-end '()))
-    (map-keymap
-     (lambda (key binding)
-       (let ((pos (seq-position menu-bar-final-items key))
-             (menu-item (cons key binding)))
-         (if pos
-             ;; If KEY is the name of an item that we want to put
-             ;; last, store it separately with explicit ordering for
-             ;; sorting.
-             (push (cons pos menu-item) menu-end)
-           (push menu-item menu-bar))))
-     (tmm-get-keybind [menu-bar]))
-    `(keymap ,@(nreverse menu-bar)
-             ,@(mapcar #'cdr (sort menu-end
-                                   (lambda (a b)
-                                     (< (car a) (car b))))))))
-
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
 ;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
 
@@ -79,33 +57,12 @@ tmm-menubar
 `tty-menu-open-use-tmm' to a non-nil value."
   (interactive)
   (run-hooks 'menu-bar-update-hook)
-  ;; Obey menu-bar-final-items; put those items last.
-  (let ((menu-bar (tmm-menubar-keymap))
-	menu-bar-item)
-    (if x-position
-	(let ((column 0)
-              prev-key)
-          (catch 'done
-            (map-keymap
-             (lambda (key binding)
-               (when (> column x-position)
-                 (setq menu-bar-item prev-key)
-                 (throw 'done nil))
-               (setq prev-key key)
-               (pcase binding
-                 ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
-                      `(menu-item ,name ,_cmd            ;Extended menu item.
-                        . ,(and props
-                                (guard (let ((visible
-                                              (plist-get props :visible)))
-                                         (or (null visible)
-                                             (eval visible)))))))
-                  (setq column (+ column (length name) 1)))))
-             menu-bar)
-            ;; Check the last menu item.
-            (when (> column x-position)
-              (setq menu-bar-item prev-key)))))
-    (tmm-prompt menu-bar nil menu-bar-item)))
+  (let ((menu-bar (menu-bar-keymap))
+        (menu-bar-item-cons (and x-position
+                                 (menu-bar-item-at-x x-position))))
+    (tmm-prompt menu-bar
+                nil
+                (and menu-bar-item-cons (car menu-bar-item-cons)))))
 
 ;;;###autoload
 (defun tmm-menubar-mouse (event)
@@ -525,14 +482,6 @@ tmm-get-keymap
 	   (or (assoc str tmm-km-list)
 	       (push (cons str (cons event km)) tmm-km-list))))))
 
-(defun tmm-get-keybind (keyseq)
-  "Return the current binding of KEYSEQ, merging prefix definitions.
-If KEYSEQ is a prefix key that has local and global bindings,
-we merge them into a single keymap which shows the proper order of the menu.
-However, for the menu bar itself, the value does not take account
-of `menu-bar-final-items'."
-  (lookup-key (cons 'keymap (nreverse (current-active-maps))) keyseq))
-
 (provide 'tmm)
 
 ;;; tmm.el ends here
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch --]
[-- Type: text/x-diff; name=0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch, Size: 13450 bytes --]

From b43a1ba2a8c6b05ab35963993be59f307d7bba90 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 3 Oct 2020 14:46:30 -0700
Subject: [PATCH 2/3] Making TTY menus work with xterm-mouse-mode.

* xt-mouse.el uses SET_ANY_EVENT_MOUSE (1003) so mouse movement can be
  reported even if no buttons are pressed.
* xt-mouse.el now respects track-mouse. It previously did not need to
  since mouse movement was only reported if a mouse button was pressed.
* Conditionally hook up mouse_get_xy to mouse-position-function so it
  works with xterm mouse. This is controlled by a new variable,
  tty-menu-calls-mouse-position-function.
* tty-navigation-map handles all the different prefixes mouse events can
  have, such as menu-bar or mode-line.
---
 doc/lispref/frames.texi |  7 +++++
 etc/NEWS                |  5 ++++
 lisp/menu-bar.el        | 64 ++++++++++++++++++++++-------------------
 lisp/xt-mouse.el        | 26 ++++++++++-------
 src/frame.c             |  8 +++++-
 src/frame.h             |  1 +
 src/term.c              | 26 ++++++++++-------
 7 files changed, 86 insertions(+), 51 deletions(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index 22d32c00d9..6aee077df8 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -3526,6 +3526,13 @@ Mouse Position
 @file{xt-mouse.el} that need to do mouse handling at the Lisp level.
 @end defvar
 
+@defvar tty-menu-calls-mouse-position-function
+If non-@code{nil}, TTY menus will call @code{mouse-position-function}
+as described above. This exists for cases where
+@code{mouse-position-function} is not safe to be called by the TTY
+menus, such as if it would trigger redisplay.
+@end defvar
+
 @defun set-mouse-position frame x y
 This function @dfn{warps the mouse} to position @var{x}, @var{y} in
 frame @var{frame}.  The arguments @var{x} and @var{y} are integers,
diff --git a/etc/NEWS b/etc/NEWS
index d05b706ea3..3a324fcf7f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1621,6 +1621,11 @@ convert them to a list '(R G B)' of primary color values.
 This user option can be one of the predefined styles or a function to
 personalize the uniquified buffer name.
 
++++
+** New variable 'tty-menu-calls-mouse-position-function'.
+This controls if TTY menu logic calls mouse-position-function.  Libraries that
+set mouse-position-function should also set this to non-nil if they can confirm
+they are compatible with the tty menu logic.
 
 \f
 * Changes in Emacs 28.1 on Non-Free Operating Systems
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 8fad8f36bb..c1125d2895 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2759,6 +2759,16 @@ mouse-buffer-menu-keymap
                 (menu-bar-buffer-vector item)))))
     km))
 
+(defun menu-bar-define-mouse-key (map key def)
+  "Like `define-key', but adds all possible prefixes for the mouse."
+  (define-key map (vector key) def)
+  (mapc (lambda (prefix) (define-key map (vector prefix key) def))
+        ;; This list only needs to contain special window areas that
+        ;; are rendered in TTYs. No need for *-scroll-bar, *-fringe, or
+        ;; *-divider.
+        '(tab-line header-line menu-bar tab-bar mode-line vertical-line
+          left-margin right-margin)))
+
 (defvar tty-menu-navigation-map
   (let ((map (make-sparse-keymap)))
     ;; The next line is disabled because it breaks interpretation of
@@ -2793,39 +2803,33 @@ tty-menu-navigation-map
     (define-key map [?\C-j] 'tty-menu-select)
     (define-key map [return] 'tty-menu-select)
     (define-key map [linefeed] 'tty-menu-select)
-    (define-key map [mouse-1] 'tty-menu-select)
-    (define-key map [drag-mouse-1] 'tty-menu-select)
-    (define-key map [mouse-2] 'tty-menu-select)
-    (define-key map [drag-mouse-2] 'tty-menu-select)
-    (define-key map [mouse-3] 'tty-menu-select)
-    (define-key map [drag-mouse-3] 'tty-menu-select)
-    (define-key map [wheel-down] 'tty-menu-next-item)
-    (define-key map [wheel-up] 'tty-menu-prev-item)
-    (define-key map [wheel-left] 'tty-menu-prev-menu)
-    (define-key map [wheel-right] 'tty-menu-next-menu)
-    ;; The following 4 bindings are for those whose text-mode mouse
+    (menu-bar-define-mouse-key map 'mouse-1 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-1 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'mouse-2 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-2 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'mouse-3 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'drag-mouse-3 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'wheel-down 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'wheel-up 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'wheel-left 'tty-menu-prev-menu)
+    (menu-bar-define-mouse-key map 'wheel-right 'tty-menu-next-menu)
+    ;; The following 6 bindings are for those whose text-mode mouse
     ;; lack the wheel.
-    (define-key map [S-mouse-1] 'tty-menu-next-item)
-    (define-key map [S-drag-mouse-1] 'tty-menu-next-item)
-    (define-key map [S-mouse-2] 'tty-menu-prev-item)
-    (define-key map [S-drag-mouse-2] 'tty-menu-prev-item)
-    (define-key map [S-mouse-3] 'tty-menu-prev-item)
-    (define-key map [S-drag-mouse-3] 'tty-menu-prev-item)
-    (define-key map [header-line mouse-1] 'tty-menu-select)
-    (define-key map [header-line drag-mouse-1] 'tty-menu-select)
+    (menu-bar-define-mouse-key map 'S-mouse-1 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-1 'tty-menu-next-item)
+    (menu-bar-define-mouse-key map 'S-mouse-2 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-2 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-mouse-3 'tty-menu-prev-item)
+    (menu-bar-define-mouse-key map 'S-drag-mouse-3 'tty-menu-prev-item)
     ;; The down-mouse events must be bound to tty-menu-ignore, so that
     ;; only releasing the mouse button pops up the menu.
-    (define-key map [mode-line down-mouse-1] 'tty-menu-ignore)
-    (define-key map [mode-line down-mouse-2] 'tty-menu-ignore)
-    (define-key map [mode-line down-mouse-3] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-1] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-2] 'tty-menu-ignore)
-    (define-key map [mode-line C-down-mouse-3] 'tty-menu-ignore)
-    (define-key map [down-mouse-1] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-1] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-2] 'tty-menu-ignore)
-    (define-key map [C-down-mouse-3] 'tty-menu-ignore)
-    (define-key map [mouse-movement] 'tty-menu-mouse-movement)
+    (menu-bar-define-mouse-key map 'down-mouse-1 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'down-mouse-2 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'down-mouse-3 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-1 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-2 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'C-down-mouse-3 'tty-menu-ignore)
+    (menu-bar-define-mouse-key map 'mouse-movement 'tty-menu-mouse-movement)
     map)
   "Keymap used while processing TTY menus.")
 
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 362d29b943..e838219960 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -76,7 +76,11 @@ xterm-mouse-translate-1
               ;; to guard against that.
               (copy-sequence event))
 	vec)
-       (is-move vec)
+       (is-move
+        (if track-mouse vec
+          ;; Mouse movement events are currently supposed to be
+          ;; suppressed. Return no event.
+          []))
        (t
 	(let* ((down (terminal-parameter nil 'xterm-mouse-last-down))
 	       (down-data (nth 1 down))
@@ -321,11 +325,13 @@ xterm-mouse-mode
   (if xterm-mouse-mode
       ;; Turn it on
       (progn
-	(setq mouse-position-function #'xterm-mouse-position-function)
+        (setq mouse-position-function #'xterm-mouse-position-function
+              tty-menu-calls-mouse-position-function t)
         (mapc #'turn-on-xterm-mouse-tracking-on-terminal (terminal-list)))
     ;; Turn it off
     (mapc #'turn-off-xterm-mouse-tracking-on-terminal (terminal-list))
-    (setq mouse-position-function nil)))
+    (setq mouse-position-function nil
+          tty-menu-calls-mouse-position-function nil)))
 
 (defun xterm-mouse-tracking-enable-sequence ()
   "Return a control sequence to enable XTerm mouse tracking.
@@ -339,8 +345,8 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
-            motion events during dragging operations.
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an
             extension to the basic mouse mode, which uses UTF-8
@@ -360,7 +366,7 @@ xterm-mouse-tracking-enable-sequence
   (apply #'concat (xterm-mouse--tracking-sequence ?h)))
 
 (defconst xterm-mouse-tracking-enable-sequence
-  "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"
+  "\e[?1000h\e[?1003h\e[?1005h\e[?1006h"
   "Control sequence to enable xterm mouse tracking.
 Enables basic mouse tracking, mouse motion events and finally
 extended tracking on terminals that support it. The following
@@ -371,8 +377,8 @@ xterm-mouse-tracking-enable-sequence
             position (<= 223), which can be reported in this
             basic mode.
 
-\"\\e[?1002h\" \"Mouse motion mode\": Enables reports for mouse
-            motion events during dragging operations.
+\"\\e[?1003h\" \"Mouse motion mode\": Enables reports for mouse
+            motion events.
 
 \"\\e[?1005h\" \"UTF-8 coordinate extension\": Enables an extension
             to the basic mouse mode, which uses UTF-8
@@ -400,7 +406,7 @@ xterm-mouse-tracking-disable-sequence
   (apply #'concat (nreverse (xterm-mouse--tracking-sequence ?l))))
 
 (defconst xterm-mouse-tracking-disable-sequence
-  "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"
+  "\e[?1006l\e[?1005l\e[?1003l\e[?1000l"
   "Reset the modes set by `xterm-mouse-tracking-enable-sequence'.")
 
 (make-obsolete-variable
@@ -414,7 +420,7 @@ xterm-mouse--tracking-sequence
 enable, ?l to disable)."
   (mapcar
    (lambda (code) (format "\e[?%d%c" code suffix))
-   `(1000 1002 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
+   `(1000 1003 ,@(when xterm-mouse-utf-8 '(1005)) 1006)))
 
 (defun turn-on-xterm-mouse-tracking-on-terminal (&optional terminal)
   "Enable xterm mouse tracking on TERMINAL."
diff --git a/src/frame.c b/src/frame.c
index 0b707c2af8..5d967a59ce 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2433,6 +2433,12 @@ DEFUN ("mouse-position", Fmouse_position, Smouse_position, 0, 0, 0,
 passing the normal return value to that function as an argument,
 and returns whatever that function returns.  */)
   (void)
+{
+  return mouse_position (true);
+}
+
+Lisp_Object
+mouse_position (bool call_mouse_position_function)
 {
   struct frame *f;
   Lisp_Object lispy_dummy;
@@ -2462,7 +2468,7 @@ DEFUN ("mouse-position", Fmouse_position, Smouse_position, 0, 0, 0,
     }
   XSETFRAME (lispy_dummy, f);
   retval = Fcons (lispy_dummy, Fcons (x, y));
-  if (!NILP (Vmouse_position_function))
+  if (call_mouse_position_function && !NILP (Vmouse_position_function))
     retval = call1 (Vmouse_position_function, retval);
   return retval;
 }
diff --git a/src/frame.h b/src/frame.h
index 476bac67fa..002ee6ebb6 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1361,6 +1361,7 @@ window_system_available (struct frame *f)
 extern void adjust_frame_size (struct frame *, int, int, int, bool, Lisp_Object);
 extern void frame_size_history_add (struct frame *f, Lisp_Object fun_symbol,
 				    int width, int height, Lisp_Object rest);
+extern Lisp_Object mouse_position (bool call_mouse_position_function);
 
 extern Lisp_Object Vframe_list;
 
diff --git a/src/term.c b/src/term.c
index 3677644845..e8b22a87ac 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2804,16 +2804,15 @@ tty_menu_calc_size (tty_menu *menu, int *width, int *height)
 static void
 mouse_get_xy (int *x, int *y)
 {
-  struct frame *sf = SELECTED_FRAME ();
-  Lisp_Object lmx = Qnil, lmy = Qnil, lisp_dummy;
-  enum scroll_bar_part part_dummy;
-  Time time_dummy;
-
-  if (FRAME_TERMINAL (sf)->mouse_position_hook)
-    (*FRAME_TERMINAL (sf)->mouse_position_hook) (&sf, -1,
-                                                 &lisp_dummy, &part_dummy,
-						 &lmx, &lmy,
-						 &time_dummy);
+  Lisp_Object lmx = Qnil, lmy = Qnil;
+  Lisp_Object mouse = mouse_position (tty_menu_calls_mouse_position_function);
+
+  if (EQ (selected_frame, XCAR (mouse)))
+    {
+      lmx = XCAR (XCDR (mouse));
+      lmy = XCDR (XCDR (mouse));
+    }
+
   if (!NILP (lmx))
     {
       *x = XFIXNUM (lmx);
@@ -4552,6 +4551,13 @@ syms_of_term (void)
 bigger, or it may make it blink, or it may do nothing at all.  */);
   visible_cursor = 1;
 
+  DEFVAR_BOOL ("tty-menu-calls-mouse-position-function",
+               tty_menu_calls_mouse_position_function,
+    doc: /* Non-nil means that TTY menus will call `mouse-position-function'.
+This should be set if the function set does just straight logic and does not
+trigger redisplay.  */);
+  tty_menu_calls_mouse_position_function = 0;
+
   defsubr (&Stty_display_color_p);
   defsubr (&Stty_display_color_cells);
   defsubr (&Stty_no_underline);
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Enabling-TTY-menus-with-xterm-mouse-mode.patch --]
[-- Type: text/x-diff; name=0003-Enabling-TTY-menus-with-xterm-mouse-mode.patch, Size: 2019 bytes --]

From 58966cded88dffd1020a793a98f32da95622638b Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Tue, 6 Oct 2020 20:04:12 -0700
Subject: [PATCH 3/3] Enabling TTY menus with xterm-mouse-mode.

---
 etc/NEWS         | 11 +++++++++++
 lisp/menu-bar.el |  2 ++
 lisp/tmm.el      |  1 -
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 3a324fcf7f..4890c8ffa0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1211,6 +1211,17 @@ never be narrower than 19 characters.
 When the bookmark.el library is loaded, a customize choice is added
 to 'tab-bar-new-tab-choice' for new tabs to show the bookmark list.
 
+** xterm-mouse mode
+
+---
+*** TTY menu navigation is now supported in 'xterm-mouse-mode'.
+
+TTY menus support mouse navigation and selection when xterm-mouse-mode
+is active.  When run in a terminal, clicking on the menu bar with the
+mouse now pops up a TTY menu by default instead of running the command
+'tmm-menubar'.  To restore the old behavior, set the variable
+'tty-menu-open-use-tmm' to non-nil.
+
 ** xwidget-webkit mode
 
 *** New xwidget commands.
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index c1125d2895..22fae028d3 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2085,6 +2085,8 @@ global-map
 (bindings--define-key global-map [menu-bar help-menu]
   (cons (purecopy "Help") menu-bar-help-menu))
 
+(define-key global-map [menu-bar mouse-1] 'menu-bar-open-mouse)
+
 (defun menu-bar-menu-frame-live-and-visible-p ()
   "Return non-nil if the menu frame is alive and visible.
 The menu frame is the frame for which we are updating the menu."
diff --git a/lisp/tmm.el b/lisp/tmm.el
index fc02fd5790..4c2855751c 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -43,7 +43,6 @@ tmm-next-shortcut-digit
 (defvar tmm-table-undef)
 
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
-;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
 
 ;;;###autoload
 (defun tmm-menubar (&optional x-position)
-- 
2.20.1

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Fri, 09 Oct 2020 22:20:36 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> All other items also addressed. Updated patches attached.

Thanks.  We are now waiting for your legal paperwork to complete.

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

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

On 2020-10-10 12:28 am, Eli Zaretskii wrote:
>> Date: Fri, 09 Oct 2020 22:20:36 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> All other items also addressed. Updated patches attached.
> 
> Thanks.  We are now waiting for your legal paperwork to complete.

While working on the next feature I wanted to get working with 
xterm-mouse (help-echo and mouse-face text properties), I noticed a bug. 
Attached is a patch to apply after the rest of the changes to fix the 
issue.

This bug *predates* my changes, but this fix relies on my changes. 
Specifically, it relies on mouse-position being able to call 
mouse-position-function.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fixing-bug-where-the-wrong-menu-would-be-triggered-b.patch --]
[-- Type: text/x-diff; name=0001-Fixing-bug-where-the-wrong-menu-would-be-triggered-b.patch, Size: 2021 bytes --]

From 6776d6899ba029f45ad76b8bb2a9f3dcd1fcac52 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sun, 11 Oct 2020 20:16:00 -0700
Subject: [PATCH] Fixing bug where the wrong menu would be triggered by mouse.

For layouts such as the following, clicking the "l" in Tools with the
right window focused would trigger the File menu, not the Tools menu.
This is because the event would have window coordinate (1 . 0).
Similarly, clicking the "p" in Help would trigger the Edit menu.

Example Emacs frame:
+--------------------------------------------------------+
|File Edit Options Buffers Tools Help                    |
|;; This buffer is for text$|;; This buffer is for text $|
|;; To create a file, visit$|;; To create a file, visit $|
|                           |                            |
|                           |                            |
|-UUU:----F1  *scratch*     |-UUU:----F1  *scratch*      |
|                                                        |
+--------------------------------------------------------+
---
 lisp/menu-bar.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 22fae028d3..79689f8cc2 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2662,14 +2662,14 @@ menu-bar-open
 
 (global-set-key [f10] 'menu-bar-open)
 
-(defun menu-bar-open-mouse (event)
+(defun menu-bar-open-mouse (position)
   "Open the menu bar for the menu item clicked on by the mouse.
-EVENT should be a mouse down or click event.
+POSITION should be a list of the form returned by `mouse-position'.
 
 Also see `menu-bar-open', which this calls.
 This command is to be used when you click the mouse in the menubar."
-  (interactive "e")
-  (let* ((x-position (car (posn-x-y (event-start event))))
+  (interactive (list (mouse-position)))
+  (let* ((x-position (cadr position))
          (menu-bar-item-cons (menu-bar-item-at-x x-position)))
     (menu-bar-open nil
                    (if menu-bar-item-cons
-- 
2.20.1

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sun, 11 Oct 2020 20:25:13 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> While working on the next feature I wanted to get working with 
> xterm-mouse (help-echo and mouse-face text properties), I noticed a bug. 
> Attached is a patch to apply after the rest of the changes to fix the 
> issue.

Thanks.

> -(defun menu-bar-open-mouse (event)
> +(defun menu-bar-open-mouse (position)
>    "Open the menu bar for the menu item clicked on by the mouse.
> -EVENT should be a mouse down or click event.
> +POSITION should be a list of the form returned by `mouse-position'.
>  
>  Also see `menu-bar-open', which this calls.
>  This command is to be used when you click the mouse in the menubar."
> -  (interactive "e")
> -  (let* ((x-position (car (posn-x-y (event-start event))))
> +  (interactive (list (mouse-position)))
> +  (let* ((x-position (cadr position))

I'd prefer not to lose the "e" interactive spec and the form of the
argument here.  If the problem is the conversion of window-relative to
frame-relative coordinates, that is easy, and the ELisp manual has an
example of how to do that in the node "Accessing Mouse".

Or is there some problem to use this here?

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

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

On 2020-10-12 7:45 am, Eli Zaretskii wrote:
>> Date: Sun, 11 Oct 2020 20:25:13 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> -(defun menu-bar-open-mouse (event)
>> +(defun menu-bar-open-mouse (position)
>>    "Open the menu bar for the menu item clicked on by the mouse.
>> -EVENT should be a mouse down or click event.
>> +POSITION should be a list of the form returned by `mouse-position'.
>> 
>>  Also see `menu-bar-open', which this calls.
>>  This command is to be used when you click the mouse in the menubar."
>> -  (interactive "e")
>> -  (let* ((x-position (car (posn-x-y (event-start event))))
>> +  (interactive (list (mouse-position)))
>> +  (let* ((x-position (cadr position))
> 
> I'd prefer not to lose the "e" interactive spec and the form of the
> argument here.  If the problem is the conversion of window-relative to
> frame-relative coordinates, that is easy, and the ELisp manual has an
> example of how to do that in the node "Accessing Mouse".
> 
> Or is there some problem to use this here?

Thanks, I investigated further and I have an improved patch attached.  
In addition to keeping the same interactive spec, it also is logically 
independent of my changes to enable xterm-mouse based menu interaction.  
I also attached repro.el, which I used to help understand the behavior 
of xterm-mouse.

The manual is not clear of the format of a posn for clicks outside a 
window, such as on the menu bar or tab bar. From the behavior I see, 
posn-window will return nil and posn-x-y will return (x . y) in frame 
coordinates. I rely on that in this patch. If this is accurate, I can 
update the manual.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fixing-bug-where-the-wrong-menu-would-be-triggered-b.patch --]
[-- Type: text/x-diff; name=0001-Fixing-bug-where-the-wrong-menu-would-be-triggered-b.patch, Size: 2406 bytes --]

From c0ed41e742fe65573a14eaa02ebba9687b8cfc5b Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sun, 11 Oct 2020 20:16:00 -0700
Subject: [PATCH] Fixing bug where the wrong menu would be triggered by mouse.

For layouts such as the following, clicking the "l" in Tools with the
right window focused would trigger the File menu, not the Tools menu.
This is because the event would have window coordinate (1 . 0).
Similarly, clicking the "p" in Help would trigger the Edit menu.

Example Emacs frame:
+--------------------------------------------------------+
|File Edit Options Buffers Tools Help                    |
|;; This buffer is for text$|;; This buffer is for text $|
|;; To create a file, visit$|;; To create a file, visit $|
|                           |                            |
|                           |                            |
|-UUU:----F1  *scratch*     |-UUU:----F1  *scratch*      |
|                                                        |
+--------------------------------------------------------+
---
 lisp/menu-bar.el | 6 ++++++
 lisp/xt-mouse.el | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 22fae028d3..3df72ea0f0 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -2669,6 +2669,12 @@ menu-bar-open-mouse
 Also see `menu-bar-open', which this calls.
 This command is to be used when you click the mouse in the menubar."
   (interactive "e")
+  ;; This only should be bound to clicks on the menu-bar, outside of
+  ;; any window.
+  (let ((window (posn-window (event-start event))))
+    (when window
+      (error "Event is inside window %s" window)))
+
   (let* ((x-position (car (posn-x-y (event-start event))))
          (menu-bar-item-cons (menu-bar-item-at-x x-position)))
     (menu-bar-open nil
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index e838219960..be261cebab 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -267,7 +267,7 @@ xterm-mouse-event
                                                     (eq y 1)))
                                            'tab-bar
                                          'menu-bar))
-                             (nthcdr 2 (posn-at-x-y x y)))))
+                             (nthcdr 2 (posn-at-x-y x y (selected-frame))))))
              (event (list type posn)))
         (setcar (nthcdr 3 posn) timestamp)
 
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: repro.el --]
[-- Type: text/x-lisp; name=repro.el, Size: 1380 bytes --]

;; This file exists to help understand xterm-mouse's bug. It is *not*
;; dependent on xterm-mouse-mode being enabled.
;;
;; Try running (test-xterm-coord-x) both with and without the text
;; marked "From patch:" enabled.

(defun test-xterm-coord-x ()
  (let (l0 ; menu bar
	l1 ; first row
	)
    (dotimes (x (frame-width))
      (push (posn-x-y (xterm-coords-to-posn x 0)) l0)
      (push (posn-x-y (xterm-coords-to-posn x 1)) l1))
    (list (mapcar 'car (nreverse l0))
	  (mapcar 'car (nreverse l1)))))

(defun test-xterm-coord-w ()
  (let (l0 ; menu bar
	l1 ; first row
	)
    (dotimes (x (frame-width))
      (push (posn-window (xterm-coords-to-posn x 0)) l0)
      (push (posn-window (xterm-coords-to-posn x 1)) l1))
    (list (nreverse l0) (nreverse l1))))
	    

(defun xterm-coords-to-posn (x y)
  "posn creation parts of `xterm-mouse-event'."
  (let* ((w (window-at x y))
	 (ltrb (window-edges w))
	 (left (nth 0 ltrb))
	 (top (nth 1 ltrb))
	 (posn (if w
		   (posn-at-x-y (- x left) (- y top) w t)
                 (append (list nil (if (and tab-bar-mode
					    (or (not menu-bar-mode)
						;; The tab-bar is on the
						;; second row below menu-bar
						(eq y 1)))
				       'tab-bar
				     'menu-bar))
			 (nthcdr 2 (posn-at-x-y x y ;From patch: (selected-frame)
                                                ))))))
    (setcar (nthcdr 3 posn) -1)
    posn))

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Mon, 12 Oct 2020 14:30:38 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> Thanks, I investigated further and I have an improved patch attached.  
> In addition to keeping the same interactive spec, it also is logically 
> independent of my changes to enable xterm-mouse based menu interaction.  
> I also attached repro.el, which I used to help understand the behavior 
> of xterm-mouse.

Thanks, I think this is a much better variant.

> The manual is not clear of the format of a posn for clicks outside a 
> window, such as on the menu bar or tab bar. From the behavior I see, 
> posn-window will return nil and posn-x-y will return (x . y) in frame 
> coordinates. I rely on that in this patch. If this is accurate, I can 
> update the manual.

Are you sure posn-window cannot return a frame?  For example, on a
TTY, I get this:

  (posn-at-x-y 0 0 (selected-frame))
   => (#<frame F1 069b0190> nil (0 . 0) 0)

And (0,0) are coordinates in the frame's menu bar.

Does it make sense to install this patch on its own?  If so, we can
install it without waiting for the legal paperwork, the changes are
small enough.

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

On 2020-10-13 7:33 am, Eli Zaretskii wrote:
>> Date: Mon, 12 Oct 2020 14:30:38 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: emacs-devel@gnu.org
>> 
>> The manual is not clear of the format of a posn for clicks outside a
>> window, such as on the menu bar or tab bar. From the behavior I see,
>> posn-window will return nil and posn-x-y will return (x . y) in frame
>> coordinates. I rely on that in this patch. If this is accurate, I can
>> update the manual.
> 
> Are you sure posn-window cannot return a frame?  For example, on a
> TTY, I get this:
> 
>   (posn-at-x-y 0 0 (selected-frame))
>    => (#<frame F1 069b0190> nil (0 . 0) 0)
> 
> And (0,0) are coordinates in the frame's menu bar.

A ha! A bug! Behavior is different depending on options selected at 
configure time.

Behavior with ./configure --with-x=no:
(posn-at-x-y 0 0 (selected-frame))
=> (nil nil (0 . 0) 0)
(selected-frame)
=> #<frame F1 0x55be32685d38>

Behavior with ./configure --with-x=yes:
(posn-at-x-y 0 0 (selected-frame))
=> (#<frame F1 0x561fedbc6b98> nil (0 . 0) 0)
(selected-frame)
=> #<frame F1 0x561fedbc6b98>

This bug has been around a while, on Debian (Emacs 26.1), emacs-no-x vs 
emacs-gtk shows the same difference in behavior.

Do you have a preference for a fix? Some options:

#1) If I was designing the API from scratch, I'd expect something like 
the following:

A function posn-window returns a window or nil if the coordinate is 
outside of any window.
A function posn-frame returns a frame. Always succeeds, since posn 
coordinates are always inside a frame.

This is compatible with --with-x=no behavior.

#2) An alternative would be to return "most specific information 
available". Something like the following:

A function posn-window-or-frame, which returns a window if the 
coordinate is inside a window and returns a frame if the coordinate is 
outside a window.
A deprecated alias posn-window for the function above.

This is compatible with --with-x=yes behavior.

I found at one other behavior difference, if you pass in out of bounds 
coordinates (example: 1000 0 for a TTY), --with-x=no you get the value 
as if you passed in 0 0, but --with-x=yes assumes the coordinate is 
valid. There are probably other differences. I can do a pass over this 
function, I'd like to know the razor to use.

> Does it make sense to install this patch on its own?  If so, we can
> install it without waiting for the legal paperwork, the changes are
> small enough.

Yes, the issue existed prior to my changes when using TMM in a terminal.

   -- MJF

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Tue, 13 Oct 2020 18:59:01 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> > Are you sure posn-window cannot return a frame?  For example, on a
> > TTY, I get this:
> > 
> >   (posn-at-x-y 0 0 (selected-frame))
> >    => (#<frame F1 069b0190> nil (0 . 0) 0)
> > 
> > And (0,0) are coordinates in the frame's menu bar.
> 
> A ha! A bug! Behavior is different depending on options selected at 
> configure time.
> 
> Behavior with ./configure --with-x=no:
> (posn-at-x-y 0 0 (selected-frame))
> => (nil nil (0 . 0) 0)
> (selected-frame)
> => #<frame F1 0x55be32685d38>
> 
> Behavior with ./configure --with-x=yes:
> (posn-at-x-y 0 0 (selected-frame))
> => (#<frame F1 0x561fedbc6b98> nil (0 . 0) 0)
> (selected-frame)
> => #<frame F1 0x561fedbc6b98>
> 
> This bug has been around a while, on Debian (Emacs 26.1), emacs-no-x vs 
> emacs-gtk shows the same difference in behavior.

Right.

> Do you have a preference for a fix? Some options:
> [...]
> I found at one other behavior difference, if you pass in out of bounds 
> coordinates (example: 1000 0 for a TTY), --with-x=no you get the value 
> as if you passed in 0 0, but --with-x=yes assumes the coordinate is 
> valid. There are probably other differences. I can do a pass over this 
> function, I'd like to know the razor to use.

These all are due to the same problem: too early exclusion of code in
builds --without-x.  I think I fixed this on the emacs-27 branch.
(While at that, I also fixed the documentation by adding the
description of the POSITION in this case.)

I don't think it's right to change the meaning of the WINDOW part of a
click even at this point: the current behavior is very old, and the
fact it's called WINDOW doesn't mean it cannot be a frame in some
cases, assuming we document that.

> > Does it make sense to install this patch on its own?  If so, we can
> > install it without waiting for the legal paperwork, the changes are
> > small enough.
> 
> Yes, the issue existed prior to my changes when using TMM in a terminal.

OK, I will install that soon.  Thanks.

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Thu, 15 Oct 2020 16:34:39 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > > Does it make sense to install this patch on its own?  If so, we can
> > > install it without waiting for the legal paperwork, the changes are
> > > small enough.
> > 
> > Yes, the issue existed prior to my changes when using TMM in a terminal.
> 
> OK, I will install that soon.  Thanks.

Actually, I now see that the change is in a function that is part of
your extended support for xt-mouse, and so I don't see how we could
install this before that larger change.  Am I missing something?

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

On 2020-10-15 11:51 pm, Eli Zaretskii wrote:
>> Date: Thu, 15 Oct 2020 16:34:39 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > > Does it make sense to install this patch on its own?  If so, we can
>> > > install it without waiting for the legal paperwork, the changes are
>> > > small enough.
>> >
>> > Yes, the issue existed prior to my changes when using TMM in a terminal.
>> 
>> OK, I will install that soon.  Thanks.
> 
> Actually, I now see that the change is in a function that is part of
> your extended support for xt-mouse, and so I don't see how we could
> install this before that larger change.  Am I missing something?

Oh, I thought you would apply the only the patch to xt-mouse.el. The 
change to menu-bar.el is error checking for the new function and can be 
done separately.

It's also fine to wait, I think I'm in the tail-end of the legal 
paperwork process.

   -- MJF

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sat, 10 Oct 2020 10:28:33 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > Date: Fri, 09 Oct 2020 22:20:36 -0700
> > From: Jared Finder <jared@finder.org>
> > Cc: emacs-devel@gnu.org
> > 
> > All other items also addressed. Updated patches attached.
> 
> Thanks.  We are now waiting for your legal paperwork to complete.

Installed on the master branch, thanks.

Please in the future make sure each of your changes is accompanied
with a ChangeLog-style commit log message (see CONTRIBUTE for
details).  (I added the log messages for you this time.)

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Mon, 12 Oct 2020 14:30:38 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> Thanks, I investigated further and I have an improved patch attached.  
> In addition to keeping the same interactive spec, it also is logically 
> independent of my changes to enable xterm-mouse based menu interaction.  
> I also attached repro.el, which I used to help understand the behavior 
> of xterm-mouse.

Thanks, installed on the master branch.

> The manual is not clear of the format of a posn for clicks outside a 
> window, such as on the menu bar or tab bar. From the behavior I see, 
> posn-window will return nil and posn-x-y will return (x . y) in frame 
> coordinates. I rely on that in this patch. If this is accurate, I can 
> update the manual.

If the current manual is still not clear, please suggest a fix.

Re: Making TTY menus more visual

[Jared Finder via Emacs development discussions.]

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

On 2020-10-24 3:31 am, Eli Zaretskii wrote:
> 
> If the current manual is still not clear, please suggest a fix.

Two further patches, one for a further doc fix and one for fixing the 
tests that were reported broken off-thread.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-unit-tests-broken-by-changes-to-xt-mouse.el.patch --]
[-- Type: text/x-diff; name=0001-Fix-unit-tests-broken-by-changes-to-xt-mouse.el.patch, Size: 1726 bytes --]

From 907933f6ccdd6329f8a72299c84b72a20402d70d Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 24 Oct 2020 08:25:06 -0700
Subject: [PATCH 1/2] Fix unit tests broken by changes to xt-mouse.el

* test/lisp/xt-mouse-tests.el (xt-mouse-tracking-basic)
(xt-mouse-tracking-utf-8): Update expected escape sequence.
---
 test/lisp/xt-mouse-tests.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/lisp/xt-mouse-tests.el b/test/lisp/xt-mouse-tests.el
index 61bd759018..12840df13f 100644
--- a/test/lisp/xt-mouse-tests.el
+++ b/test/lisp/xt-mouse-tests.el
@@ -53,9 +53,9 @@ with-xterm-mouse-mode
 
 (ert-deftest xt-mouse-tracking-basic ()
   (should (equal (xterm-mouse-tracking-enable-sequence)
-                 "\e[?1000h\e[?1002h\e[?1006h"))
+                 "\e[?1000h\e[?1003h\e[?1006h"))
   (should (equal (xterm-mouse-tracking-disable-sequence)
-                 "\e[?1006l\e[?1002l\e[?1000l"))
+                 "\e[?1006l\e[?1003l\e[?1000l"))
   (with-xterm-mouse-mode
     (should xterm-mouse-mode)
     (should (terminal-parameter nil 'xterm-mouse-mode))
@@ -73,9 +73,9 @@ xt-mouse-tracking-basic
 (ert-deftest xt-mouse-tracking-utf-8 ()
   (let ((xterm-mouse-utf-8 t))
     (should (equal (xterm-mouse-tracking-enable-sequence)
-                   "\e[?1000h\e[?1002h\e[?1005h\e[?1006h"))
+                   "\e[?1000h\e[?1003h\e[?1005h\e[?1006h"))
     (should (equal (xterm-mouse-tracking-disable-sequence)
-                   "\e[?1006l\e[?1005l\e[?1002l\e[?1000l"))
+                   "\e[?1006l\e[?1005l\e[?1003l\e[?1000l"))
     (with-xterm-mouse-mode
       (should xterm-mouse-mode)
       (should (terminal-parameter nil 'xterm-mouse-mode))
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Updating-docs-with-all-special-window-prefix-keys.patch --]
[-- Type: text/x-diff; name=0002-Updating-docs-with-all-special-window-prefix-keys.patch, Size: 2421 bytes --]

From 988cd8e73d3d23308fa72e6b8660f5dd35ef00be Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sun, 4 Oct 2020 00:01:23 -0700
Subject: [PATCH 2/2] Updating docs with all special window prefix keys.

* doc/lispref/commands.texi (Key Sequence Input): Add documentation for
missing special window areas.  Explicitly call out window or frame.
---
 doc/lispref/commands.texi | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index 8959175def..85bd2c1ed9 100644
--- a/doc/lispref/commands.texi
+++ b/doc/lispref/commands.texi
@@ -2516,8 +2516,14 @@ Key Sequence Input
 @cindex @code{vertical-scroll-bar} prefix key
 @cindex @code{menu-bar} prefix key
 @cindex @code{tab-bar} prefix key
-@cindex mouse events, in special parts of frame
-When mouse events occur in special parts of a window, such as a mode
+@cindex @code{left-margin} prefix key
+@cindex @code{right-margin} prefix key
+@cindex @code{left-fringe} prefix key
+@cindex @code{right-fringe} prefix key
+@cindex @code{right-divider} prefix key
+@cindex @code{bottom-divider} prefix key
+@cindex mouse events, in special parts of window or frame
+When mouse events occur in special parts of a window or frame, such as a mode
 line or a scroll bar, the event type shows nothing special---it is the
 same symbol that would normally represent that combination of mouse
 button and modifier keys.  The information about the window part is kept
@@ -2525,9 +2531,11 @@ Key Sequence Input
 @code{read-key-sequence} translates this information into imaginary
 prefix keys, all of which are symbols: @code{tab-line}, @code{header-line},
 @code{horizontal-scroll-bar}, @code{menu-bar}, @code{tab-bar}, @code{mode-line},
-@code{vertical-line}, and @code{vertical-scroll-bar}.  You can define
-meanings for mouse clicks in special window parts by defining key
-sequences using these imaginary prefix keys.
+@code{vertical-line}, @code{vertical-scroll-bar}, @code{left-margin},
+@code{right-margin}, @code{left-fringe}, @code{right-fringe},
+@code{right-divider}, and @code{bottom-divider}.  You can define meanings for
+mouse clicks in special window parts by defining key sequences using these
+imaginary prefix keys.
 
 For example, if you call @code{read-key-sequence} and then click the
 mouse on the window's mode line, you get two events, like this:
-- 
2.20.1

Re: Making TTY menus more visual

[Eli Zaretskii]

> Date: Sat, 24 Oct 2020 17:27:07 -0700
> From: Jared Finder <jared@finder.org>
> Cc: emacs-devel@gnu.org
> 
> Two further patches, one for a further doc fix and one for fixing the 
> tests that were reported broken off-thread.

Thanks, I installed the first one on master and the second one on
emacs-27.