justbur / emacs-vdiff

Like vimdiff for Emacs
GNU General Public License v3.0
213 stars 17 forks source link

Lock scrolling is not working when the cursor is not on the scrolled buffer #29

Open otadmor opened 3 years ago

otadmor commented 3 years ago

How to reproduce: 1) start emacs within terminal 2) do M-x xterm-mouse-mode 3) M-x eval-expression (setq vdiff-lock-scrolling t) 4) do M-x vdiff-files with two files (the amount of lines / difference between them dont seem to matter). 5) place the cursor on the left buffer 6) hover the mouse above the right buffer 7) scroll the right buffer with the mouse wheel, keeping the cursor on the left buffer and the mouse-cursor on the right buffer

What happens? 1) only the right buffer scrolls

What should happen? 1) both of the buffers should scroll

Notes: 1) Running from X (and not the terminal) have the same behavior. 2) Also happens when the cursor is on the right buffer and the mouse scrolls on the left one.

justbur commented 3 years ago

It looks like this is a limitation of emacs, and I'm not sure how to fix it. vdiff uses the window-scroll-functions hook to determine when to update the buffers, and this hook supposedly fires when window-start is updated in a buffer, but it seems that scrolling with a mouse does not have this effect and I'm not sure why.

To be honest, synchronizing the scrolling was the most difficult aspect of getting vdiff to work, and I never found a perfect way to do it. I would very much take any help that anyone would be willing to provide.

billsacks commented 2 years ago

As a workaround for this issue, I have put the following in my emacs customizations:

;; In vdiff-mode, the synchronized scrolling doesn't work if you try to scroll the
;; non-currently-active window (see also
;; https://github.com/justbur/emacs-vdiff/issues/29). So force mouse-based scrolling to
;; always scroll the active window in vdiff-mode. (I thought we would be able to do this
;; by setting mouse-wheel-follow-mouse, but that doesn't seem to have any effect - maybe a
;; bug?)
(defun my-vdiff-mouse-wheel--get-scroll-window (orig-fun &rest args)
  (if (bound-and-true-p vdiff-mode)
      (selected-window)
    (apply orig-fun args)))
(advice-add 'mouse-wheel--get-scroll-window :around #'my-vdiff-mouse-wheel--get-scroll-window)

Basically, this bypasses the function for choosing which window to scroll when we're in vdiff-mode, instead always choosing the active window. This feels like reasonable behavior when we're in vdiff-mode. (As noted in the comment, I first tried to do this by setting mouse-wheel-follow-mouse, but setting that to nil doesn't seem to work, at least on my Mac with emacs28. So I was forced to override the entire relevant function via advice.)

Slightly off-topic but slightly related: I also added the following to make scrolling work better:

(defun my-vdiff-scroll-up-command (arg)
  "Version of scroll-up-command for use in vdiff-mode

This generally works the same as scroll-up-command, but if we
can't scroll the current window, then we switch to the other
window before scrolling up.

This is needed in vdiff mode when the top of a buffer contains
virtual removed lines: trying to scroll that buffer by small
amounts doesn't work.

Note that this can lead to the other window becoming active."
  (let ((orig-window-start (window-start)))
    (scroll-up-command arg)
    (when (eq (window-start) orig-window-start)
      ;; the scroll was unsuccessful; switch to the other window and try scrolling there

      ;; this assumes that (other-window 1) gives the other window associated with
      ;; this vdiff session
      (other-window 1)
      (scroll-up-command arg))))
(defun my-maybe-vdiff-scroll-up-command (arg)
  "Generally works the same as scroll-up-command, but in vdiff mode
this might switch to the other buffer before scrolling up."
  (if (bound-and-true-p vdiff-mode)
      (my-vdiff-scroll-up-command arg)
    (scroll-up-command arg)))

(setq mwheel-scroll-up-function 'my-maybe-vdiff-scroll-up-command)

The motivation for this was: I found that, if the cursor is in a window where there are a few removed lines at the top of the window, scrolling gets stuck if you try to scroll by small amounts. I'm not sure if there's a way to fix this elegantly within vdiff itself (my elisp isn't that great), but this works around the issue in a slightly hacky way: If we try to scroll but find that nothing happened, then we take that as an indication that scrolling has gotten stuck like this. In that case, we switch to the other window (assuming that (other-window 1) always gives the other window in this vdiff session, which might not be a great assumption but works for my workflow) and try the scroll command again from there. This potentially leads to the cursor bouncing back and forth between the buffers as you scroll, but I feel like this is an acceptable trade-off for being able to scroll continuously without scrolling getting stuck. This second set of changes made the modification to mouse-wheel--get-scroll-window even more important, though: for mouse-based scrolling to work smoothly, both of these need to be done together.

I should note that I have only been using vdiff for a couple of days, so I don't yet have extensive experience with these changes, but so far they are making scrolling through diffs feel pretty smooth.

Thank you very much for your work on this vdiff package! Compared to ediff, I really like (1) the better alignment of diff regions, which stay aligned as you scroll; and (2) the highlighting of all fine diffs, rather than just those of the current region. I find that these two things make it easier to quickly review diffs.