Re: Moving point after character when clicking latter half of it

[Moritz Maxeiner <mm@ucw.sh>]

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

On Tuesday 11 July 2023 15:10:24 CEST Po Lu wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> > I'm not sure this is TRT.  In particular, it sounds like the change
> > you made in xterm.c also overrides the Y coordinate condition? that
> > doesn't sound right to me.  Po Lu, WDYT about this?
> > 
> > The other changes LGTM, thanks.
> 
> This part of the patch
> 
> >    if (frame != dpyinfo->last_mouse_glyph_frame
> >    
> >        || event->x < r->x || event->x >= r->x + r->width
> > 
> > -      || event->y < r->y || event->y >= r->y + r->height)
> > +      || event->y < r->y || event->y >= r->y + r->height
> > +      || mouse_click_prefer_closest_char && EQ (track_mouse,
> > Qdrag_tracking))
> appears to special case a value of `track_mouse', and unconditionally
> report mouse movement upon such a value being set, without considering
> if the mouse pointer has moved to a position within the glyph that could
> lead to a new position being reported.

True, though it is still conditional on the optional feature being enabled 
(both mouse_click_prefer_closest_char and drag_tracking, which mouse.el sets, 
need to be true). That being said I tried before to have this only be reported 
when the mouse crosses halfway point (without modifying the glyph itself) but 
that didn't really work.

> This is not only wrong (since
> Emacs shouldn't report mouse events differently even if `track_mouse' is
> some other non-nil value), but is also inefficient, as setting
> mouse_moved unnecessarily will result in an excessive number of mouse
> motion events being reported.

I'm not clear on why that would make it wrong (as in incorrect semantics). 
It's definitely inefficient (though I've not noticed the overhead in practice), 
which is why I asked if there's a more elegant solution.

> 
> I don't think we should disable the optimizations here entirely;
> instead, it should be just as effective to save only the half of the
> glyph containing the mouse pointer inside `remember_mouse_glyph' when
> `mouse_click_prefer_closest_char' is true.

Which this is, thank you very much for pointing that out. I've changed that 
part of the patch accordingly, though this does unfortunately mean that a new 
function is required, due to multiple places setting the glyph rectangle as it 
relates to dragging.


> 
> > +  DEFVAR_BOOL ("mouse-click-prefer-closest-char",
> > mouse_click_prefer_closest_char, +	       doc: /* Non-nil means mouse
> > click prefers the closest glyph as point. +When this is non-nil, clicking
> > inside a glyph picks up the next glyph if the click +is closer to it then
> > half the width of the clicked glyph.  */);
> > +  mouse_click_prefer_closest_char = false;
> 
> Since this affects much more than just mouse clicks, shouldn't the
> variable be named something else?  How about naming the variable
> `mouse-prefer-closest-glyph', and using the following doc string
> instead:
> 
>   Non-nil means mouse position lists are reported relative to the glyph
>   closest to their coordinates.
> 
>   When non-nil, mouse position lists will be reported with their
>   `posn-point' set to the position of the glyph closest to the mouse
>   pointer, instead of the glyph immediately under.

I've included those changes.

> 
> Also, shouldn't the Lisp reference manual mention this variable?  In
> (elisp)Accessing Mouse, for example.

Assuming this version of the implementation meets muster I will work on the 
etc/NEWS entry and can look into adding something to (elisp)Accessing Mouse, 
as well.

[-- Attachment #2: emacs-29-move_it_in_display_line_to-nextglyphafterhalf-poc-0.5.patch --]
[-- Type: text/x-patch, Size: 3956 bytes --]

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 054683d7cf6..2ed904f178f 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -231,6 +231,7 @@ Leaving \"Default\" unchecked is equivalent with specifying a default of
 	     (inverse-video display boolean)
 	     (visible-bell display boolean)
 	     (no-redraw-on-reenter display boolean)
+	     (mouse-prefer-closest-glyph display boolean)
 
              ;; doc.c
              (text-quoting-style display
diff --git a/src/dispnew.c b/src/dispnew.c
index 65d9cf9b4e1..669cd5f4d33 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5611,6 +5611,15 @@ buffer_posn_from_coords (struct window *w, int *x, int *y, struct display_pos *p
      argument is ZV to prevent move_it_in_display_line from matching
      based on buffer positions.  */
   move_it_in_display_line (&it, ZV, to_x, MOVE_TO_X);
+  if (mouse_prefer_closest_glyph)
+    {
+      int next_x = it.current_x + it.pixel_width;
+      int before_dx = to_x - it.current_x;
+      int after_dx = next_x - to_x;
+      if (before_dx > after_dx)
+        move_it_in_display_line (&it, ZV, next_x, MOVE_TO_X);
+    }
+
   bidi_unshelve_cache (itdata, 0);
 
   Fset_buffer (old_current_buffer);
@@ -6788,6 +6797,15 @@ predicates which report frame's specific UI-related capabilities.  */);
   DEFVAR_BOOL ("cursor-in-echo-area", cursor_in_echo_area,
 	       doc: /* Non-nil means put cursor in minibuffer, at end of any message there.  */);
 
+  DEFVAR_BOOL ("mouse-prefer-closest-glyph", mouse_prefer_closest_glyph,
+	       doc: /*  Non-nil means mouse position lists are reported relative
+to the glyph closest to their coordinates.
+
+ When non-nil, mouse position lists will be reported with their
+`posn-point' set to the position of the glyph closest to the mouse
+pointer, instead of the glyph immediately under.  */);
+  mouse_prefer_closest_glyph = false;
+
   DEFVAR_LISP ("glyph-table", Vglyph_table,
 	       doc: /* Table defining how to output a glyph code to the frame.
 If not nil, this is a vector indexed by glyph code to define the glyph.
diff --git a/src/xterm.c b/src/xterm.c
index 5840b15bcb7..b460603068c 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -14161,6 +14161,20 @@ x_construct_mouse_click (struct input_event *result,
   return Qnil;
 }
 
+/* Function to bisect `glyph` into left and right halves, then
+   replace it with the half in which `x` is.  */
+
+static void
+x_vertical_bisect_glyph(XRectangle *glyph, int x)
+{
+  int halfwidth = glyph->width / 2;
+  glyph->width = halfwidth;
+
+  int bisection = glyph->x + halfwidth;
+  if (x > bisection)
+    glyph->x = bisection;
+}
+
 /* Function to report a mouse movement to the mainstream Emacs code.
    The input handler calls this.
 
@@ -14218,6 +14232,8 @@ x_note_mouse_movement (struct frame *frame, const XMotionEvent *event,
       note_mouse_highlight (frame, event->x, event->y);
       /* Remember which glyph we're now on.  */
       remember_mouse_glyph (frame, event->x, event->y, r);
+      if (mouse_prefer_closest_glyph)
+        x_vertical_bisect_glyph(r, event->x);
       dpyinfo->last_mouse_glyph_frame = frame;
       return true;
     }
@@ -14382,6 +14398,8 @@ x_fast_mouse_position (struct frame **fp, int insist, Lisp_Object *bar_window,
 	  remember_mouse_glyph (f1, win_x, win_y,
 				&dpyinfo->last_mouse_glyph);
 	  dpyinfo->last_mouse_glyph_frame = f1;
+	  if (mouse_prefer_closest_glyph)
+	    x_vertical_bisect_glyph(&dpyinfo->last_mouse_glyph, win_x);
 
 	  *bar_window = Qnil;
 	  *part = scroll_bar_nowhere;
@@ -14733,6 +14751,8 @@ XTmouse_position (struct frame **fp, int insist, Lisp_Object *bar_window,
 	    dpyinfo = FRAME_DISPLAY_INFO (f1);
 	    remember_mouse_glyph (f1, win_x, win_y, &dpyinfo->last_mouse_glyph);
 	    dpyinfo->last_mouse_glyph_frame = f1;
+	    if (mouse_prefer_closest_glyph)
+	      x_vertical_bisect_glyph(&dpyinfo->last_mouse_glyph, win_x);
 
 	    *bar_window = Qnil;
 	    *part = 0;