Re: mouse-face and help echo support for xterm mouse

[Jared Finder via "Emacs development discussions." <emacs-devel@gnu.org>]

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



On 2020-11-01 5:39 am, Stefan Monnier wrote:
> Hi Jared,
> 
> I really like this new feature but have just one comment/question?
> 
>> +  previous_help_echo_string = help_echo_string;
>> +  help_echo_string = Qnil;
>> +
>> +  note_mouse_highlight(XFRAME(frame), XFIXNUM (mouse_x), XFIXNUM 
>> (mouse_y));
>> +
>> +  if (!NILP (help_echo_string)
>> +      || !NILP (previous_help_echo_string))
>> +    {
>> +      gen_help_event (help_echo_string, frame, help_echo_window,
>> +                      help_echo_object, help_echo_pos);
>> +    }
> 
> I see this exact same code in other C files.
> Could we move it to a file where we can share it instead of having
> N copies?

With the code simplification in, this logic is now sharable between 
xterm-mouse and GPM.  Attached is an updated patch.

This patch does have one actual logic change: Previously 
handle_one_term_event might call gen_help_event if a GPM_MOVE_EVENT or 
GPM_DRAG_EVENT happened but the mouse position did not change.  With 
this patch, this is no longer the case.  From testing locally with 
running GPM mouse, this seems to not cause any user-visible change.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Face-changing-text-properties-and-help-echo-now-work.patch --]
[-- Type: text/x-diff; name=0001-Face-changing-text-properties-and-help-echo-now-work.patch, Size: 6680 bytes --]

From 98e18207726ddeb1f922e9b95f1fd4822502a66a Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 31 Oct 2020 21:25:47 -0800
Subject: [PATCH] Face-changing text properties and help-echo now work with
 xterm-mouse.

