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

[Moritz Maxeiner <mm@ucw.sh>]

On Wednesday, 12 July 2023 23:17:41 CEST Yuan Fu wrote:
> 
> > On Jul 12, 2023, at 12:58 PM, Moritz Maxeiner <mm@ucw.sh> wrote:
> > 
> > On Wednesday 12 July 2023 02:52:01 CEST Po Lu wrote:
> >> Moritz Maxeiner <mm@ucw.sh> writes:
> >>> 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.
> >> 
> >> The overhead is apparent when options such as
> >> `mouse-drag-and-drop-region' are enabled and the connection to the X
> >> server is slow.
> >> 
> >>> 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.
> >> 
> >> I find that difficult to believe.  Would you please describe the other
> >> callers of `remember_mouse_glyph' that make adjustments there
> >> impossible?
> >> 
> >> I asked you to make the change in `remember_mouse_glyph' because that
> >> would avoid the need to modify each of the *term.[cm] files
> >> individually.  Replacing it with a different function would miss the
> >> point of the change.
> > 
> > My apologies, after rereading your previous message I realize I misunderstood. 
> > I thought I was to put the changes after the remember_mouse_glyph call (like I 
> > was previously asked to do for move_it_in_display_line instead of modifying 
> > another xdisp.c function, move_it_in_display_line_to). My comment derived from 
> > that misunderstanding as there are multiple calls to remember_mouse_glyph that 
> > need to be affected.
> > 
> > I have adjusted the patch as requested (I think), added some documentation in 
> > commands.texi, as well as a NEWS entry. I'm not sure about correct placement / 
> > formatting of the latter two.
> > 
> > Btw. I'm not particularly happy about needed to add the `original_gx' 
> > variable, but since the function seems to overwrite its argument, which I need 
> > access to at its end (once the full glyph has been determined), I don't see 
> > another option. I'm also not super happy about the needed division, but I also 
> > don't see a way around that. If you know a more elegant solution I'd be happy 
> > to hear it.
> > 
> >> 
> >>> 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.
> >> 
> >> Several other comments below:
> >>> +/* 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;
> >>> +}
> >> 
> >> Please follow the GNU coding standards for both function declarators and
> >> comments, by capitalizing (not quoting) arguments which appear in the
> >> commentary, and placing a space between the identifier name and the
> >> opening parentheses of the parameter type list.  Also, use the active
> >> voice when describing the behavior of a function within its commentary:
> >> 
> >> /* Replace *GLYPH, a rectangle containing the bounds of a glyph, with
> >>   the half of the rectangle containing the position X.  */
> >> 
> >> static void
> >> x_vertical_bisect_glyph (XRectangle *glyph, int x)
> >> {
> >>  /* ... */
> >> }
> >> 
> >> In addition, we don't use Markdown style quotes for code.  When quoting
> >> identifier names in the future, either write:
> >> 
> >>  /* `foo' is used to perform ...  */
> >> 
> >> or
> >> 
> >>  /* 'foo' is used to perform ...  */
> >> 
> >>> /* 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);
> >> 
> >> Please place a space between the identifier name and the opening brace
> >> of each function call.
> >> 
> >> However, implementing this feature shouldn't need changes to xterm.c at
> >> all.  Please make the change in `remember_mouse_glyph', not in window
> >> system specific code.
> > 
> > Since that function (and calls to it) are now merged into 
> > `remember_mouse_glyph', I think these are solved by default. I'll take more 
> > care next time, though, thank you for the correction.<emacs-29-move_it_in_display_line_to-nextglyphafterhalf-poc-0.6.patch>
> 
> Sorry for joining late into the discussion. But I wonder if a Elisp solution would suffice? Someone had requested this feature to me in the past and this is my solution at the time.
> 
> Yuan
> 
> 

Thank you for the code. I've tried it out and it does seem to do the same thing for a single mouse click, but it does not change the behavior of mouse dragging (selection highlighting) in the same way, which I also want/need (and this patch does).