Edits remove all marks on affected lines

[Diomendius]

Checklist

• [x] Have you updated the plugin to the latest version on main branch?
• [x] Have you checked the Breaking Changes issue?
• [x] Have you read through :h nvim-surround to see if there might be any relevant information there?

Describe the bug Operations such as the ys, ds and cs default mappings remove marks and extmarks from the lines they modify.

To Reproduce

1. Type some text on a single line into a buffer.
2. Place a mark somewhere in this line.
3. Change the line with a command from this plugin, such as ysl" with default mappings.
4. Use ` or ' to jump to the mark you set.
5. Observe the error E20: Mark not set instead of the cursor jumping to the mark you set. :marks also does not show the mark.

Expected behavior All marks (including extmarks such as those used by plugins such as LuaSnip) in regions of text not modified should be preserved.

Additional context This plugin uses nvim_buf_set_lines() which removes all marks and extmarks from the lines it replaces, but not nvim_buf_set_text() which preserves regular marks outside the specified region and preserves extmarks even within the specified region, updating their positions/regions according to their gravity. As I understand it, positions within the replaced region are moved to the left boundary of the replaced region if the position has left-gravity, otherwise they are moved to the right boundary. This doesn't always do the Right Thing when replacing a region which contains an extmark or extmark boundary, but it's generally preferable to removing the mark entirely and should behave sanely when replacing any other region, including a region entirely enclosed within an extmark.

nvim_buf_set_text() was added in Neovim v0.5.0 and the only breaking change since then has been to how it treats negative indices, which shouldn't be relevant for anything this plugin does.

https://github.com/kylechui/nvim-surround/blob/030a4373aea4354d28052108650bf4e916d3283a/lua/nvim-surround/buffer.lua#L123-L133 In particular, the body of the above function can probably be replaced with a one-liner call to nvim_buf_set_text().

[kylechui]

@Diomendius Thanks for spotting yet another bug, hopefully the linked pull request resolves it?

Edit: It does not (yet). It seems to still erase the marks when an additive surround occurs on the same line, before the mark.

P.S. It also simplifies the code quite a bit, so thanks again for tipping me off about vim.api.nvim_buf_set_text :tada:

[kylechui]

It seems to me that adding this new text will inevitably delete the marks, but I'm not 100% sure.

[Diomendius]

I thought I tested this and normal marks didn't get deleted, but it looks like all normal marks on lines affected by nvim_buf_set_text() get deleted. Vanilla editing operations actually don't preserve marks correctly either; they don't delete marks, but inserting or deleting text within a line doesn't shift the positions of any marks within that line. Inserting or deleting lines will shift the row coordinates of marks on other lines, but both nvim_buf_set_*() functions also do this AFAIK.

Normal marks are generally only used by users, so while it's annoying for plugins to delete them, it shouldn't break the functionality of anything else. Extmarks are used by Nvim plugins and are specifically designed to keep their positions in the text when surrounding or contained text is edited, and nvim_buf_set_text() should behave well in this regard, so long as you avoid reading text from the buffer then writing that same text over itself with nvim_buf_set_text().

[kylechui]

Agreed; should I just merge this in for now, seeing as there probably isn't a way to "properly" fix this without rewriting/testing a bunch of new code?

[Diomendius]

I just tested your branch and it looks like it mostly works as expected with regards to extmarks. LuaSnip uses them to store jump history and the only issue I've encountered is that changing delimiters immediately adjacent to an extmark region (in this case a snippet insertion point surrounded by parens/braces/whatever) can cause the extmark region to grow to include the new delimiters where it excluded the old ones.

This is actually the same as what happens if you manually replace the delimiters with r, so I don't think we can do better here. I think this is configurable by changing the gravity of the extmarks, so this may be fixable on LuaSnip's end, though I think it's a tradeoff between two different wrong behaviours. Given some text ab, I don't think there's currently any distinction between using rx on the a versus i<bs>x on the b. Both methods produce xb and I think in both cases an extmark on b with left-gravity will move to x while one with right-gravity will stay on b. Thus, I don't think there's a way to anchor an extmark relative to a position between two characters such that changes initiated from the left keep the extmark to the right and inserting text before the character on the right keeps the extmark to the left.

Since this works as well as is possible, you might as well merge it. Any proper fixes would probably have to be improvements to Neovim, either to introduce a distinction between insert and append so that extmarks can be more configurable, or to make nvim_buf_set_text() preserve normal marks, perhaps in conjunction with a larger change to make normal marks anchor their positions within lines as extmarks do, not just between lines.

[kylechui]

Yeah I don't actually know if buf_set_text clearing the marks is intended behavior or not, otherwise it might be a good idea to open up an issue upstream. Thanks again for taking the time to give me such detailed feedback/thoughts :sparkling_heart: