goolord / alpha-nvim

a lua powered greeter like vim-startify / dashboard-nvim
MIT License
1.78k stars 104 forks source link

fix: pass alpha window to move_cursor() in CursorMoved callback #201

Closed jdrouhard closed 1 year ago

jdrouhard commented 1 year ago

The CursorMoved autocmd should pass the explicit alpha window handle to move_cursor() (and on to the API calls). This doesn't directly address the FIXME comment directly above the change, but this does fix an actual issue when using alpha and fzf-lua.

Due to this commit in neovim, CursorMoved autocmds are now fired when calling vim.api.nvim_win_set_cursor(). There's likely an upstream neovim bug in the mix here, but it seems that if we just tell the API to use the "current window" in that callback (even if its a buffer-local callback for the alpha buffer only), neovim gets confused and jacks up the offsets for determining where to open float windows. Its as if neovim is firing the CursorMoved callback for the alpha buffer, but the window has already been switched to the new float, so it tried to move the cursor in the wrong window.

Generally, probably better to be explicit about the specific windows and buffers in play for the autocommands since we have the handle numbers, anyway.

For reference, this is the PR and issue that I started this issue (and likely needs to be followed up with another fix):

https://github.com/neovim/neovim/pull/21072 https://github.com/neovim/neovim/issues/19063

goolord commented 1 year ago

thanks for the really detailed description! unfortunately i'm leaving for vacation as i'm typing this so i won't have a good chance to test this until ~tuesday, but this looks good so i might just merge it

jdrouhard commented 1 year ago

ping @goolord

goolord commented 1 year ago

ok, got around to testing this a little. it looks good to me. i feel like i had it like this at one point and it caused some issues, but i don't recall and this seems more principled, so i'm going to merge it. fingers crossed this doesn't break anything, lol

goolord commented 1 year ago

and thank you for your contribution! 🖤

goolord commented 1 year ago

@jdrouhard this reintroduced #168 so i had to revert it. i'll try to look at this later for a solution that solves both problems

goolord commented 1 year ago

i think i have to buckle down and figure out how to get an up to date buffer -> [window] mapping

jdrouhard commented 1 year ago

Using the actual window's handle should work correctly. If it doesn't, it's probably a neovim bug.

goolord commented 1 year ago

the bug is with alpha. the problem is that you can open 1 alpha buffer in multiple windows, so if you open alpha in a new window and close the window stored in state.window, calling nvim_win_set_cursor will error since you're referring to a window that doesn't exist. trying to strap a gui over a vim buffer is just hacky business

goolord commented 1 year ago

fixed in https://github.com/goolord/alpha-nvim/commit/5b7ef660c6f80fbd04cccdf69624bf8c47513471, credited you as the co-author

jdrouhard commented 1 year ago

Appreciate it. The new version doesn't actually fix the issue I was seeing with fzf-lua being opened in the alpha window, but maybe that's the underlying neovim bug I need to hunt down.

goolord commented 1 year ago

neovim's autocmd events are a mess, strapping a gui over a vim buffer is just very difficult

goolord commented 1 year ago

i believe (not 100% sure) the problem is that the intentional behavior of CursorMoved is such that when a new window is opened and the cursor moves to that window, the last buffer, which is the alpha buffer in this case, is set initially, and then the new buffer is loaded. so, there's no way to tell if when the window is different on CursorMoved weather it's an alpha split or a new window