Re: Pixel scrolling support

[Eli Zaretskii <eliz@gnu.org>]

> From: Po Lu <luangruo@yahoo.com>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Fri, 26 Nov 2021 15:01:57 +0800
> 
> > Separate mode should be fine, but we need a good name for it ("better
> > pixel scroll" is not a good name).
> 
> Thanks, how about `pixel-scroll-precise-mode'?

pixel-scroll-precision-mode sounds better to me.

> -** New minor mode 'better-pixel-scroll-mode'.
> +** New minor mode 'pixel-scroll-precise-mode'.
>  When enabled, using this mode with a capable scroll wheel will result
>  in the display being scrolled precisely according to the turning of
>  that wheel.

This text doesn't really describe what the mode does.  It basically
says something like "pixel-scroll-precise-mode scrolls precisely".
the main part that was left unexplained is "according to the turning of
that wheel", and specifically the "according" part.

> +(defun pixel-scroll-precise-scroll-down (delta)
> +  "Scroll the current window down by DELTA pixels.
> +Note that this function doesn't work if DELTA is larger than
> +the height of the current window."

What is the problem with scrolling by more than the window's height?

> +	  (when (eobp)
> +	    (error "End of buffer")))

I think you should detect the EOB condition early on and simply not
scroll at all in that case.

> +(defun pixel-scroll-precise (event &optional arg)
> +  "Scroll the display according to EVENT.

This sentence should include something to indicate the "precise"
feature.  Otherwise it is too general, indistinguishable from any
other scroll command.

> +Take into account any pixel deltas in EVENT to scroll the display
> +according to the user's turning the mouse wheel.  If EVENT does
> +not have precise scrolling deltas, call `mwheel-scroll' instead.

This describes what the code does, not what the user should expect in
terms of the effect on the screen.

> +ARG is passed to `mwheel-scroll', should that be called."

Likewise: the description of ARG should be similar to how we describe
the effect of prefix arg in other similar commands.

> +;;;###autoload
> +(define-minor-mode pixel-scroll-precise-mode
> +  "Toggle pixel scrolling.
> +When enabled, this minor mode allows to scroll the display
> +precisely, according to the turning of the mouse wheel."

Shouldn't this say something about "setting the variable doesn't have
any effect"?

And what about horizontal scrolling?

Thanks.