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

nvim-surround looks backwards for 'nearest' replacement even with separators on the current line #50

Closed andrewferrier closed 2 years ago

andrewferrier commented 2 years ago

Checklist

To Reproduce Buffer:

a = b(
     c.d("xyz")
)

Place cursor on c, type c(

Expected behavior The braces surrounding "xyz" are highlighted ready for replacement

Actual behavior The braces after b and on the standalone line are highlighted ready for replacement

Explanation It seems to me that in general if nvim-surround is not on the replacements, it would want to look forward first - at least on the same line. I get that if it's inside the braces (which to be fair it is for the wider braces), that seems like the right ones to pick up, but I wonder if a special case for the current line would make sense?

kylechui commented 2 years ago

I was also thinking about this considering if your file is quite large, the surrounding braces might not even be on the screen. I might consider adding an option for this, but I'm pretty sure vim-surround would exhibit the same behavior

andrewferrier commented 2 years ago

Yeah, I just checked and you are correct - vim-surround does behave the same way (which doesn't mean that's the right way, but does at least mean you are consistent ;). I wouldn't object strongly if you closed this. In effect, what I'm asking for here would be a special case that could be confusing.

kylechui commented 2 years ago

I'll leave this open for a bit in case others would like to think about it, but I think that logically speaking the "jumping" is currently quite simple, e.g. prefer not jumping, then forwards, then reverse. Adding this edge case could be quite confusing (and also would require additional logic haha)

kylechui commented 2 years ago

@andrewferrier I think ideally I would want to open up a discussions thread for this, but I feel like nobody checks it :/ I wish polls would "notify" everybody that looks at this repo

NoahTheDuke commented 2 years ago

I would prefer to match how it works currently. When "inside" a pair of braces (of any kind), my expectation is that all actions will interact with them and not later braces I'm not currently inside. I might be biased because I work in a lisp (Clojure), but to me, interacting with the surrounding braces is the most important part of a "surround" plugin.

kylechui commented 2 years ago

@NoahTheDuke Indeed with so "controversial" a change as this one I would most definitely make it a user-defined option in settings, but in any case I'm leaning towards not implementing it at all for simplicity reasons

kylechui commented 2 years ago

Closing this as I don't intend on adding this to the plugin.