* src/dispnew.c (update_mouse_position): New function for mouse movement
logic in 'handle_one_term_event' that should be shared across GPM and
xterm-mouse.
(handle-lisp-mouse-motion): New lisp function, call it.
* lisp/xt-mouse.el (xterm-mouse--handle-mouse-motion): New function that
calls 'handle-lisp-mouse-motion'.
(xterm-mouse-translate-1): Call it.
* src/term.c (handle_one_term_event): Inline logic from
'term_mouse_movement' and call 'update_mouse_position'.
(term_mouse_movement): Delete.
---
 lisp/xt-mouse.el |  9 +++++++++
 src/dispextern.h |  1 +
 src/dispnew.c    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 src/term.c       | 46 ++++++++--------------------------------------
 4 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index f9c08f9a17..37550276f8 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -77,6 +77,7 @@ xterm-mouse-translate-1
               (copy-sequence event))
 	vec)
        (is-move
+        (xterm-mouse--handle-mouse-motion)
         (if track-mouse vec
           ;; Mouse movement events are currently supposed to be
           ;; suppressed.  Return no event.
@@ -106,8 +107,16 @@ xterm-mouse-translate-1
 	      (if (null track-mouse)
 		  (vector drag)
 		(push drag unread-command-events)
+                (xterm-mouse--handle-mouse-motion)
 		(vector (list 'mouse-movement ev-data))))))))))))
 
+(defun xterm-mouse--handle-mouse-motion ()
+  "Handle mouse motion that was just generated for XTerm mouse."
+  (let ((frame (selected-frame)))
+    (handle-lisp-mouse-motion frame
+                              (terminal-parameter frame 'xterm-mouse-x)
+                              (terminal-parameter frame 'xterm-mouse-y))))
+
 ;; These two variables have been converted to terminal parameters.
 ;;
 ;;(defvar xterm-mouse-x 0
diff --git a/src/dispextern.h b/src/dispextern.h
index 848d3bcd20..da51772b37 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3606,6 +3606,7 @@ #define IMAGE_BACKGROUND_TRANSPARENT(img, f, mask)			      \
 extern void redraw_frame (struct frame *);
 extern bool update_frame (struct frame *, bool, bool);
 extern void update_frame_with_menu (struct frame *, int, int);
+extern int update_mouse_position (struct frame *, int, int);
 extern void bitch_at_user (void);
 extern void adjust_frame_glyphs (struct frame *);
 void free_glyphs (struct frame *);
diff --git a/src/dispnew.c b/src/dispnew.c
index 3f2ae3e6ad..545cae628f 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -3323,6 +3323,52 @@ update_frame_with_menu (struct frame *f, int row, int col)
   display_completed = !paused_p;
 }
 
+/* Update the mouse position for a frame F.  This handles both
+   updating the display for mouse-face propreties and updating the
+   help echo text.
+
+   Returns the number of events generated.  */
+int
+update_mouse_position (struct frame *f, int x, int y)
+{
+  previous_help_echo_string = help_echo_string;
+  help_echo_string = Qnil;
+
+  note_mouse_highlight (f, x, y);
+
+  /* If the contents of the global variable help_echo_string
+     has changed, generate a HELP_EVENT.  */
+  if (!NILP (help_echo_string)
+      || !NILP (previous_help_echo_string))
+    {
+      Lisp_Object frame;
+      XSETFRAME(frame, f);
+
+      gen_help_event (help_echo_string, frame, help_echo_window,
+                      help_echo_object, help_echo_pos);
+      return 1;
+    }
+
+  return 0;
+}
+
+DEFUN ("handle-lisp-mouse-motion", Fhandle_lisp_mouse_motion,
+       Shandle_lisp_mouse_motion, 3, 3, 0,
+       doc: /* Handle mouse motion detected by Lisp code.
+
+This function should be called when Lisp code detects the mouse has
+moved, even if `track-mouse' is nil.  This handles updates that do not
+not rely on input events such as updating display for mouse-face
+proprties or updating the help echo text.  */)
+  (Lisp_Object frame, Lisp_Object mouse_x, Lisp_Object mouse_y)
+{
+  if (NILP (frame))
+    frame = selected_frame;
+
+  update_mouse_position (XFRAME (frame), XFIXNUM (mouse_x), XFIXNUM (mouse_y));
+  return Qnil;
+}
+
 \f
 /************************************************************************
 			 Window-based updates
@@ -6490,6 +6536,7 @@ syms_of_display (void)
 {
   defsubr (&Sredraw_frame);
   defsubr (&Sredraw_display);
+  defsubr (&Shandle_lisp_mouse_motion);
   defsubr (&Sframe_or_buffer_changed_p);
   defsubr (&Sopen_termscript);
   defsubr (&Sding);
diff --git a/src/term.c b/src/term.c
index 3a13da165e..df34983344 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2430,22 +2430,6 @@ tty_draw_row_with_mouse_face (struct window *w, struct glyph_row *row,
   cursor_to (f, save_y, save_x);
 }
 
-static bool
-term_mouse_movement (struct frame *frame, Gpm_Event *event)
-{
-  /* Has the mouse moved off the glyph it was on at the last sighting?  */
-  if (event->x != last_mouse_x || event->y != last_mouse_y)
-    {
-      frame->mouse_moved = 1;
-      note_mouse_highlight (frame, event->x, event->y);
-      /* Remember which glyph we're now on.  */
-      last_mouse_x = event->x;
-      last_mouse_y = event->y;
-      return 1;
-    }
-  return 0;
-}
-
 /* Return the current time, as a Time value.  Wrap around on overflow.  */
 static Time
 current_Time (void)
@@ -2562,30 +2546,16 @@ handle_one_term_event (struct tty_display_info *tty, Gpm_Event *event)
 
   if (event->type & (GPM_MOVE | GPM_DRAG))
     {
-      previous_help_echo_string = help_echo_string;
-      help_echo_string = Qnil;
-
       Gpm_DrawPointer (event->x, event->y, fileno (tty->output));
 
-      if (!term_mouse_movement (f, event))
-        help_echo_string = previous_help_echo_string;
-
-      /* If the contents of the global variable help_echo_string
-         has changed, generate a HELP_EVENT.  */
-      if (!NILP (help_echo_string)
-	  || !NILP (previous_help_echo_string))
-	{
-	  Lisp_Object frame;
-
-	  if (f)
-	    XSETFRAME (frame, f);
-	  else
-	    frame = Qnil;
-
-	  gen_help_event (help_echo_string, frame, help_echo_window,
-		          help_echo_object, help_echo_pos);
-	  count++;
-	}
+      /* Has the mouse moved off the glyph it was on at the last sighting?  */
+      if (event->x != last_mouse_x || event->y != last_mouse_y)
+        {
+          last_mouse_x = event->x;
+          last_mouse_y = event->y;
+          f->mouse_moved = 1;
+          count += update_mouse_position (f, event->x, event->y);
+        }
     }
   else
     {
-- 
2.20.1