ocaml / merlin

Context sensitive completion for OCaml in Vim and Emacs
https://ocaml.github.io/merlin/
MIT License
1.57k stars 233 forks source link

In Neovim "MerlinTypeOf" shifts entire buffer #820

Open macthecadillac opened 6 years ago

macthecadillac commented 6 years ago

OS: Debian 9.4/Testing (on two separate machines) Editor: NVIM v0.2.2 Merlin version: 3.0.5

With the version of Merlin I have installed, when I invoke MerlinTypeOf in Neovim, the entire buffer moves until the cursor touches the statusline. This is not the expected behavior from previous versions (I don't know which version specifically since I only update my opam packages once in a blue moon).

Minimal init.vim setup that reproduces the problem:

let g:opamshare = $HOME . '/.opam/4.06.1/share'
execute 'set rtp+=' . g:opamshare . '/merlin/vim'
execute 'set rtp+=' . g:opamshare . '/ocp-index/vim'
execute 'set rtp^=' . g:opamshare . '/ocp-indent/vim'

Thanks

copy commented 6 years ago

I can't reproduce this (on Arch Linux with neovim v0.2.2 and Merlin 3.0.5). Can you provide an OCaml source file and the location where you run MerlinTypeOf when this happens?

macthecadillac commented 6 years ago

This file:

https://github.com/macthecadillac/Mancala/blob/master/common.ml

Thanks

copy commented 6 years ago

@macthecadillac Can you try https://github.com/ocaml/merlin/pull/825 and see if it fixes the problem for you?

(git clone https://github.com/copy/merlin.git && cd merlin && git checkout neovim-fixes && opam pin add .)

macthecadillac commented 6 years ago

@copy It didn't fix it

copy commented 6 years ago

@macthecadillac Can you try it with opam pin add merlin 'git+https://github.com/copy/merlin.git#neovim-fixes'?

yeshengm commented 4 years ago

@copy This issue is still not resolved.

mreppen commented 4 years ago

I can also replicate. Here are the details:

ocamlmerlin -version
The Merlin toolkit version 73e9771b, for Ocaml 4.09.0

NVIM v0.4.3

Minimal neovim config merlintypeof.nvim.init

let g:opamshare = substitute(system('opam config var share'),'\n$','','''')
execute 'set rtp+=' . g:opamshare . '/merlin/vim'

I cloned this repository at f6954e953b4168e6a798a0255d6a2dfbd868a3c6. The following replicates the behavior.

nvim -u merlintypeof.nvim.init -c ':normal 113G7|zz\t' merlin/src/ocaml/utils/identifiable.ml

The buffer is shifted so that the cursor is at the top of the window, despite being centered before :MerlinTypeOf

mreppen commented 4 years ago

@copy Can you reproduce it with the setup from my above posts?

copy commented 4 years ago

@mreppen Yes, I can reproduce it. Unfortunately the issue came back in some version of neovim. I don't have time to look at it at the moment, but contributions are welcome.

mreppen commented 4 years ago

@copy I have a commit that fixes the issue. The change is simple, but I don't know if it has unintended consequences. Would you mind having a quick look before I submit a PR?

https://github.com/mreppen/merlin/commit/43b32068a2a2b2e100d3eaf36d29c18264ea3114

mreppen commented 4 years ago

@copy I feel a bit dumb having missed your neovim-fixes branch. I had a look at your previous commits and mine looks pretty much identical. Did you encounter problem with yours?

copy commented 4 years ago

@mreppen IIRC, we couldn't remove this workaround, because it was still necessary for some versions of vim or neovim.

let-def commented 4 years ago

@mreppen: I am at least partially responsible for some of the "magic incantations" that your patch remove. I think it is safe to do so but as @copy said, there were necessary for some versions of vim that I am fairly sure are still in use. It would probably good to merge with a conservative version check (do the clean stuff for recent versions we can test, fallback to the weird workaround otherwise).

mreppen commented 4 years ago

That makes sense. I tried a version check like in 327b1cfa805d1984f456669bdba5e66583a1371b and that works in neovim: https://github.com/mreppen/merlin/commit/9fc126dec7b1261dc0e749756f18f7b4dd88e13a My version is 0.4.3 so that is the most conservative check I can verify works. Was there a reason 327b1cfa805d1984f456669bdba5e66583a1371b was not merged or just the lack of somebody to test it?

My understanding is that vim never had this issue to begin with, so version check would not be necessary. FWIW, it does work with my vim 8.2 patches 1-148.

mreppen commented 4 years ago

@let-def @copy I can no longer reproduce.

Changes on my side:

This also works both on the commits from when I wrote previously and on the current master branch (as merlin's only change is the python version detection).

To me, this looks resolved.

mreppen commented 4 years ago

I retract my previous assessment. It still happens.

I "need" this to work, so I am keeping my own version pinned and up to date with a fix: https://github.com/mreppen/merlin/tree/nvimfix

It has the same version check discussed previously. I'd be happy to make a PR if that helps anybody.