Multicursors do not expand snippets as expected

Question

Multicursors do not expand snippets as expected

Closed this issue 3 months ago · 32 comments

RAV64 commented 3 months ago

As the title states - Extra cursors do not expand snippets as expected.

Here is an example of this being issue in Rust code:

Screen.Recording.2024-10-01.at.10.47.19.mov

And reproducible Lua example:

Screen.Recording.2024-10-01.at.10.58.21.mov

If helpful, this is how I've set snippet expansion in completion plugin nvim-cmp

snippet = {
  expand = function(args)
    vim.snippet.expand(args.body)
  end,
},

Answer 1 · 2024-10-01T08:41:23.000Z

try to run the snippet without multicursors installed and then press . to see if it repeats correctly.

if it doesn't repeat correctly, then i think it should be considered a bug with the snippet because how are we supposed to know what should be repeated?

for example, imagine a snippet has a side effect of importing a file. if we repeat that, then we end up importing the file multiple times (once for each cursor).

Answer 2 · 2024-10-01T08:53:55.000Z

i will play around with this though

Answer 3 · 2024-10-01T08:57:07.000Z

Ah - I'm starting to get how multicursors are implemented. Pressing . does not repeat the snippet correctly (At least based on what I assume is correct 😅).

Answer 4 · 2024-10-01T09:01:45.000Z

i may be able to make my own version of vim.snippet.expand so on so forth. i could wrap vim.snippet so people using it automatically use mine.

Answer 5 · 2024-10-01T09:10:08.000Z

example.mov

Rust-analyzer provides the struct fields as jumpable snippet thingies - Showcased in the included video.

I don't know if its feasible to make this work with multicursor.nvim - I can work around this issue in these cases too by not expanding the snippet.

Answer 6 · 2024-10-01T10:36:00.000Z

Screen.Recording.2024-10-01.at.6.34.16.PM.mov

looks like it's possible but a lot of work to do

Answer 7 · 2024-10-01T10:40:03.000Z

i still need to

keep track of each marker
move to the first marker after expanding snippet
reenter insert mode
wrap vim.snippet.next() to move each cursor to the next/previous marker
use original vim.snippet instead of my wrapper when only a single cursor
make it work with undo/redo

Answer 8 · 2024-10-01T10:55:25.000Z

@RAV64 can you think of a single use case where jumping between snippet markers is useful for multicursor? seems like something you'd only do with a single cursor when creating something new. i think i could do a first iteration where jumping between markers does nothing. simply expand the snippet and jump to the default location

Answer 9 · 2024-10-01T11:02:57.000Z

I'm mainly interested in the actual expansion working (since all functions in rust require that), not jumping between the snippet markers.

In my initial comment I would like the () to be appended to the "default" function calls.

example.mov

Rust-analyzer provides the struct fields as jumpable snippet thingies - Showcased in the included video.

I don't know if its feasible to make this work with multicursor.nvim - I can work around this issue in these cases too by not expanding the snippet.

In this example having the "example" calls expanded and then being able to to modify them, where at the end everything would be as expected is good enough for me.

The main issue I have is when multicursors end up in a state that is different to the main cursor.

Answer 10 · 2024-10-01T11:04:49.000Z

Reaching a state like this would allow me to edit these in consistent way.

Answer 11 · 2024-10-01T12:51:04.000Z

@RAV64 can i get you to try out the snippet branch and let me know if everything works ok

{
    "jake-stewart/multicursor.nvim",
    branch = "snippet",
    config = function()
        ...
    end
}

EDIT: since you use vim.snippet it should work out of the box.
EDIT 2: pressing ? in normal mode after doing a snippet will repeat the snippet. i know about this issue.

Answer 12 · 2024-10-01T13:05:57.000Z

Screen.Recording.2024-10-01.at.16.03.56.mov

There's an extra "a" character after expanding. Otherwise looks great so far.

Answer 13 · 2024-10-01T13:11:25.000Z

Offtopic. I have something like this set for my tab key in insert mode which allows me to jump out of treesitter nodes:

local smart_tab = function()
	local node_ok, node = pcall(vim.treesitter.get_node)
	if not node_ok then
		vim.notify("TS not available")
		return false
	end
	local row, col = node:end_()
	local ok = pcall(vim.api.nvim_win_set_cursor, 0, { row + 1, col })
	return ok
end

