Re: Making TTY menus more visual

[Jared Finder]

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

0001-Adding-mouse-controls-to-menu-bar.el.patch
Description: Text Data

0002-Making-TTY-menus-work-with-xterm-mouse-mode.patch
Description: Text Data