clausreinke / typescript-tools

(repo no longer active) Tools related to the TypeScript language
Apache License 2.0
266 stars 29 forks source link

Add compatibility for neocomplete #42

Closed HerringtonDarkholme closed 9 years ago

HerringtonDarkholme commented 9 years ago

Typescript tools is a fantastic vim plugin. However, I found it does not work nicely with neocomplete, a autocompletion plugin for vim. I use this compatibility patch myself, but the update of typescript-tools incurs a maintenance pain. While this patch makes typescript-tools cooperates with neocomplete, it does not break manual omnicomplete (as opposed to other solution)

clausreinke commented 9 years ago

Perhaps I'm a bit slow, but I'm having a hard time figuring out what your patches are supposed to do: most changes seem to be reverted later, and I'm not going to add a buffer variable to preserve info across function invocations unless I have to.

Can you explain in words what you think the compatibility issue is? We've had such reports before, with other extended completion scripts, and usually, they turn out to work fine since the completion function API is fairly well defined for vim. It is always possible that there is still a bug in this code, though.

HerringtonDarkholme commented 9 years ago

NeoComplete will call the omnifunc three times, first two with findstart=1 and one with findstart=0. The first omnicfunc call will return the correct index(including the computation of the position of dot), but the second call, which gets wrong cursor position, returns a wrong completion start position. So the last omnifunc call also returns a list of completions based on a wrong position.

The patch here maintains a buffer variable to make the omnifunc idempotent, as long as the findstart is 1, the index it returned is the same as it is first called.

This requires document comment, indeed. If you think there's a chance for this patch to be merged, I will add explanation in the code.

clausreinke commented 9 years ago

That does sound like a NeoComplete issue to me: according to :help complete-functions, the function should be call twice, not three times, and as far as I can tell, our function does not move the cursor, so calling it again should not give a different result. Unless someone else moves the cursor.

HerringtonDarkholme commented 9 years ago

It's Vim's feature, though. To enable auto popup complete, the complete-functions have to be called multiple times. Various other plugins(eclim, jedi.vim, etc..) cannot be fixed because of their side-effect(e.g. modifying buffer) or asynchronous nature(which Vim disabled after plugin function is call). Other plugins, like tern, make complete functions idempotent by reading the whole buffer. Typescript-tools does have a chance, thanks to its side-effect free synchronous completion, to work nicely with NeoComplete even under Vim's (absurd) feature.

For more info, this issue should helps. https://github.com/Shougo/neocomplete.vim/issues/355

clausreinke commented 9 years ago

It's Vim's feature

yes, and typescript-tools follows Vim's spec for this feature. Your suggested change would make the completefunc violate that spec (eg, if interrupted after the first call, the next attempt to complete anything would give completely bogus results). All other users and completion plugins depend on me following the spec, so I won't make this change.

If you're absolutely sure you want this kind of completefunc, you can override and wrap TSScompleteFunc, though. Untested code follows:

let b:MyCompleteFunc_Counter = 0
function! MyCompleteFunc(findstart,base)
  let b:MyCompleteFunc_Counter = (b:MyCompleteFunc_Counter + 1)%3
  echomsg b%3
  return TSScompleteFunc(findstart,base)
endfunction
setlocal omnifunc=MyCompleteFunc

If you put something like this in elsewhere/ftplugin/typescript.vim, and make sure you add elsewhere to the end of rtp, after the typescript-tools path, then it will override and wrap TSScompleteFunc. In the wrapper, you can code your desired functionality.

HerringtonDarkholme commented 9 years ago

OK, thank for your suggestion. But I will still argue that omnifunc interruption also violates the spec since the second call of onmifunc shall followed the first one unless the first call returns an index less than zero( -1, -2, -3). A negative index is less than or equal to the buffer variable and makes buffer variable still spec compatible. Otherwise, if a index is non-negative, omnifunc should reset buffer variable to -1.