[PATCH] Improve interaction between mouse-drag-region and scroll-margin

[PATCH] Improve interaction between mouse-drag-region and scroll-margin

[Yuri D'Elia]

* lisp/mouse.el (mouse-drag-track): Disable both scroll-margin and
auto-hscroll-mode in mouse-drag-region and do not re-enable them until
dragging is over, making selections work as expected when inside the
margins.

Since mouse-drag-region is initiated on the initial down-mouse event, we
_shouldn't_ scroll the point until up-mouse is finally received, as it
would otherwise interfere with the regular mouse-1 event. If the point
is moved on down-mouse an immediate motion event results, jump-starting
the drag.

Complicating things, as the tracking is done in a transient map, a
let-bound scroll-margin is not sufficient. We must save/restore its
value, as done already for auto-hscroll-margin.

The point will still moved according to scroll-margin due to mouse-1,
but now it's done only when releasing the mouse button. Simply clicking
inside the scroll-margin region now behaves as expected.

This partially addresses Bug#42150.

Clicking inside the scroll-margin region still occasionally causes some
regions to be selected, although it doesn't cause the buffer to scroll
uncontrollably anymore.
---
 lisp/mouse.el | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lisp/mouse.el b/lisp/mouse.el
index 9f1417f42..a6c999594 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -1578,8 +1578,7 @@ mouse-drag-track
   (mouse-minibuffer-check start-event)
   (setq mouse-selection-click-count-buffer (current-buffer))
   (deactivate-mark)
-  (let* ((scroll-margin 0) ; Avoid margin scrolling (Bug#9541).
-	 (start-posn (event-start start-event))
+  (let* ((start-posn (event-start start-event))
 	 (start-point (posn-point start-posn))
 	 (start-window (posn-window start-posn))
 	 (_ (with-current-buffer (window-buffer start-window)
@@ -1601,12 +1600,20 @@ mouse-drag-track
 		   ;; Don't count the mode line.
 		   (1- (nth 3 bounds))))
 	 (click-count (1- (event-click-count start-event)))
-	 ;; Suppress automatic hscrolling, because that is a nuisance
-	 ;; when setting point near the right fringe (but see below).
+         ;; Save original automatic scrolling behavior (see below).
 	 (auto-hscroll-mode-saved auto-hscroll-mode)
+	 (scroll-margin-saved scroll-margin)
          (old-track-mouse track-mouse))
 
     (setq mouse-selection-click-count click-count)
+
+    ;; Suppress automatic scrolling near the edges while tracking
+    ;; movement, as it interferes with the natural dragging behavior
+    ;; (point will unexpectedly be moved beneath the pointer, making
+    ;; selections in auto-scrolling margins impossible).
+    (setq auto-hscroll-mode nil)
+    (setq scroll-margin 0)
+
     ;; In case the down click is in the middle of some intangible text,
     ;; use the end of that text, and put it in START-POINT.
     (if (< (point) start-point)
@@ -1625,7 +1632,6 @@ mouse-drag-track
 
     (setf (terminal-parameter nil 'mouse-drag-start) start-event)
     (setq track-mouse t)
-    (setq auto-hscroll-mode nil)
 
     (set-transient-map
      (let ((map (make-sparse-keymap)))
@@ -1636,8 +1642,6 @@ mouse-drag-track
            (let* ((end (event-end event))
                   (end-point (posn-point end)))
              (unless (eq end-point start-point)
-               ;; As soon as the user moves, we can re-enable auto-hscroll.
-               (setq auto-hscroll-mode auto-hscroll-mode-saved)
                ;; And remember that we have moved, so mouse-set-region can know
                ;; its event is really a drag event.
                (setcar start-event 'mouse-movement))
@@ -1658,6 +1662,7 @@ mouse-drag-track
      t (lambda ()
          (setq track-mouse old-track-mouse)
          (setq auto-hscroll-mode auto-hscroll-mode-saved)
+         (setq scroll-margin scroll-margin-saved)
          ;; Don't deactivate the mark when the context menu was invoked
          ;; by down-mouse-3 immediately after down-mouse-1 and without
          ;; releasing the mouse button with mouse-1. This allows to use
-- 
2.33.0

Re: [PATCH] Improve interaction between mouse-drag-region and scroll-margin

[Eli Zaretskii]

> From: Yuri D'Elia <wavexx@thregr.org>
> Date: Sat, 25 Sep 2021 18:09:22 +0200
> Cc: Yuri D'Elia <wavexx@thregr.org>
> 
> * lisp/mouse.el (mouse-drag-track): Disable both scroll-margin and
> auto-hscroll-mode in mouse-drag-region and do not re-enable them until
> dragging is over, making selections work as expected when inside the
> margins.

If you disable auto-hscroll-mode, why do you also need to zero out
hscroll-margin?

> +    ;; Suppress automatic scrolling near the edges while tracking
> +    ;; movement, as it interferes with the natural dragging behavior
> +    ;; (point will unexpectedly be moved beneath the pointer, making
> +    ;; selections in auto-scrolling margins impossible).
> +    (setq auto-hscroll-mode nil)
> +    (setq scroll-margin 0)

Shouldn't this be in unwind-protect?  What happens if Emacs signals an
error or there's a quit while in this code?

And I think this also warrants a NEWS entry.

Thanks.

Re: [PATCH] Improve interaction between mouse-drag-region and scroll-margin

[Yuri D'Elia]

On Sat, Sep 25 2021, Eli Zaretskii wrote:
>> auto-hscroll-mode in mouse-drag-region and do not re-enable them until
>> dragging is over, making selections work as expected when inside the
>> margins.
>
> If you disable auto-hscroll-mode, why do you also need to zero out
> hscroll-margin?

It doesn't, it just disables auto-hscroll-mode. Maybe I should reword
the commit.

>> +    ;; Suppress automatic scrolling near the edges while tracking
>> +    ;; movement, as it interferes with the natural dragging behavior
>> +    ;; (point will unexpectedly be moved beneath the pointer, making
>> +    ;; selections in auto-scrolling margins impossible).
>> +    (setq auto-hscroll-mode nil)
>> +    (setq scroll-margin 0)
>
> Shouldn't this be in unwind-protect?

I guess, yes.

There's probably more side-effect that would be need to be
taken-care of inside that big let* (looking at that mouse-set-point...).

> And I think this also warrants a NEWS entry.

Ok

Re: [PATCH] Improve interaction between mouse-drag-region and scroll-margin

[Eli Zaretskii]

> From: Yuri D'Elia <wavexx@thregr.org>
> Date: Sat, 25 Sep 2021 18:31:40 +0200
> 
> On Sat, Sep 25 2021, Eli Zaretskii wrote:
> >> auto-hscroll-mode in mouse-drag-region and do not re-enable them until
> >> dragging is over, making selections work as expected when inside the
> >> margins.
> >
> > If you disable auto-hscroll-mode, why do you also need to zero out
> > hscroll-margin?
> 
> It doesn't, it just disables auto-hscroll-mode.

Sorry, I don't understand "it doesn't" what?

Don't you want to disable the automatic horizontal scrolling of the
window when point gets close to the window edges?  If so, disabling
auto-hscroll-mode should do just that, regardless of the value of
hscroll-margin.  Or what am I missing?

Re: [PATCH] Improve interaction between mouse-drag-region and scroll-margin

[Yuri D'Elia]

On Sat, Sep 25 2021, Eli Zaretskii wrote:
>> It doesn't, it just disables auto-hscroll-mode.
>
> Sorry, I don't understand "it doesn't" what?
>
> Don't you want to disable the automatic horizontal scrolling of the
> window when point gets close to the window edges?  If so, disabling
> auto-hscroll-mode should do just that, regardless of the value of
> hscroll-margin.  Or what am I missing?

We don't touch hscroll-margin directly, we just disable the mode.
The comment was misleading.

Re: [PATCH] Improve interaction between mouse-drag-region and scroll-margin

[Eli Zaretskii]

> From: Yuri D'Elia <wavexx@thregr.org>
> Date: Sat, 25 Sep 2021 19:03:59 +0200
> 
> We don't touch hscroll-margin directly, we just disable the mode.
> The comment was misleading.

???  Then what is this part of the patch about:

>> +    (setq scroll-margin 0)

Re: [PATCH] Improve interaction between mouse-drag-region and scroll-margin

[Yuri D'Elia]

On Sat, Sep 25 2021, Eli Zaretskii wrote:
>> From: Yuri D'Elia <wavexx@thregr.org>
>> Date: Sat, 25 Sep 2021 19:03:59 +0200
>> 
>> We don't touch hscroll-margin directly, we just disable the mode.
>> The comment was misleading.
>
> ???  Then what is this part of the patch about:
>
>>> +    (setq scroll-margin 0)

We're handling two independent auto-scrolling settings.

One is scroll-margin which does automatic vertical scrolling: there's no
mode for it.

But we previously (before this patch) also handled auto-hscroll-mode,
which does horizontal autoscroll.