unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Moritz Maxeiner <mm@ucw.sh>
To: Po Lu <luangruo@yahoo.com>, Eli Zaretskii <eliz@gnu.org>
Cc: emacs-devel@gnu.org
Subject: Re: Moving point after character when clicking latter half of it
Date: Wed, 12 Jul 2023 21:58:44 +0200	[thread overview]
Message-ID: <3242369.aeNJFYEL58@anduin> (raw)
In-Reply-To: <87mt02ar4e.fsf@yahoo.com>

[-- Attachment #1: Type: text/plain, Size: 5480 bytes --]

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.

[-- Attachment #2: emacs-29-move_it_in_display_line_to-nextglyphafterhalf-poc-0.6.patch --]
[-- Type: text/x-patch, Size: 6061 bytes --]

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index cd1745614eb..274c456d8c3 100644
--- a/doc/lispref/commands.texi
+++ b/doc/lispref/commands.texi
@@ -1969,6 +1969,8 @@ events do not appear.  @xref{Mouse Tracking}.
 When non-@code{nil}, mouse motion events are generated even for very
 small movements.  Otherwise, motion events are not generated as long
 as the mouse cursor remains pointing to the same glyph in the text.
+Note that non-@code{nil} @code{mouse-prefer-closest-glyph} changes
+that to the left/right half of the glyph under the mouse cursor instead.
 @end defvar
 
 @node Touchscreen Events
@@ -2751,6 +2753,14 @@ If @var{whole} is non-@code{nil}, the @var{x} coordinate is relative
 to the entire window area including scroll bars, margins and fringes.
 @end defun
 
+@defvar mouse-prefer-closest-glyph
+If you set this variable to non-@code{nil}, whenever you click or drag the mouse,
+instead of the point being always set in front of the clicked glyph, the point
+horizontally closest to the mouse position will be used.
+So if you click in the left half of a glyph, point is set in front of it,
+but if you click in the right half, point is set after it.
+@end defvar
+
 @node Accessing Scroll
 @subsection Accessing Scroll Bar Events
 @cindex scroll bar events, data in
diff --git a/etc/NEWS b/etc/NEWS
index 9e6f0c16bcd..978ba0dfa69 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -952,6 +952,14 @@ wheel reports.  Unlike 'pixel-scroll-mode', this mode scrolls the
 display pixel-by-pixel, as opposed to only animating line-by-line
 scrolls.
 
++++
+** New user option 'mouse-prefer-closest-glyph'.
+When enabled, clicking or dragging with the mouse will put the point
+in front of the glyph with the closest x coordinate to the mouse pointer.
+In other words, if the mouse pointer is in the right half of a glyph,
+point will be put after that glyph, while if the mouse pointer is in the
+left half of a glyph, point will be put in front of that glyph.
+
 ** Terminal Emacs
 
 ---
diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 054683d7cf6..2ed904f178f 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -231,6 +231,7 @@ Leaving \"Default\" unchecked is equivalent with specifying a default of
 	     (inverse-video display boolean)
 	     (visible-bell display boolean)
 	     (no-redraw-on-reenter display boolean)
+	     (mouse-prefer-closest-glyph display boolean)
 
              ;; doc.c
              (text-quoting-style display
diff --git a/src/dispnew.c b/src/dispnew.c
index 65d9cf9b4e1..669cd5f4d33 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5611,6 +5611,15 @@ buffer_posn_from_coords (struct window *w, int *x, int *y, struct display_pos *p
      argument is ZV to prevent move_it_in_display_line from matching
      based on buffer positions.  */
   move_it_in_display_line (&it, ZV, to_x, MOVE_TO_X);
+  if (mouse_prefer_closest_glyph)
+    {
+      int next_x = it.current_x + it.pixel_width;
+      int before_dx = to_x - it.current_x;
+      int after_dx = next_x - to_x;
+      if (before_dx > after_dx)
+        move_it_in_display_line (&it, ZV, next_x, MOVE_TO_X);
+    }
+
   bidi_unshelve_cache (itdata, 0);
 
   Fset_buffer (old_current_buffer);
@@ -6788,6 +6797,15 @@ predicates which report frame's specific UI-related capabilities.  */);
   DEFVAR_BOOL ("cursor-in-echo-area", cursor_in_echo_area,
 	       doc: /* Non-nil means put cursor in minibuffer, at end of any message there.  */);
 
+  DEFVAR_BOOL ("mouse-prefer-closest-glyph", mouse_prefer_closest_glyph,
+	       doc: /*  Non-nil means mouse position lists are reported relative
+to the glyph closest to their coordinates.
+
+ When non-nil, mouse position lists will be reported with their
+`posn-point' set to the position of the glyph closest to the mouse
+pointer, instead of the glyph immediately under.  */);
+  mouse_prefer_closest_glyph = false;
+
   DEFVAR_LISP ("glyph-table", Vglyph_table,
 	       doc: /* Table defining how to output a glyph code to the frame.
 If not nil, this is a vector indexed by glyph code to define the glyph.
diff --git a/src/xdisp.c b/src/xdisp.c
index 763af7d3bc8..4ab53d47f0c 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2723,6 +2723,7 @@ remember_mouse_glyph (struct frame *f, int gx, int gy, NativeRectangle *rect)
   enum window_part part;
   enum glyph_row_area area;
   int x, y, width, height;
+  int original_gx;
 
   if (mouse_fine_grained_tracking)
     {
@@ -2733,6 +2734,8 @@ remember_mouse_glyph (struct frame *f, int gx, int gy, NativeRectangle *rect)
   /* Try to determine frame pixel position and size of the glyph under
      frame pixel coordinates X/Y on frame F.  */
 
+  original_gx = gx;
+
   if (window_resize_pixelwise)
     {
       width = height = 1;
@@ -2950,6 +2953,16 @@ remember_mouse_glyph (struct frame *f, int gx, int gy, NativeRectangle *rect)
  store_rect:
   STORE_NATIVE_RECT (*rect, gx, gy, width, height);
 
+  if (mouse_prefer_closest_glyph)
+    {
+      int half_width = rect->width / 2;
+      rect->width = half_width;
+
+      int bisection = rect->x + half_width;
+      if (original_gx > bisection)
+        rect->x = bisection;
+    }
+
   /* Visible feedback for debugging.  */
 #if false && defined HAVE_X_WINDOWS
   XDrawRectangle (FRAME_X_DISPLAY (f), FRAME_X_DRAWABLE (f),
@@ -37345,7 +37358,10 @@ may be more familiar to users.  */);
   DEFVAR_BOOL ("mouse-fine-grained-tracking", mouse_fine_grained_tracking,
     doc: /* Non-nil for pixel-wise mouse-movement.
 When nil, mouse-movement events will not be generated as long as the
-mouse stays within the extent of a single glyph (except for images).  */);
+mouse stays within the extent of a single glyph (except for images).
+When nil and mouse-prefer-closest-glyph is non-nil, mouse-movement
+events will instead not be generated as long as the mouse stays within
+the extent of a single left/right half glyph (except for images).  */);
   mouse_fine_grained_tracking = false;
 
   DEFVAR_BOOL ("tab-bar--dragging-in-progress", tab_bar__dragging_in_progress,

  reply	other threads:[~2023-07-12 19:58 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-08 21:01 Moving point after character when clicking latter half of it Moritz Maxeiner
2023-07-09  6:35 ` Eli Zaretskii
2023-07-09 12:44   ` Moritz Maxeiner
2023-07-09 13:23     ` Eli Zaretskii
2023-07-09 13:51       ` Moritz Maxeiner
2023-07-09 14:14         ` Eli Zaretskii
2023-07-09 21:47           ` Moritz Maxeiner
2023-07-10 12:46             ` Eli Zaretskii
2023-07-10 14:43               ` [External] : " Drew Adams
2023-07-10 20:02               ` Moritz Maxeiner
2023-07-11 12:29                 ` Eli Zaretskii
2023-07-11 13:10                   ` Po Lu
2023-07-11 18:01                     ` Moritz Maxeiner
2023-07-12  0:52                       ` Po Lu
2023-07-12 19:58                         ` Moritz Maxeiner [this message]
2023-07-12 21:17                           ` Yuan Fu
2023-07-12 21:36                             ` Moritz Maxeiner
2023-07-12 22:08                               ` Yuan Fu
2023-07-13  5:27                             ` Eli Zaretskii
2023-07-13 23:25                               ` Yuan Fu
2023-07-13  0:31                           ` Po Lu
2023-07-13  8:47                           ` Eli Zaretskii
2023-07-21 19:04                             ` Moritz Maxeiner
2023-07-21 23:57                               ` Po Lu
2023-07-22  5:41                                 ` Eli Zaretskii
2023-07-22 10:07                                   ` Moritz Maxeiner
2023-07-22 11:31                                     ` Po Lu
2023-07-22 12:51                                     ` Eli Zaretskii
2023-07-22 15:28                                       ` Moritz Maxeiner
2023-07-22 15:51                                         ` Eli Zaretskii
2023-07-22 15:59                                           ` Moritz Maxeiner
2023-07-22 16:34                                             ` Eli Zaretskii
2023-07-22 19:10                                             ` Yuan Fu
2023-07-09 13:58       ` Yuri Khan
2023-07-09 12:40 ` Benjamin Riefenstahl
2023-07-09 12:47   ` Moritz Maxeiner
2023-07-09 13:37     ` Benjamin Riefenstahl
2023-07-09 15:15   ` [External] : " Drew Adams
2023-07-09 15:33     ` Moritz Maxeiner
2023-07-09 16:06       ` Drew Adams
2023-07-09 16:21       ` Brian Cully via Emacs development discussions.
2023-07-09 18:01         ` Jens Schmidt
2023-07-09 16:43       ` [External] : " Eli Zaretskii
2023-07-12 18:21     ` Benjamin Riefenstahl
2023-07-12 18:32       ` Eli Zaretskii
     [not found] ` <12248204.O9o76ZdvQC@anduin>
     [not found]   ` <87ilac2kla.fsf@yahoo.com>
2023-07-22 14:48     ` Moritz Maxeiner
2023-07-22 15:26       ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3242369.aeNJFYEL58@anduin \
    --to=mm@ucw.sh \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=luangruo@yahoo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).