neoscroll.nvim icon indicating copy to clipboard operation
neoscroll.nvim copied to clipboard

[Feature request] Implement scrolling for an arbitrary window

Open alfaix opened this issue 2 years ago • 3 comments

Basically title. Currently, as far as I can tell, only supported for current window.

Why: to scroll popup/preview/qf/diagnostics windows without having to move the cursor there. Especially annoying if the hover() documentation does not fit in the popup.

I initially tried to do something like vim.fn.win_execute(winid, "lua require'nescroll'.scroll(...)"), but that only scrolls the window for one line and then keeps scrolling the current one. From the looks of it, implementing this would require refactoring some globals into per-winid tables and calling vim.fn.* (such as winline()) inside win_execute context - which is a handful, but possible.

I'm just moving from Vim to Neovim and learning Lua, but I would be happy to take a crack at it in a PR. However, if the feature is not needed or impossible for some reason I can't see, please let me know.

alfaix avatar Jun 17 '22 20:06 alfaix

Made a rough draft that mostly works, though not tested with all possible configs ~~and has a bug with gg~~: https://github.com/karb94/neoscroll.nvim/compare/master...alfaix:alfaix/arbitrary-winid

Discovered a couple of bugs:

  • with so_scope = wo, scrolloff can be -1, which causes very annoying off-by-1 errors (e.g., zt causes the current line to disappear) (fixed)
  • I think currently the timer is always on and does nothing most of the time, but still takes resources. Called timer:stop() in stop_scrolling() (fixed)
  • closing window while scrolling sometimes causes errors, sometimes keeps scrolling the new current window. In the new implementation it still scrolls the current window (though maybe it's worth patching that out), but if an error occurs it doesn't get spammed a billion times over (partially fixed)
  • G also maps for <count>G and goes to the last line, which it most definitely shouldn't (not fixed)

Please let me know if a PR would be ok

alfaix avatar Jun 18 '22 16:06 alfaix

I also think you should be able to apply scroll() to any window. As you said this requires quite a bit of refactoring and no one was really asking for it so I focused on other features. Right now my priority is to build tests so that I don't miss any bugs like some of the ones you mentioned moving forward. Once I set the tests and fix all bugs (including the ones you mentioned) I will be happy to help with/accept a PR on this.

Regarding the bugs you've identified, can you clarify what is wrong with the timer? stop_scrolling() already calls timer:stop() as far as I can tell.

Thank you for all your help.

karb94 avatar Jun 19 '22 23:06 karb94

I made a PR, though, as you said, it is probably best to wait for tests before merging it. Meanwhile, I'll just tell packer to use the fork. When the tests are ready, I'll add them to the PR, and make sure everything runs.

Regarding stop_scrolling() - I'm stupid, must have deleted it while refactoring and then wonder where did it go. Sorry :)

Will wait for tests, thanks!

alfaix avatar Jun 20 '22 09:06 alfaix