mg979 / vim-visual-multi

Multiple cursors plugin for vim/neovim
MIT License
4.24k stars 82 forks source link

Fix LunarWatcher/auto-pairs compatibility errors #256

Closed mmrwoods closed 9 months ago

mmrwoods commented 9 months ago

LunarWatcher/auto-pairs is a maintained fork of jiangmiao/auto-pairs, which moves functions to autoload, so AutoPairsTryInit() does not exist.

This commit disables the compatibility hacks for this maintained fork, because they disable auto-pairs for the buffer and fail to re-enable it.

At a glance the fork seems to work ok with vim-visual-multi, at least it continues to complete pairs after using vim-visual-multi without calling autopairs#AutoPairsTryInit(), as long as b:autopairs_enabled is true.

There may well be some compatibility issues I'm not aware of, but these could be handled via a additional entry in the compatibility dictionary, or updating the existing entry to be aware of the maintained auto-pairs fork, while also being backwards compatible with the original plugin.

mg979 commented 9 months ago

Thanks.

mmrwoods commented 9 months ago

That was quick :-) Thanks for merging!

LunarWatcher commented 8 months ago

Weird. Auto-pairs should already be patching this:

https://github.com/LunarWatcher/auto-pairs/blob/11fec7442de5cd63d07563ae64767fcee7a594a2/plugin/autopairs.vim#L41-L52

I have not had any problems with it anyway, and it has worked since the recommendation to use that system back in 2021 (#165). The part of vim-visual-multi that processes that dict is in an autoload file, while the dict is set in auto-pairs' plugin file, so I doubt it's a load order problem (of which I've had the displeasure of dealing with far too many of, but that's Vim's fault for not having a real way to have plugins that depend on other plugins without causing potential instability because load order)

... on second thought, it might happen if auto-pairs loads before vim-visual-multi because reasons:tm:. I'll add a dedicated config variable toggle or something to auto-pairs instead of relying on detecting config variables

mmrwoods commented 8 months ago

@LunarWatcher looks like g:VM_plugins_compatibilty is no longer declared as a variable with an initial value, so your conditional code does not run, i.e exists('g:VM_plugins_compatibilty') is false

LunarWatcher commented 8 months ago

@mmrwoods I already accounted for that. That's why there's an or statement that checks for the existence of a specific vim-visual-multi autoload function. exists is pretty useful like that - that part is a check for whether or not an autoload function exists (see https://vi.stackexchange.com/a/38491/21251), which I assumed would be bulletproof. By the time the plugin file is called, the autoload files have all been at least recognised by vim, so that part should work even if g:VM_plugins_compatibilty doesn't exist.

I have not read up on the inner workings of checking for autoload functions, but it's possible I misunderstood whether or not the autoload file is loaded when existence is checked. It's a very real possibility that the check I'm using causes the file defining s:plugins to be loaded before the plugin compat variable can be set, thereby preventing the override from working. However, I can't get this to make sense either, because it works fine for me, and if this was correct, I'd imagine it would be universally broken.

Honestly, I'm not sure if an if statement makes sense at all. It doesn't cause any major overhead not to have one. But that's an implementation detail I'll figure out in a few hours

mmrwoods commented 8 months ago

Ah, sorry, my bad, I glanced at the code quickly and didn't spot the else

mmrwoods commented 8 months ago

I'll have a look at it with a minimal vimrc later and see if I can reproduce

mmrwoods commented 8 months ago

Seems to be related to plugin/package load order...

Tested using Vim 8.2 on Debian 11 (this was just the easiest thing to test with as I had a Dockerfile handy, see https://github.com/mmrwoods/vim-friendly/blob/main/Dockerfile, replaced buster with bullseye)

Cloning both mg979/vim-visual-multi and LunarWatcher/auto-pairs to ~/.vim/pack/bundle/start, then reverting my commit to vim-visual-multi (https://github.com/mg979/vim-visual-multi/pull/256/commits/75eaf6a4947729bfd566904b5e833582fd8123b9) I can reproduce the error after I trigger visual multi using CTRL-N and then exit using ESC

If I edit my, otherwise empty, ~/.vimrc to load vim-visual-multi before auto-pairs it works fine, e.g.

packadd vim-visual-multi
packadd auto-pairs
mmrwoods commented 8 months ago

Possible solution, load auto-pairs compatibility hacks on VimEnter, e.g. https://github.com/LunarWatcher/auto-pairs/pull/86