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: 2223 bytes --]

On 2020-11-05 10:00 pm, Eli Zaretskii wrote:
>> Date: Thu, 05 Nov 2020 21:23:26 -0800
>> Cc: "Jared Finder via \"Emacs development discussions.\"" 
>> <emacs-devel@gnu.org>
>> From: Jared Finder via "Emacs development discussions." 
>> <emacs-devel@gnu.org>
>> 
>> All points addressed.  New patch attached.
> 
> Thanks.
> 
>> * src/dispnew.c (update_mouse_position): New function for mouse 
>> movement
>> logic in 'handle_one_term_event' that can be shared across different
>> mouse backends.
>> (display--update-for-mouse-movement): New lisp function, call it.
>> * lisp/xt-mouse.el (xterm-mouse--handle-mouse-movement): New function 
>> that
>> calls 'display--update-for-mouse-movement'.
>> (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.
> 
> Nitpicking: the lines in the change log are too long, they will
> overflow 80 columns when indented by TABs (which happens when we
> generate a ChangeLog file from Git log).  Please use one of the Emacs
> commands available for generating ChangeLog entries, they will keep
> you from making these mistakes.

Oops, sorry.  I hand-verified each row was 72 or less characters just 
now.  I will try to learn the Emacs commands for dealing with 
changelogs.

>> +      XSETFRAME(frame, f);
>                   ^
> Please leave a space before the opening parenthesis, to conform to our
> coding conventsions.

Done.

>> +  update_mouse_position (XFRAME (selected_frame), XFIXNUM (mouse_x),
>                             ^^^^^^^^^^^^^^^^^^^^^^^
> A.k.a. SELECTED_FRAME().

Done.

>>  (defface mode-line-highlight
>> -  '((((class color) (min-colors 88))
>> +  '((((type graphic) (class color) (min-colors 88))
>>       :box (:line-width 2 :color "grey40" :style released-button))
> 
> I don't think I understand the rationale.  With TTYs supporting many
> colors nowadays, and mode-line-highlight available on TTYs, what is
> the problem you tried to fix here?

Are there any TTYs that support :box?  None of the platforms I tested 
locally on do, they instead just ignore the :box aspect of any face.


Updated patches attached.

   -- 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: 6987 bytes --]

From 039c399a5078a9e95d9f49cb8b5a9a941494bf57 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Sat, 31 Oct 2020 21:25:47 -0800
Subject: [PATCH 1/2] 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 can be shared across
different mouse backends.
(display--update-for-mouse-movement): New lisp function, call it.
* lisp/xt-mouse.el (xterm-mouse--handle-mouse-movement): New function
that calls 'display--update-for-mouse-movement'.
(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 |  7 +++++++
 src/dispextern.h |  1 +
 src/dispnew.c    | 48 ++++++++++++++++++++++++++++++++++++++++++++
 src/term.c       | 52 +++++++++++++-----------------------------------
 4 files changed, 70 insertions(+), 38 deletions(-)

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index f9c08f9a17..9301476e81 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-movement)
         (if track-mouse vec
           ;; Mouse movement events are currently supposed to be
           ;; suppressed.  Return no event.
@@ -106,8 +107,14 @@ xterm-mouse-translate-1
 	      (if (null track-mouse)
 		  (vector drag)
 		(push drag unread-command-events)
+                (xterm-mouse--handle-mouse-movement)
 		(vector (list 'mouse-movement ev-data))))))))))))
 
+(defun xterm-mouse--handle-mouse-movement ()
+  "Handle mouse motion that was just generated for XTerm mouse."
+  (display--update-for-mouse-movement (terminal-parameter nil 'xterm-mouse-x)
+                                      (terminal-parameter nil '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..2e40d458d1 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -3323,6 +3323,53 @@ 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 ("display--update-for-mouse-movement", Fdisplay__update_for_mouse_movement,
+       Sdisplay__update_for_mouse_movement, 2, 2, 0,
+       doc: /* Handle mouse movement 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
+rely on input events such as updating display for mouse-face
+properties or updating the help echo text.  */)
+  (Lisp_Object mouse_x, Lisp_Object mouse_y)
+{
+  CHECK_FIXNUM (mouse_x);
+  CHECK_FIXNUM (mouse_y);
+
+  update_mouse_position (SELECTED_FRAME (), XFIXNUM (mouse_x),
+                         XFIXNUM (mouse_y));
+  return Qnil;
+}
+
 \f
 /************************************************************************
 			 Window-based updates
@@ -6490,6 +6537,7 @@ syms_of_display (void)
 {
   defsubr (&Sredraw_frame);
   defsubr (&Sredraw_display);
+  defsubr (&Sdisplay__update_for_mouse_movement);
   defsubr (&Sframe_or_buffer_changed_p);
   defsubr (&Sopen_termscript);
   defsubr (&Sding);
diff --git a/src/term.c b/src/term.c
index 3a13da165e..a0738594bf 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,22 @@ 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)
+        {
+          /* FIXME: These three lines can not be moved into
+             update_mouse_position unless xterm-mouse gets updated to
+             generate mouse events via C code.  See
+             https://lists.gnu.org/archive/html/emacs-devel/2020-11/msg00163.html */
+          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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-lisp-faces.el-mode-line-highlight-Use-box-only-on-no.patch --]
[-- Type: text/x-diff; name=0002-lisp-faces.el-mode-line-highlight-Use-box-only-on-no.patch, Size: 730 bytes --]

From dcf2b7070f48676960fd99f497df6bcb1035ce2a Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Thu, 5 Nov 2020 21:15:08 -0800
Subject: [PATCH 2/2] * lisp/faces.el (mode-line-highlight): Use :box only on
 non-TTYs.

---
 lisp/faces.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/faces.el b/lisp/faces.el
index 728f8b0fe6..875bee9910 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2578,7 +2578,7 @@ mode-line-inactive
   :group 'basic-faces)
 
 (defface mode-line-highlight
-  '((((class color) (min-colors 88))
+  '((((type graphic) (class color) (min-colors 88))
      :box (:line-width 2 :color "grey40" :style released-button))
     (t
      :inherit highlight))
-- 
2.20.1