fix(pdf-roll): send a negative scroll argument the other way, once

[alberti42]

What happens

pdf-roll-scroll-forward and pdf-roll-scroll-backward both mishandle a negative argument. Starting from page 4 at vscroll 500, with frame-char-height 23, on a 16 page document:

call	before	after	expected
(pdf-roll-scroll-forward -3 win)	3144	431	431, i.e. (pdf-roll-scroll-backward 3 win)
(pdf-roll-scroll-backward -3 win)	3282	569	569, i.e. (pdf-roll-scroll-forward 3 win)

Reachable interactively as C-u -3 C-n or C-u -3 <down>, since pdf-view-roll-minor-mode remaps the line commands onto these two, and through pdf-roll-scroll-screen-forward and pdf-roll-scroll-screen-backward with a negative prefix, which pass a negative pixel count straight down.

Why

Both functions start like this:

(setq n (* (or n 1) (if pixels 1 (frame-char-height))))
(setq window (or window (selected-window)))
(when (> 0 n) (pdf-roll-scroll-backward (- n) window))

which is wrong in three ways.

The when does not return, so after delegating, the body runs as well and the window is scrolled a second time in the original direction.

n has already been multiplied by frame-char-height when the call is made, and pixels is not passed on, so the callee multiplies it again. That is where 3144 comes from rather than 431: the larger the pages, the further it goes.

And in pdf-roll-scroll-backward the call goes to pdf-roll-scroll-backward — the direction it was asked to reverse.

The fix

Dispatch on the sign before scaling, and return:

(setq n (or n 1))
(setq window (or window (selected-window)))
(if (> 0 n)
    (pdf-roll-scroll-backward (- n) window pixels)
  (setq n (* n (if pixels 1 (frame-char-height))))
  ...)

This is the same shape as the dispatch in #338, so it should not get in the way there. The rest of both functions is unchanged; the diff looks larger than it is because the body moves into the else branch.

Priority

Lower than #361, #362 and #364 — it needs a deliberate negative prefix argument, so it is not something a normal binding reaches.

Conflict

Conflicts with #364 in lisp/pdf-roll.el: that PR changes the line immediately after this one in both functions. I am happy to rebase this on top of whichever of the two you take first — the resolution is to keep #364's starting position inside this if.