Use builtin methods for getting selection.

[L3MON4D3]

Okay, there are too many edgecases for implementing this without spending wayyyy too much time on it.
This just saves all registers affected by s (1-9, -, unnamed, z) cuts the selected (VISUAL) text into an arbritary register (z), and then applies the code we already have to get the various variables (TM_SELECTED_TEXT, LS_SELECT_DEDENT, LS_SELECT_RAW).
This is now a bit more messy (need to get all lines/positions before cutting them) so I've opted to put all of this into its own module, util.select.

@leiserfg Could you check this over? I'm especially unsure if this will actually not change any register-state.

Once that is certain, I'll add some tests, won't go through that hassle if there's a chance we might have to scrap this :D

[hinell]

Take a look at: #1026

[hinell]

Overall I think my PR #1026 is better. More importantly: it keeps backwards compatibility. Constantly changing names of exposed apis isn't good.

[L3MON4D3]

In general of course, but store_selection is not exposed, the only thing the user should interact with is store_selection_keys in setup.
We'll keep this one, sorry :/

[hinell]

@L3MON4D3 Well I think you have to export store_selection cause it's required for completion plugin integration. e.g. for nvim-cmp See my reply above.

[L3MON4D3]

You'll have to expand on that, I don't see how cmp_luasnip needs to use luasnips API to get the current selection.

[bew]

In general of course, but store_selection is not exposed, the only thing the user should interact with is store_selection_keys in setup.

Note: I'm personally depending on store_selection directly because I want to define keys myself (in the future I want to dynamically set keybindings, only where/when needed), not set keys in plugin's setup function.
Do you have a specific reason for not exposing store_selection directly?

[L3MON4D3]

Well, mostly to allow us to do changes like this :D
For example, if we say the API is "define the keys to use in a keybinding", we are free to change the Implementation from a purely lua-function to something that more strongly depends on being used in a key binding (which is more-or-less what is happening here, except that the API would already be hard to communicate cleanly, since it depends on the keybindig to remove the keys)
We can talk about exposing, say, select_keys, then you can still set the keybindig yourself :)

[hinell]

You'll have to expand on that, I don't see

More fine-grained control over ${SELECTION} expansion.

We can talk about exposing, say,

Vague. Forcing users to use some config that is unstable across versions to config keybindings is very bad way to anything. Interference with others bindings is another concern.

[L3MON4D3]

More fine-grained control over ${SELECTION} expansion.

Sorry, I still don't understand how an exposed store_selection helps with that, could you give a full example?

Vague. Forcing users to use some config that is unstable across versions to config keybindings is very bad way to anything. Interference with others bindings is another concern.

No, the idea is to add the a function/field that exposes the rhs of the mapping to our official API (s.t. it is not unstable across versions). Users can then just call vim.keymap.set themselves.

[hinell]

@L3MON4D3 Refer to #1029

[bew]

[free to change impl] to something that more strongly depends on being used in a key binding (which is more-or-less what is happening here, except that the API would already be hard to communicate cleanly, since it depends on the keybinding to remove the keys)

Sorry but I don't understand that sentence 😬

What benefit would you get from strongly depending on the fn being used in a key binding?

About the part at the end about depending on the keybinding to remove the key:
Searching for store_selection_keys only gives me a result of it being used to set the keybinding globally, do you need it somewhere else to remove it?

[bew]

I'm especially unsure if this will actually not change any register-state.

After some local testing and docs reading, I've found that using "zs on single-line selection would only change registers "z & "", but for a multi-line selection it would additionally change "1 (and loose "9). "- is not 'infected' at all.

I've also found an alternative (maybe?): If we use "zygv"_s (yank to "z, re-select, cut into blackhole), for both single-line and multi-line selection only "z & "" are changed. ("0, "1>"9 & "- are not changed at all, thus shouldn't need to be saved.

Try it by copy/pasting vnoremap <A-z> "zygv"_sfoo<esc>:registers: will store selection in "z, substitute with foo and pre-fill cmdline with :registers view (just have to press enter to view all registers).

[L3MON4D3]

Sorry but I don't understand that sentence 😬
What benefit would you get from strongly depending on the fn being used in a key binding?

Well, we want to do something like call lua function -> give nvim some keys to process -> call lua function (store registers -> cut -> restore registers), which can be implemented by calling the lua-functions by feeding something like <cmd>lua ... or :lua ... in the rhs of a mapping.
And since this function is very likely to only be used in a mapping (I presume), exposing the rhs of the mapping seems like a good alternative to exposing a lua-function.

... do you need it somewhere else to remove it?

Ah, that "remove the keys" was wrong, I wanted to type "remove the text" 🤦 Sorry for the confusion.

[L3MON4D3]

"- is not 'infected' at all.

Ah right, because we specify a register 👍

I've also found an alternative (maybe?): If we use "zygv"_s (yank to "z, re-select, cut into blackhole), for both single-line and multi-line selection only "z & "" are changed. ("0, "1>"9 & "- are not changed at all, thus shouldn't need to be saved.

Oh cool, yeah that looks great, I'll incorporate that 👍

[bew]

And since this function is very likely to only be used in a mapping (I presume), exposing the rhs of the mapping seems like a good alternative to exposing a lua-function.

I see one downside, using nmap <the-key> to show what is mapped will show the raw rhs which I (the user) don't really care about (it could even be considered as a leak of abstraction)
Yet another alternative could be to have a pure Lua function as rhs that does some feedkeys in the middle to inject the necessary yank/cut keys?

[L3MON4D3]

I see one downside, using nmap to show what is mapped will show the raw rhs which I (the user) don't really care about (it could even be considered as a leak of abstraction

Phuh, yeeaaah okay, but that really does not seem like a big deal😅

Yet another alternative could be to have a pure Lua function as rhs that does some feedkeys in the middle to inject the necessary yank/cut keys?

Mhmm true, we could do that.. only using it in mappings is a bit weird, since we'd do pure keys -> lua-function -> pure keys, so the official mapping would still use the "leaky" rhs.
Also, not 100% sure this actually works with feedkeys since it seems to sometimes have issues with immediate execution, which we'd really need to make this always work.

[L3MON4D3]

Alright, I'd prefer to stick with only exporting the keys for now, it should work via e.g.
vim.keymap.set("x", ".", ls.select_keys, {silent = true}). I've briefly tried passing the rhs to feedkeys, but that didn't really work as desired, so leaving that for now.
I also implemented your suggestions @bew, and although fewer registers are affected, we now get a message about yanking to "z" (can't seem to silence it :/ ), so we should probably stick to the dirtier version

[leiserfg]

Sorry @L3MON4D3 no idea how I missed this, too many noise in my notifications. You jus need to store register 9, as 1 to 8 should be now 2 to 9 so you can iterate over them and set the values back.

[L3MON4D3]

Oh, no worries :)

You just need to store register 9, as 1 to 8 should be now 2 to 9 so you can iterate over them and set the values back.

Riiiiiiiiiiight yeah of course.. had not considered that at all xD
I do think it's a bit more elegant, but this push-pop is so simple, I think we should just stick with that 😅

[L3MON4D3]

CI is lying, tests all pass :D