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

[Moritz Maxeiner <mm@ucw.sh>]

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

On Sunday 9 July 2023 08:35:42 CEST Eli Zaretskii wrote:
> > From: Moritz Maxeiner <mm@ucw.sh>
> > Date: Sat, 08 Jul 2023 23:01:12 +0200
> > 
> > I am using emacs gtk gui alongside other graphical text editors (left to
> > right text only).
> > 
> > In all of the ones I'm using (other than emacs), clicking with the mouse
> > on a character moves the point either in front of, or after that
> > character,
> > depending on whether you clicked the left or right half of it.
> > 
> > In emacs, however, point seems to always be moved in front of the clicked
> > character, regardless of where on it you click. It would be great if
> > emacs could (optionally) also support the before/after behavior described
> > above.
> > 
> > After delving into the code, it seems that this would need changes in C.
> > I have attached a proof of concept patch that changes emacs' behavior,
> > but it lacks an option mechanism. I also am not familiar enough
> > with what unintended consequences this change may have.
> 
> Thanks, but the place where you suggest to make the change is not the
> correct one.  The function move_it_in_display_line_to is the workhorse
> of "display emulation", and is used by any code which needs to perform
> calculations related to display layout without producing anything on
> the glass.  It is fundamentally wrong to modify how all of that code
> finds the position corresponding to a certain X coordinate just
> because the user wants a mouse click to move point to the next
> character.
> 
> The right place is in buffer_posn_from_coords.  The change will be
> more complex there, but there's no way around this, since we want this
> change to affect only mouse clicks.

I am not surprised, given that I know little about Emacs internals at this 
time. Thank you for the explanation. After looking at it a bit more I'm not 
sure if/how it can be accomplished without any modifications to 
move_it_in_display_line_to, given that it uses/modifies the iterator in many 
places and afaict we don't have access to glyph width outside of it.

Would adding another option to move_it_in_display_line_to be acceptable? That 
way only functions that explicitly select the new halfway behavior, like I did 
with buffer_posn_from_coords in the new version of the poc patch.

So far it doesn't yet include an user-configurable option as I don't yet know 
how to do that in emacs' c source.

> 
> > I am looking for feedback of whether this feature in general is something
> > that would be acceptable to have in emacs and if there are any pitfalls
> > this change may cause that I need to be aware of.
> 
> I think this will be acceptable as optional behavior, yes.  But we
> need to consider all of its implications, such as what should happen
> when the use drags the mouse -- do we also want the drag to affect
> only the following character if the initial click is half-way across a
> character's glyph?

That's good to hear. With respect to mouse dragging: Specifically in the case 
that it's used to select text, yes, I would also like it to use the new 
halfway behavior in that case, but I have not yet found where in emacs' source 
code that change would need to happen.

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

diff --git a/src/dispextern.h b/src/dispextern.h
index ece128949f5..d5eceef2369 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2841,7 +2841,10 @@ #define PRODUCE_GLYPHS(IT)                              \
   MOVE_TO_VPOS = 0x04,
 
   /* Stop if specified buffer or string position is reached.  */
-  MOVE_TO_POS = 0x08
+  MOVE_TO_POS = 0x08,
+
+  /* If MOVE_TO_X, x-position is only reached by a glyph's first half. */
+  MOVE_TO_X_FIRSTHALF = 0x10
 };
 
 /***********************************************************************
diff --git a/src/dispnew.c b/src/dispnew.c
index 65d9cf9b4e1..e0d972c8be2 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5610,7 +5610,7 @@ buffer_posn_from_coords (struct window *w, int *x, int *y, struct display_pos *p
   /* Now move horizontally in the row to the glyph under *X.  Second
      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);
+  move_it_in_display_line (&it, ZV, to_x, MOVE_TO_X | MOVE_TO_X_FIRSTHALF);
   bidi_unshelve_cache (itdata, 0);
 
   Fset_buffer (old_current_buffer);
diff --git a/src/xdisp.c b/src/xdisp.c
index 763af7d3bc8..d3fdfa0b583 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -9905,6 +9905,7 @@ #define IT_RESET_X_ASCENT_DESCENT(IT)			\
 	  /* More than one glyph or glyph doesn't fit on line.  All
 	     glyphs have the same width.  */
 	  int single_glyph_width = it->pixel_width / it->nglyphs;
+	  int single_glyph_halfwidth = ceil(single_glyph_width / 2.0);
 	  int new_x;
 	  int x_before_this_char = x;
 	  int hpos_before_this_char = it->hpos;
@@ -9914,7 +9915,7 @@ #define IT_RESET_X_ASCENT_DESCENT(IT)			\
 	      new_x = x + single_glyph_width;
 
 	      /* We want to leave anything reaching TO_X to the caller.  */
-	      if ((op & MOVE_TO_X) && new_x > to_x)
+	      if ((op & MOVE_TO_X) && ((op & MOVE_TO_X_FIRSTHALF)? (x + single_glyph_halfwidth) : new_x) > to_x)
 		{
 		  if (BUFFER_POS_REACHED_P ())
 		    {