jake-stewart/multicursor.nvim

mc.{first,last,next,prev}Cursor don't work when cursors are disabled

Closed this issue · 6 comments

ajvondrak commented

Circling back on #24, it seems that the fixes in 168a621 were subsequently rendered ineffective by 90d88b1. Using this minimal config:

return {
  "jake-stewart/multicursor.nvim",
  commit = "c55145b7e5adbe8f1cdd60b4736c7ad18302a5c0", -- latest 1.0, as of this writing
  config = function()
    local mc = require("multicursor-nvim")
    mc.setup()
    vim.keymap.set("n", "<c-q>", function()
      -- could think of this as mc.toggleCursor, but that function isn't
      -- available on older commits needed for this repro.
      mc.disableCursors()
      mc.addCursor()
    end)
    vim.keymap.set("n", "]q", mc.nextCursor)
    vim.keymap.set("n", "[q", mc.prevCursor)
    vim.keymap.set("n", "]Q", mc.lastCursor)
    vim.keymap.set("n", "[Q", mc.firstCursor)
  end,
}
  • With commit = "c55145b7e5adbe8f1cdd60b4736c7ad18302a5c0", I once more have the behavior where my first/next/prev/last bindings will not cycle through disabled cursors, instead just staying on the main cursor.
  • As far back as commit = "90d88b142f7fef44bb7232b1c528e7312c6e6e00", the issue persists.
  • However, I can confirm that commit = "90d88b142f7fef44bb7232b1c528e7312c6e6e00^" cycles through disabled cursors as expected (note the ^, indicating that 90d88b is the culprit).

I think there are two issues, largely similar to the problems in #24:

  1. CursorContext:getCursors is not ultimately being passed any CursorQuery from the examples.{first,last,next,prev}Cursor functions, so defaults to { enabledCursors = true, disabledCursors = false }. Like before, it will not loop through the disabled cursors at all.
  2. Even when passing { enabledCursors = false, disabledCursors = true } locally (just to manually test), the subsequent cursor:select() in the examples.* functions still has the side effect of enabling the cursor we move to, not just changing the position of the main cursor. This was previously accounted for by the logic here, but I don't know if/how this was maintained by the refactoring.
jake-stewart commented

Sorry about that.

I had to do a lot of refactoring when making it possible to have multiple cursors AND multiple disabled cursors, making the disabled cursors behave sort of like an extra layer.

I am busy the rest of today but will take a look tonight.

  • ❤️1
ajvondrak commented

No worries. I've only been hacking on multicursors intermittently lately, and only just got around to a :Lazy update recently, or else I probably would've noticed this sooner, lol. Appreciate the responsiveness!

jake-stewart commented

I've pushed some changes which I think are a nice solution:

When you have multiple enabled cursors then nextCursor and prevCursor rotate between these enabled cursors and do not select disabled cursors.

When you have a single enabled cursor (the main cursor) then nextCursor and prevCursor moves the main cursor to the disabled cursors.

I think this should satisfy your use-case while allowing multiple enabled + multiple disabled to work sanely.

Let me know if any issues.

ajvondrak commented

Seems to clear up the nextCursor and prevCursor functionality, thanks. 👍

But with one enabled cursor (the main cursor) and several disabled cursors, the firstCursor and lastCursor functions still just stay on the main cursor (i.e., the [Q and ]Q mappings above don't move the cursor at all). Probably owing to them just calling ctx:{first,last}Cursor() without passing in any CursorQuery, so CursorContext:getCursors() will only ever return the main cursor (since it defaults to enabledCursors = true, disabledCursors = false).

jake-stewart commented

I have made firstCursor and lastCursor behave the same.

ajvondrak commented

Awesome, thanks. Seems to do the trick! 👍

  • ❤️1