Scrolling with evil-visual-type set to "line" causes point motion to fail

[ideasman42]

Bug report

Using 'line' selection prevents emacs built in scrolling commands from reposition the window, jumping back to the location instead of scrolling.

This report includes scroll commands that work properly except when evil-modes visual line selection is enabled.

Environment

Emacs version: 29.0.50 Operating System: Linux, 6.0.2-arch1-1 Evil version: 1.15.0 Evil installation type: MELPA Graphical/Terminal: Wayland Tested in a make emacs session (see CONTRIBUTING.md): Yes

Reproduction steps

• Start Emacs
• Paste and evaluate this text.

(defun my-scroll-and-clamp--forward-line (n)
  "Wrap `forward-line', supporting Emacs built-in goal column.
Argument N the number of lines, passed to `forward-line'."
  (let ((next-column
         (or goal-column
             (and (memq last-command
                        '(next-line previous-line line-move))
                  (if (consp temporary-goal-column)
                      (car temporary-goal-column)
                    temporary-goal-column)))))
    (unless next-column
      (setq temporary-goal-column (current-column))
      (setq next-column temporary-goal-column))
    (forward-line n)
    (move-to-column next-column))
  ;; Needed so `temporary-goal-column' is respected in the future.
  (setq this-command 'line-move))

(defun my-scroll-and-clamp--evil-visual-mode-workaround ()
  "Workaround for `evil-mode' line-mode."
  ;; Without this, the line mode point jumps back to the origin,
  ;; the mark needs to be set to the `point'.
  (when (and (fboundp 'evil-visual-state-p)
             (funcall 'evil-visual-state-p)
             (fboundp 'evil-visual-type)
             (eq (funcall 'evil-visual-type) 'line)
             (boundp 'evil-visual-point))
    (let ((mark (symbol-value 'evil-visual-point)))
      (when (markerp mark)
        (set-marker mark (point))))))

(defun my-scroll-and-clamp-up-command ()
  (interactive)
  (let ((height (window-height)))

    ;; Move point.
    (my-scroll-and-clamp--forward-line height)

    ;; Move window.
    (set-window-start
      (selected-window)
      (min
        (save-excursion ;; new point.
          (forward-line (- (/ height 2)))
          (point))
        (save-excursion ;; max point.
          (goto-char (point-max))
          (beginning-of-line)
          (forward-line (- (- height (+ 1 (* 2 scroll-margin)))))
          (point)))))
  ;; (my-scroll-and-clamp--evil-visual-mode-workaround)
  (redisplay))

(defun my-scroll-and-clamp-down-command ()
  (interactive)
  (let* ((height (window-height)))

    ;; Move point.
    (my-scroll-and-clamp--forward-line (- height))
    (setq this-command 'line-move)

    ;; Move window.
    (set-window-start
      (selected-window)
      (save-excursion ;; new point.
        (forward-line (- (/ height 2)))
        (point))))
  ;; (my-scroll-and-clamp--evil-visual-mode-workaround)
  (redisplay))

(global-set-key (kbd "C-M-k") 'my-scroll-and-clamp-down-command)
(global-set-key (kbd "C-M-j") 'my-scroll-and-clamp-up-command)

• Make the window height 20 lines high.
• Press C-M-j 4 times, C-M-k, 4 times.
• Notice this performs a kind of page-up/down operations (as expected).
• Press S-v (to enter visual line mode)
• Press C-M-j 4 times, C-M-k, 4 times.
• Notice this DOES NOT perform page-up/down operations.

Expected behavior

• C-M-j / C-M-k - should scroll down/up both in normal mode and visual line mode.

Actual behavior

• When in visual line mode, the scroll operations are ignored (the start point of visual line mode locks scrolling).

Further notes

• Un-comment ;; (my-scroll-and-clamp--evil-visual-mode-workaround) and re-evaluating the code works around the problem by setting evil-visual-point.
• This bug is spesific to (evil-visual-type) being set to 'line and doesn't occur for character level selection.
• This also impacts packages:
  • https://codeberg.org/ideasman42/emacs-scroll-on-drag
  • https://codeberg.org/ideasman42/emacs-scroll-on-jump

[tomdl89]

Thanks for the bug report. Seeing as you're no stranger to elisp yourself, I'd encourage you to submit a PR if you have the time/inclination. I'm happy to help with questions if need be.

[ideasman42]

@tomdl89 I've looked into this a bit further and the current behavior seems to be working as intended.

From what I can tell evil's visual-line-mode pre/post hooks assume that the action being performed is a text editing operation, rather than navigation - so it sets the point accordingly.

For navigation - clamping the point to line bounds doesn't make sense, so it would be good if there was a generic way for navigation commands to support this:

• Without having to depend directly on evil mode.
• Without having to declare a property on the command (in my use case at least).

An example of why this is needed in my case: I maintain a package called scroll-on-drag which defines a macro scroll-on-drag-with-fallback, in this case I can't set the command property as that will be part of a function defined by the user. So I'm forced to manipulate evil visual properties to support this in a portable way.

The same goes for my scroll-on-jump package.

I ended up writing macros to handle this in a more generic way:

(defun scroll-on-drag--evil-visual-line-data ()
  "Return data associated with visual line mode or nil when none is found."
  ;; The checks are written so as not to require evil mode as a dependency.
  (when
    (and
      (fboundp 'evil-visual-state-p)
      (funcall 'evil-visual-state-p)
      (fboundp 'evil-visual-type)
      (eq (funcall 'evil-visual-type) 'line)
      (boundp 'evil-visual-point))
    (let ((mark (symbol-value 'evil-visual-point)))
      (when (markerp mark)
        mark))))

(defmacro scroll-on-drag--with-evil-visual-mode-hack (visual-line-data &rest body)
  "Execute BODY without evil-visual-mode line constraints.
Run when MARK is non-nil.
VISUAL-LINE-DATA is the result of `scroll-on-drag--evil-visual-line-data'."
  `
  (unwind-protect
    (progn
      (goto-char (marker-position ,visual-line-data))
      ,@body)
    (set-marker ,visual-line-data (point))))