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

[Eli Zaretskii <eliz@gnu.org>]

> From: Moritz Maxeiner <mm@ucw.sh>
> Cc: emacs-devel@gnu.org
> Date: Sun, 09 Jul 2023 23:47:51 +0200
> 
> > Basically, you need to examine it.current_x after the last call to
> > move_it_in_display_line returns, and if it is closer to
> > to_x+it.pixel_width than to to_x, call move_it_in_display_line again
> > to get to to_x+it.pixel_width.
> 
> Thanks, I've implemented something along those lines. Same source position,
> but the check is if to_x exceeds half of the selected display element's width
> and if so rerun move_it_in_display_line at it.current_x + it.pixel_width
> (so first pixel of the next display element).
> Running it at to_x+it.pixel_width might run into trouble if the current display
> element is a lot wider than the one after it.

OK, but would you please rewrite the condition to avoid unnecessary
floating-point computations?  There's no need to compute half of
pixel_width, only to see if to_x is closer to the next display element
than it is to the current display element.

> I've also added an option mouse_point_alternate (temporary name, didn't know
> what else to call it, open to suggestions) to enable it. It defaults to off.

I can suggest mouse-click-prefer-closest-char.

> But I have to say it feels wasteful to call the function twice instead of having
> it deal with it "correctly" the first time around with an option.

That's because you aren't accustomed with this kind of code.  We do
this all the time, and there's nothing wasteful about it.

>  #include "sysstdio.h"
>  #include <stdlib.h>
> +#include <math.h>
>  #include <unistd.h>

This will be unnecessary once you get rid of the FP calculations.

>    move_it_in_display_line (&it, ZV, to_x, MOVE_TO_X);
> +  if (mouse_point_alternate && to_x > it.current_x + ceil (it.pixel_width / 2.0))
> +    move_it_in_display_line (&it, ZV, it.current_x + it.pixel_width, MOVE_TO_X);

These two lines are too long, please break them in two.

> @@ -6468,6 +6472,7 @@ init_display_interactive (void)
>  
>    inverse_video = 0;
>    cursor_in_echo_area = false;
> +  mouse_point_alternate = false;

This initialization should be immediately after the DEFVAR.

> +  DEFVAR_BOOL ("mouse-point-alternate", mouse_point_alternate,
> +	       doc: /* Non-nil means use alternate mode for mouse click selection of point.

The first line of the doc string will be more useful if it says
something about the effect of the option.  Something like this,
perhaps:

  Non-nil means mouse click prefers the closest glyph.

> +In alternate mode, if the clicked x coordinate exceeds half of the selected
> +display element's width, put point after it.  */);

This describes the effect in terms of the implementation more than in
terms of UX.  How about

  When this is non-nil, clicking inside a glyph picks up the next
  glyph if the click is closer to it than half the width of the
  clicked glyph.

This variable should be added to cus-start.el, with a suitable
customization form, since this is a user option.  It should also be
called out in etc/NEWS.

Thanks.