kylechui / nvim-surround

Add/change/delete surrounding delimiter pairs with ease. Written with :heart: in Lua.
MIT License
3.09k stars 61 forks source link

Scrolled window when use aliases #149

Closed rapan931 closed 1 year ago

rapan931 commented 2 years ago

Checklist

To reproduce

-- minimal_init.lua
vim.o.runtimepath = vim.o.runtimepath .. ',' .. '~/.local/share/nvim/site/pack/p/start/nvim-surround/'
require("nvim-surround").setup({
  aliases = {
    ["q"] = { '"', "'", "`" },
    ["s"] = { "}", "]", ")", ">", '"', "'", "`" },
  }
})
  1. nvim --clean ~/binary_comments/README.md(here
  2. source ~/minimal_init.lua
  3. gg10j20l
  4. css[ (windows scrolled)

sample nvim-surround

Expected behavior

not scrolled

Actual behavior

scrolled

Additional context

env

windows10 & wsl & ubuntu

rapan931@rHost:~$ nvim --version
NVIM v0.8.0-dev-1010-ge085d0be3
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
kylechui commented 1 year ago

Ok this issue seems to be due to the fact that the s alias is trying all other keys out, some of which may be off-screen. I'll see what I can do to "reset" the buffer status to before the surround.

kylechui commented 1 year ago

It seems like this can be done with a combination of :mkview and :loadview before and after performing the surround action. However, this modifies the state of the user's views. This would also probably complicate some code regarding the cursor positioning... If anybody has any alternative ideas for how to implement this I'm all ears!

kylechui commented 1 year ago

TIL that vim.fn.winsaveview and vim.fn.winrestview are functions; they should be added where appropriate in the change/delete callback functions (before/after the selections are computed).

kylechui commented 1 year ago

I would appreciate it if @rapan931, @kohane27, @astier, @ribru17 would all test this for me, as it seems to be working fine on my end (at least for the one case that started this issue). It's the linked branch fix-scrolling.

ribru17 commented 1 year ago

I've been testing and it seems to work perfectly for me, thank you so much!! One thing I'll mention just in case (this is probably the best behavior anyway) is that if I have brackets spanning multiple lines, e.g.:

(
...
)

and the first bracket is on a line less than the defined scrolloff limit (but the cursor isn't) there will be a little bit of scrolling since the cursor will be positioned at the first bracket which means it needs to scroll a bit to account for the scroll offset defined by the user. But again I think this is probably a good thing. With single line delimiter deletion the cursor stays in the same spot but for multiline this might cause confusion if it's unclear which delimiters were deleted, so I think the current solution is good. Thanks again!

kylechui commented 1 year ago

I certainly didn't intend for things to work out like that, but yeah I agree that it's probably best. The screen will inevitably have to scroll to show the cursor on screen, and the default behavior for nvim-surround is for the cursor to jump to the left delimiter.

rapan931 commented 1 year ago

I checked and it was fixed! Thanks!

astier commented 1 year ago

I don't use nvim-surround right now. But I hope it works.