kylechui / nvim-surround

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

`dsb` / `ysiw"` actions etc. do not work when run inside a different buffer #84

Closed smjonas closed 2 years ago

smjonas commented 2 years ago

Checklist

Describe the bug Ok, this might be kind of an unusual bug but please hear me out. When running :norm dsb in the current buffer, everything works as expected. However, run it in a different buffer and it won't work anymore. I came across this bug while working on my plugin command-preview.nvim.

To Reproduce

Paste this Lua code somewhere in your config and run it:

-- Create a scratch buffer with a single line
local scratch_buf = vim.api.nvim_create_buf(true, true)
vim.api.nvim_buf_set_lines(scratch_buf, 0, -1, false, { "Some (Line)" })

-- Run "norm dsb" in the scratch buffer using bufdo
vim.cmd(scratch_buf .. "bufdo norm dsb")

-- Retrieve the lines in the scratch buffer
local updated_lines = vim.api.nvim_buf_get_lines(scratch_buf, 0, -1, false) or {}
-- Prints { "Some (Line)" }; expected: { "Some Line" }
vim.pretty_print(updated_lines)

Expected behavior Deletion of brackets should work inside the scratch buffer, { "Some Line" } should be printed.

Additional context To confirm that my reproduction code makes sense, simply replace args = { "norm dsb" } with args = { "norm Axxx" }. This will output { "Some (Line)xxx" }, as expected.

kylechui commented 2 years ago

Unsure if I'm misreading this, but I created a new scratch buffer (just enter nvim from the terminal), and I populated it with the contents

test "string"

Then running :bufdo norm ds" deletes the quotes. I'm not sure if this is the same thing, nor if the new buffer is considered a "scratch" buffer, but it does say [No Name] for the file name.

smjonas commented 2 years ago

I think I found the issue: The order in which the relevant events happen is as follows:

So it seems I need to wait until this autocommand has been triggered. I'll report back if I found the final solution, maybe I'll have to use vim.schedule or something :)

smjonas commented 2 years ago

Could the mappings simply be defined globally? This would solve my issue (I have tested it :tada:). Currently, I don't see why the mappings should be buffer-local anyway:

What do you think?

kylechui commented 2 years ago

The point of buffer-local maps is to let the user have different mappings per buffer, e.g. ysiwf could have different meanings per language, as different function syntaxes are used. I think your concern about performance/runtime order is valid though, and I have a fix in mind for that (perhaps coming later today).

kylechui commented 2 years ago

If you want to get more familiar with the codebase, the TL;DR is that as of right now buffer-local maps are getting created/called for every buffer, via that autocommand. Furthermore, config.get_opts() is only referring to the buffer-local opts (which always exist, as they are created on every BufEnter event). My idea right now is just to remove the autocommand, and then have config.get_opts() "fallback" on config.user_opts whenever the buffer-local variable doesn't exist. Hopefully that clears some things up.

Edit: I guess in a sense they already are defined globally via config.user_opts, I just didn't think to use it that way after creating the buffer-local options haha

kylechui commented 2 years ago

Since I'm feeling quite stuck on #58, I'll write this up real quick on branch lazy-load-options.

Edit: I didn't really add/change much since I already had a layer of abstraction via config.get_opts(), so hopefully it "just works" for your needs? Let me know if it fixes your issue!

smjonas commented 2 years ago

Yes, it is working now, thank you very much! :sparkles: So this can be closed when the lazy-load-options branch has been merged.

kylechui commented 2 years ago

Merged as of #92.