Feature Request: support scroll-margin>0

[psionic-k]

Frankly can't stand the herky jerk of opening the mini-buffer with packages like vertico, ivy, transient etc. I had used another solution for this, but it relies on scroll margin, and scroll margin really plays badly with pixel precision scrolling. It rubber bands like an idiot.

To kill this menace once and for all, I'm developing a window point solution that will push the window points on certain entry hooks and restore them afterward if they were pushed. Here's where I got so far:

;; Eliminate stupid window movements caused by minibuffer or transient opening
;; and closing.
(defcustom pmx-no-herky-jerk-margin 12
  "Number of lines to protect from incidental scrolling.
A good value is the maximum height of your minibuffer, such as
configured by `ivy-height' and similar variables that configure packages
like `vertico' and `helm'."
  :type 'integer
  :group 'scrolling)

;; You would think we need multiple restore points.  However, there seems to be
;; a behavior where window points in non-selected windows are restored all the
;; time.  This was only apparent after moving them.
(defvar pmx--no-herky-jerk-restore nil
  "Where to restore selected buffer point.
List of BUFFER WINDOW SAFE-MARKER and RESTORE-MARKER.")

;; Counting line height would be more correct.  In general, lines are taller but
;; not shorter than the default, so this is a conservative approximation that
;; treats all lines as the default height.
(defun pmx--no-herky-jerk-enter (&rest _)
  "Adjust window points to prevent implicit scrolling."
  (unless (> (minibuffer-depth) 1)
    (let ((windows (window-at-side-list
                    (window-frame (selected-window))
                    'bottom))
          ;; height of default lines
          (frame-char-height (frame-char-height
                              (window-frame (selected-window)))))
      (while-let ((w (pop windows)))
        (with-current-buffer (window-buffer w)
          (let* ((current-line (line-number-at-pos (window-point w)))
                 (end-line (line-number-at-pos (window-end w)))
                 (window-pixel-height (window-pixel-height w))
                 (window-used-height (cdr (window-text-pixel-size
                                           w (window-start w) (window-end w))))
                 (margin-height (* frame-char-height pmx-no-herky-jerk-margin))
                 (unsafe-height (- window-used-height
                                   (- window-pixel-height margin-height)))
                 (unsafe-lines (+ 2 (ceiling (/ unsafe-height frame-char-height))))
                 (exceeded-lines (- unsafe-lines (- end-line current-line))))
            (when (> exceeded-lines 0)
              ;;  save value for restore
              (let* ((buffer (window-buffer w))
                     (restore-marker (let ((marker (make-marker)))
                                       ;; XXX this may error?
                                       (set-marker marker (window-point w)
                                                   buffer)))
                     (safe-point (progn
                                   (goto-char restore-marker)
                                   ;; XXX goes up too many lines when skipping
                                   ;; wrapped lines
                                   (ignore-error '(beginning-of-buffer
                                                   end-of-buffer)
                                     (previous-line exceeded-lines t))
                                   (end-of-line)
                                   (point))))
                (set-window-point w safe-point)
                (when (eq w (minibuffer-selected-window))
                  (let ((safe-marker (make-marker)))
                    (set-marker safe-marker safe-point buffer)
                    (setq pmx--no-herky-jerk-restore
                          (list buffer w safe-marker restore-marker))))
                (goto-char (marker-position restore-marker))))))))))

(defun pmx--no-herky-jerk-exit ()
  "Restore window points that were rescued from implicit scrolling."
  (when (and pmx--no-herky-jerk-restore
             (= (minibuffer-depth) 1)
             (null (transient-active-prefix)))
    (when-let* ((restore pmx--no-herky-jerk-restore)
                (buffer (pop restore))
                (w (pop restore))
                (safe-marker (pop restore))
                (restore-marker (pop restore)))
      (when (and (window-live-p w)
                 (eq (window-buffer w) buffer)
                 (= (window-point w) (marker-position safe-marker)))
        (goto-char restore-marker)
        (set-window-point w restore-marker))
      (set-marker restore-marker nil)
      (set-marker safe-marker nil)
      (setq pmx--no-herky-jerk-restore nil))))

(add-hook 'minibuffer-setup-hook #'pmx--no-herky-jerk-enter)
(add-hook 'minibuffer-exit-hook #'pmx--no-herky-jerk-exit)

;; Add the same for transient
(with-eval-after-load 'transient
  (advice-add 'transient-setup :before #'pmx--no-herky-jerk-enter)
  (add-hook 'transient-exit-hook #'pmx--no-herky-jerk-exit)
  (setopt transient-hide-during-minibuffer-read t))

edit: added a skip for the minibuffer, side detection, and text-area based logic. Discovered that non-selected windows already restore, which was unexpected. Only the selected window restores.

This alleviates the need for scroll margin. But scroll margin is kind of nice. I hate having to bring lines into context manually after the point goes beyond certain bounds. I figure if I can use window point manipulation to decapitate the beast of pure evil that is minibuffer-induced window scrolling, a smarter scroll margin that works with precision scrolling should be feasible?

[jdtsmith]

See the details, and also #3 where I mentioned:

scroll-margin>0 is highly verboten

The reason why is you need to be able to place point at the window boundaries reliably and then keep emacs from popping it back. One idea I've thought of to relax this constraint is to temporarily set scroll-margin to 0 "during scrolls", then reset it after. Sadly there's no good cross-platform way to know when a "scroll has ended". You also have created another problem, in that you need to leave point on a position at least scroll-margin away from the top and bottom, or it will jump.

For simple equal height lines, it "should" be possible to keep point scroll-margin lines away from the top/bottom during the scroll; that's clearly inside the window so should work out. This is all fine and well, but now consider jumbo lines (tall images, taller than the window height). Now you have the ambiguity of questions like "where is point when you have 2 lines of scroll margin top and bottom in a 3 line tall window?". There are answers to such questions, but they'd have to be uncovered (and proven to be equivalent across builds).

So in principle pixel scrolling could be made compatible with scroll-margin, but it will take a lot of experimenting. Maybe it could be made to work.

Not sure I understand your point about vertico; is it that point gets pushed up when the bottom of the buffer is hidden by the mini-buffer? I think that's unavoidable and is just a mini-buffer issue, though I guess if you had a lot of scroll margin you could prevent any point movement.

[psionic-k]

Not sure I understand your point about vertico; is it that point gets pushed up when the bottom of the buffer is hidden by the mini-buffer? I think that's unavoidable and is just a mini-buffer issue, though I guess if you had a lot of scroll margin you could prevent any point movement.

Yes. The code above mostly fixes the issue. There is a remaining detail where the selected window does not get its point restored. I figure the end of the command loop is clobbering the restoration but Idk yet.

I think what feels right for this package is to leave scroll-margin zero and instead offer a separate custom variable. To implement, just bump the window point up or down ahead of scrolling. If it's not fighting with the silly scroll-margin behavior, it should be a lot easier.

[jdtsmith]

just bump the window point up or down ahead of scrolling

You really wouldn't believe what a world of pain you enter with such a simple incantation, such is the rube-goldbergian system of knobs and levers that determines what gets displayed in a window. It's all about the corner cases: large line heights at 1/4, 1/2, 1x, 2x window height.

I will leave this open in hopes that I or someone else can find some time to experiment with robust support for scroll-margin>0. If it worked out (and based on all my experiments thus far, I have limited confidence it would), ultra-scroll can just examine scroll-margin and "try" to keep point that far away from top/bottom boundaries while scrolling (doing "something" when this is impossible, e.g. due to jumbo lines), so that redisplay never has a chance to interfere.

I wish there were a way to temporarily disable all the boundless redisplay code that re-centers or "moves point back on screen", but alas, redisplay insists on doing that right in the middle of your scrolling commands.

[jdtsmith]

If someone wants to play around with cursor movement (just line by line, not pixel scrolling) when (< (window-height) (* 2 scroll-margin)) (e.g. with a tall line showing images) and report their findings, that would be a start.

[AjaiKN]

In case it provides inspiration to anyone, here's a very hacky solution I've been using for a while to temporarily set scroll-margin>0 and scroll-conservatively<101 while running certain commands.

Code

;; Scrolling conservatively means it won't recenter as much, which speeds up smooth scrolling a lot.
;; But, for example, when I'm incrementally searching, I want it to recenter, so I temporarily change it back.
(defun akn/do-scroll-conservatively (&rest _)
  (setq scroll-conservatively 101
        scroll-margin 0))
(defun akn/scroll-progressively (&rest _)
  (setq scroll-conservatively 10
        scroll-margin 10))
(defun akn/do-scroll-conservatively-unless-minibuffer (&rest _)
  (unless (when-let ((w (minibuffer-window))) (minibuffer-window-active-p w))
    (akn/do-scroll-conservatively)))
(defun akn/scroll-conservatively (&rest _)
  (run-with-idle-timer 0.1 nil
                       #'akn/do-scroll-conservatively-unless-minibuffer))

(akn/do-scroll-conservatively)
(ultra-scroll-mode 1)

(add-hook 'minibuffer-setup-hook #'akn/scroll-progressively)
(add-hook 'isearch-mode-hook #'akn/scroll-progressively)
(add-hook 'minibuffer-exit-hook #'akn/scroll-conservatively)
(add-hook 'isearch-mode-end-hook #'akn/scroll-conservatively)
(dolist (fn '(evil-ex-search evil-search
              (evil-ex-start-search . evil-ex-search-abort)
              isearch-printing-char
              (nil . evil-ex-search-exit)
              (evil-ex-search-forward . nil) (evil-search-forward . nil)
              (evil-ex-search-backward . nil) (evil-search-backward . nil)
              (nil . abort-recursive-edit)
              flycheck-next-error
              flycheck-previous-error
              flycheck-error-list-next-error
              flycheck-error-list-previous-error
              better-jumper-jump-forward
              better-jumper-jump-backward
              better-jumper-jump-newest
              akn/hard-reload-buffer
              akn/reload-buffer
              +fold/overview
              +fold/table-of-contents
              +fold/unfold-all-headings
              +fold/drag-stuff-down
              +fold/drag-stuff-up
              +fold/outline-cycle-all
              +fold/outline-cycle-all-simple
              +fold/next
              +fold/previous
              +fold/toggle
              +fold/open
              +fold/open-all
              +fold/close-all
              +fold/all-headings
              org-cycle
              org-shifttab
              find-file
              +lookup/definition
              visible-mode
              diff-hl-next-hunk
              smerge-next
              smerge-vc-next-conflict
              smerge-prev
              evil-forward-section-begin evil-forward-section-end
              evil-backward-section-begin evil-backward-section-end
              next-error previous-error
              +evil/next-beginning-of-method +evil/next-end-of-method
              ;; goto-line goto-char
              goto-last-change goto-last-change-reverse
              evil-goto-line evil-goto-char evil-goto-error evil-goto-first-line
              evil-goto-last-change evil-goto-last-change-reverse
              evil-goto-mark evil-goto-mark-line))
  (when (not (listp fn))
    (setq fn (cons fn fn)))
  (when (car fn)
    (advice-add (car fn) :before #'akn/scroll-progressively))
  (when (cdr fn)
    (advice-add (cdr fn) :after #'akn/scroll-conservatively)))

[jdtsmith]

Thanks, that's an interesting idea: leave it off except during specific non-scrolling commands. The main issue with this approach is that the scroll/re-centering settings come into effect only on redisplay, not while the code which moves point operates. I see you are trying to work around that by putting it in an idle-timer. Have you tried it with line-move? I'd suggest you save the idle timer you set up in akn/scroll-conservatively to a global variable, and there only create a new timer if one doesn't already exist. That way if you hit a bunch of searches in quick succession, you don't accumulate a bunch of timers all about to do the same thing.

What's the reason for avoiding the mini-buffer, btw?

[AjaiKN]

Those are some good ideas, and I'll try them when I get a chance. (I wrote that code a while ago and haven't changed it, since it's been working well enough for me.)

What's the reason for avoiding the mini-buffer, btw?

For me, the minibuffer being open is just a reasonable (but imperfect) proxy for times when I want "progressive" scrolling. When the minibuffer is open, that's often because I'm using a searching command like find-file, consult-line, or consult-ripgrep. The search preview buffer jumps around as I'm typing, and I want to see a few lines of context.