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

[Yuan Fu]

> On Jul 12, 2023, at 2:36 PM, Moritz Maxeiner <mm@ucw.sh> wrote:
> 
> 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).

Ah, the code is just to show that similar things might be accomplishable by 
modifying Elisp functions. I don’t suggest using it as-is. Besides what you 
pointed out, it also uses advice, which is bad.

Yuan