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

[Moritz Maxeiner]

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
Description: Text Data