emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Yuan Fu
Subject: Re: Moving point after character when clicking latter half of it
Date: Wed, 12 Jul 2023 14:17:41 -0700


> 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

Attachment: delicate-click.el
Description: Binary data



reply via email to

[Prev in Thread] Current Thread [Next in Thread]