Screen.Recording.2024-10-01.at.16.09.32.mov

This also causes desync between where main cursor is and where I expect the other cursors to be.

Should I modify this code in some way to make it work with the multicursors?

Answer 14 · 2024-10-01T13:24:02.000Z

not possible in insert mode. i should add a function which lets you do random insert mode shit & update the cursors after. i can't do this after every keypress otherwise it breaks autocompletion etc

Answer 15 · 2024-10-01T14:15:54.000Z

@RAV64 i did some fixes. hopefully everything works now. please let me know, thanks. :Lazy update

Answer 16 · 2024-10-01T16:39:14.000Z

Amazing work - I had to play around a bit to find anything not working 😅

Screen.Recording.2024-10-01.at.19.35.24.mov

Here I expand a snippet, and another snippet inside of it, then undo the nested expanded snippet and the main cursor ends up in a wrong position.
I'm not sure how to reproduce - but once when testing the main cursor undo'd on the first undo call, then on the second undo call the extra cursors work was undo'd.

Answer 17 · 2024-10-01T16:44:31.000Z

Edit - was completely wrong on my first example 😆

Screen.Recording.2024-10-01.at.19.48.10.mov

If there are characters AFTER the expanded snippet - main cursor ends in wrong position after undoing the snippet expansion.

Answer 18 · 2024-10-01T23:39:52.000Z

fixed and now in main and 1.0 branches

Answer 19 · 2024-10-02T01:57:09.000Z

undo/redo still a little scuffed. will fix

Answer 20 · 2024-10-02T02:26:44.000Z

should be fixed

Answer 21 · 2024-10-02T02:36:21.000Z

Awesome 😄

Minor problem still exists:
All the cursors stay consistent now but are arguably in wrong position after undo.

Screen.Recording.2024-10-02.at.5.34.58.mov

Answer 22 · 2024-10-02T02:39:22.000Z

Screen.Recording.2024-10-02.at.5.38.00.mov

Found a case where one of the cursors still got desync'd when there are characters after the expanded snippet.

Answer 23 · 2024-10-02T12:59:15.000Z

@RAV64 give it a shot now, should all be fixed.

my implementation for snippets is such a hack i think when i have time i will copy paste neovim's snippets and just modify as needed instead of trying to work alongside it.

Answer 24 · 2024-10-04T04:53:58.000Z

Sorry I've not had much time to get back to you. Thanks for the excellent work again. The snippet support does seem like a tough challenge, maybe a robust rewrite would be needed :)

I've noticed that when you have two occurrences of the snippet expansion on the same line the behavior is still broken/not consistent with expectations. The behavior also changes depending on which cursor is the main cursor.

You can reproduce this by having a line like "a a", setting multicursor on both a and trying to expand a snippet. The undo behavior is also broken and the cursors end up in not expected places. Try to have "a a" with no indentation and then in indented scope.

Answer 25 · 2024-10-04T05:57:17.000Z

@RAV64 try now

Answer 26 · 2024-10-04T06:20:32.000Z

Screen.Recording.2024-10-04.at.9.16.09.mov

(Note, the "out" text is in correct place)

Whether the main cursor is the first or the last, when writing after expansion and undoing theres a desync.

If the main cursor is the first cursor on the line and you undo the snippet expansion, the cursors end up in wrong places.

Answer 27 · 2024-10-04T06:25:30.000Z

i think this is expected behaviour. if you do it without multicursor on the last a, after undoing it ends up in the same spot because it's the last column.

or am i confused?

EDIT: well without multicursor undoing undoes the entire insert. wonder if i can join the undo somehow

Answer 28 · 2024-10-04T06:49:13.000Z

Ah... - It might indeed be "correct" behavior, but the cursors still get desync'd. I would assume it's not the behavior one would "expect"? It's your call which you deem more important in this case 👍🏻

Answer 29 · 2024-10-04T07:09:34.000Z

@RAV64 can you try the branch snippet-undo and let me know if everything works ok. it makes undo work correctly by making the insert process a single undo via undojoin

Answer 30 · 2024-10-04T07:16:21.000Z

I've noticed some issues

Answer 31 · 2024-10-04T07:34:46.000Z

@RAV64 should be good now

Answer 32 · 2024-10-04T09:13:34.000Z

nevermind, i've abandoned the undojoin idea. 1.0 has latest fixes. if you want cursors to be back in sync you can just hit u again