doomemacs

doomemacs copied to clipboard

Fixes #5871

I added a module under ui for providing the good-scroll package. I also activate the minor more in the autoload file

This is the first time I contribute, so there is probably some mistakes that I've done involuntarly, I also don't know how to test out my changes locally. As it's a very small module I thought that it would still be safe to propose the changes and get some pointers on documentation that I should take a look at.

I have some suggestions.

1. In emacs-29 Po Lu introduced pixel-scroll-precision-mode, emacs can support scroll in pixel natively.

2. And in emacs-mac, there is a variable mac-mouse-wheel-smooth-scroll which can support pixel scroll natively, too.

In those situations, there's no need to introduce good-scroll or iscroll to support image/text smooth scroll.

Thanks for the feedback! And nice to hear that work is being done by the emacs contributors directly! I see commits from Po Lu on this mode from 2h/3h ago, do you know what the timeframe is for this feature or when emacs 29 will be released? I don't know anything about Emacs' release schedule.

If it's a bit far in the future, the module can probably be added as a solution to the problem right now, and then later, when there is no use for it anymore (i.e, the main version is Emacs 29), then we can delete the module. Another way would be to change it's behavior once Emacs 29 is the new default. I honestly don't know what is the best solution in this kind of situation, but waiting for Emacs 29 seems to be a long wait.

Thanks for the feedback! And nice to hear that work is being done by the emacs contributors directly!

I see commits from Po Lu on this mode from 2h/3h ago, do you know what the timeframe is for this feature or when emacs 29 will be released? I don't know anything about Emacs' release schedule.

If it's a bit far in the future, the module can probably be added as a solution to the problem right now, and then later, when there is no use for it anymore (i.e, the main version is Emacs 29), then we can delete the module.

Another way would be to change it's behavior once Emacs 29 is the new default.

I honestly don't know what is the best solution in this kind of situation, but waiting for Emacs 29 seems to be a long wait.

Well, I don't know the timeline either. I meant you could add some "if" in your commit. For example, if emacs version greater than 29, or if it's emacs-mac, you can use those implemented methods instead of good-scroll and iscroll(if you wonna better image scroll in org-mode). Those mode doesn't enabled as default. After that doom users can experience as same if they enable your module, no matter which emacs version

If you don't mind, I think following can work? But I wrote ugly. It's depend on you whether use it or not.

package.el

(when (not (and (emacs-major-version > 28)  (boundp mac-mouse-wheel-smooth-scroll)))
  (package! good-scroll))

config.el

(if (boundp mac-mouse-wheel-smooth-scroll)
    (setq  mac-mouse-wheel-smooth-scroll t)
  (if (emacs-major-version > 28)
       (pixel-scroll-precision-mode)
   (progn
      (use-package! good-scroll)))))

Oh woaw! that seems super nice, I'm not familiar with elisp so all this is new to me :sweat_smile: I'll commit the change. Also, do you happen to know if there is a simple way to test a module? Or should I simply use my fork as my .emacs.d? I'll do that for now but don't really know if that's the correct way to do things basically :wink:

Oh woaw! that seems super nice, I'm not familiar with elisp so all this is new to me :sweat_smile:

I'll commit the change. Also, do you happen to know if there is a simple way to test a module?

Or should I simply use my fork as my .emacs.d?

I'll do that for now but don't really know if that's the correct way to do things basically :wink:

I think you can try it as your private module? Put it to .doom.d/modules/ui/goodscroll

If it's going to just enable pixel-scrolling via a variety of methods, perhaps the module should just be pixel-scroll?

Sorry for the delay, yeah I'll probably rename the module ;)

I tested the module and fixed a bunch of things, I also added your two suggestions ;) Now it works really well! thanks, I learned a lot :smile:

Hi, @Narice!

Fantastic addition! I've just tested your changes, and the module works really nice on my setup. It uses CPU like a hog, but I guess that can't be helped until the native solution for emacs 29 that @ErnestDong mentioned is generally available. Nevertheless, a mention to the fact that the CPU usage will increase somewhat would perhaps be a nice touch to the doc.

I reckon there is still a tiny change missing, which is to add a line under the :ui section of init.example.el with good-scroll plus the customary witty comment (ostensibly, the hardest part to write ;-P)

Thanks a bunch!

Perhaps

       :ui
       ;; ...
       ophints           ; highlight the region an operation acts on
       ;;pixel-scroll      ; buttery buffers
       (popup +defaults)   ; tame sudden yet inevitable temporary windows

? 🧈

Although it looks like most of the ui modules eschew the wit in favor of a more literal tagline, so we'll have to defer to Henrik on that :)

I found this PR after wanting neovims smooth scrolling in doom, so I am glad to see its happening. A suggestion:

• could we add an option to rebind C-<u/d> which is evil-scroll-<up/down> to good-scroll-<up/down>-full-screen maybe with a +evil flag or something

I am working on funcitonality to work with larger motions like [[ and gg but performance has been kinda bad so I am not sure about putting it in. Kinda seems it needs an addition to the evil-collection either way

:warning: This pull request has been automatically marked stale due to 60 days of inactivity. If this PR is still valid, please reply to it or it will be closed in 7 days.

Hey there! I'm preparing to move modules out of Doom's core to doomemacs/modules (official modules) and doomemacs/contrib-modules (community-maintained/contributed modules), and am resolving/transferring related PRs and issues. As this repo will no longer house modules (and Github offers no way to transfer PRs), I am closing it, but I'm still interested in its contents. Would you mind re-PRing it to https://github.com/doomemacs/contrib-modules?

There'll be an announcement on Discourse (under #news) this weekend about why this is happening and the general state and direction of Doom, if you're curious.

The reason this is being directed to contrib-modules instead of modules is because I don't think smooth/pixel scrolling in Emacs is in an acceptable state yet. It (and the packages that implement it) are terribly inefficient except in the most trivial cases, like small text buffers or the splash screen. It takes precious little to slow Emacs to crawl with it on. I've seen similarly poor results testing good-scroll across different OSes and a vast performance difference across Emacs versions and builds.

That said, mac-mouse-wheel-smooth-scroll (28+ and mac only) and pixel-scroll-precision-mode (29+) are promising, and with native-comp plus performance optimizations awaiting us in 29, I don't think we'll have to wait too long, but we're just not there yet.

In any case, sorry for the trouble and thank you for the PR and help!

Thanks! I'll be making another PR then :yum:

Also @hlissner, do you have some info on what the directory structure will look like? I can also later make another PR to add that info in the Contribute section of the readme if that can help :smile:

Ah, my bad: please PR modules under a top-level modules/ directory. So modules/ui/pixel-scroll/, in this case.

I can also later make another PR to add that info in the Contribute section of the readme if that can help

Give me another week, first. I have a more complete README that I'm withholding until I formally start transferring modules. Thanks for the offer in any case!

Thanks!