jake-stewart / multicursor.nvim

multiple cursors in neovim
MIT License
573 stars 4 forks source link

Multicursors do not expand snippets as expected #46

Closed RAV64 closed 5 days ago

RAV64 commented 6 days ago

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

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

https://github.com/user-attachments/assets/a9cbf54a-894a-4e7f-8df2-1643986fc78d

And reproducible Lua example:

https://github.com/user-attachments/assets/52dbcc3a-0e09-42ac-953a-9aad34158258

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,
},
jake-stewart commented 6 days ago

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).

jake-stewart commented 6 days ago

i will play around with this though

RAV64 commented 6 days ago

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 😅).

jake-stewart commented 6 days ago

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.

RAV64 commented 6 days ago

https://github.com/user-attachments/assets/dfb0a28c-7c62-4770-ac99-c5519cbee1b1

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.

jake-stewart commented 6 days ago

https://github.com/user-attachments/assets/08c99f95-98b7-42aa-a987-e3fd09d10fe8

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

jake-stewart commented 6 days ago

i still need to

  1. keep track of each marker
  2. move to the first marker after expanding snippet
  3. reenter insert mode
  4. wrap vim.snippet.next() to move each cursor to the next/previous marker
  5. use original vim.snippet instead of my wrapper when only a single cursor
  6. make it work with undo/redo
jake-stewart commented 6 days ago

@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

RAV64 commented 6 days ago

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.

RAV64 commented 6 days ago
image

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

jake-stewart commented 6 days ago

@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.

RAV64 commented 6 days ago

https://github.com/user-attachments/assets/3246d0b2-a9af-481c-9f61-31deb10dcab7

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

RAV64 commented 6 days ago

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

https://github.com/user-attachments/assets/99dd3772-aaab-4ff6-912c-8401e9b32676

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?

jake-stewart commented 6 days ago

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

jake-stewart commented 6 days ago

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

RAV64 commented 6 days ago

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

https://github.com/user-attachments/assets/aba4820a-1e7f-4191-9645-0a4f8e699b2b

  1. 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.

  2. 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.

RAV64 commented 6 days ago

Edit - was completely wrong on my first example 😆

https://github.com/user-attachments/assets/11672221-6de0-4991-a171-6c2384dabedb

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

jake-stewart commented 5 days ago

fixed and now in main and 1.0 branches

jake-stewart commented 5 days ago

undo/redo still a little scuffed. will fix

jake-stewart commented 5 days ago

should be fixed

RAV64 commented 5 days ago

Awesome 😄

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

https://github.com/user-attachments/assets/a74b8534-6d6b-4df6-bfee-3d5f252ce7b3

RAV64 commented 5 days ago

https://github.com/user-attachments/assets/f61c6880-2e92-4a3c-9704-4678c5d7dbd3

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

jake-stewart commented 5 days ago

@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.

RAV64 commented 3 days ago

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.

jake-stewart commented 3 days ago

@RAV64 try now

RAV64 commented 3 days ago

https://github.com/user-attachments/assets/6a2e3a81-0e70-4d2e-8bab-f833df0a8d86

(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.

jake-stewart commented 3 days ago

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

RAV64 commented 3 days ago

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 👍🏻

jake-stewart commented 3 days ago

@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

jake-stewart commented 3 days ago

I've noticed some issues

jake-stewart commented 3 days ago

@RAV64 should be good now

jake-stewart commented 3 days ago

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