search

kylechui / nvim-surround

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

fix: line surrounds are properly dot-repeatable #262

Closed ribru17 closed 8 months ago

ribru17 commented 9 months ago

Also corrects a dot-repeat test

Sorry, it seems https://github.com/kylechui/nvim-surround/pull/261 got closed after an emotional rollercoaster. In any case the tests are working now. Thanks for taking a look!

kylechui commented 9 months ago

Thanks for putting this PR together! This has definitely been a pain point for me as well at times, so I'm glad to see it addressed. However, I'm hoping that the implementation can be improved a bit (in particular, I'm not a huge fan of embedding code into strings). Do you mind explaining real quick roughly how the PR solves the issue? It might give some insight as for what general approach works to fix it. My current understanding is something along the lines of: packing everything into a single string makes dot-repeating repeat "more stuff", and includes the beginning ^ motion instead of just dot-repeating to the end of the line.

ribru17 commented 9 months ago

No problem, and yeah totally valid concerns! I'd love to try and find a smarter way of doing this. Your description sums it up perfectly, I think just the fact that the motions are after the <Plug> call allows them to be dot repeatable. I couldn't think of a great way to combine the two motions (hopefully there is one), hence the hacky norm! ^vg_ approach.

ribru17 commented 9 months ago

Alright see what you think of this solution. Much less hacky, and takes advantage of the fact that whitespace is trimmed from the selection. That really makes things simpler here. Also fully backwards compatible with previous functionality in terms of [count]yss and (after a yss), [count]..

It works because we just put the whole selection in visual line mode, and g_ will move us down [count] - 1 lines (the g_ is sort of arbitrary, it would also work with _ or other motions, but a motion like j would be less desirable because, for example, 1j would take us down a line while 1g_ will keep us on the same line which works well in this case).

kylechui commented 9 months ago

Sorry for the delay. Do you know what the behavior is for adding a <Plug> mapping and then performing a visual-mode motion in an expr keymap? It looks like this works but I'd love to know the underlying reason why.

ribru17 commented 9 months ago

Basically, any visual mode key inputs directly after the <Plug> mapping are treated only as specifiers of the type of visual selection to be applied to the motion that follows. These keys also ignore the count, which is applied again to the following motion. So in this case we specify that we want a linewise selection, and since this plugin trims whitespace anyway we get a nice, simple way of selecting the content of the line. And as a bonus it now will also work with dot repeat and respect move_cursor = false. You can test this yourself with ysV<motion> and the result is the same. You can also see that for whatever reason you can type an arbitrary amount of visual motions and only the last will be actually applied (e.g. ysvvVV<C-v>vV is equivalent to ysV).

kylechui commented 8 months ago

Thanks for the PR and explanation!