company-lsp icon indicating copy to clipboard operation
company-lsp copied to clipboard

Problems with gopls autocompleting partially written functions

Open storvik opened this issue 5 years ago • 16 comments

Cannot get autocompletion of partially written functions to work with go-mode and gopls. completion-at-point seems to be working correctly.

I am able to reproduce using https://github.com/emacs-lsp/lsp-mode/blob/master/scripts/lsp-start-plain.el only adding go-mode to installed packages. Installed gopls by running go get -u golang.org/x/tools/cmd/goplsSee attached file for minimal example.tar.gz. Example must be placed in $GOPATH/src/test/ or similar in order to run.

What happens is when trying to autocomplete fmt. every possible candidate is showed, Printf, Print, Println, etc. But when trying to autocomplete fmt.Pri I get nothing. Checked output of *lsp-log* to find that candidate is returned from language server.

Output returned in *Messages* is:

No completion found user-error: Cannot complete at point

complation-at-point autocompletes fmt.Pri to fmt.Print however.

Checked (company-lsp--completion-prefix) to find that prefix is ("" . t) when autocompleting fmt. and (#("Pri" 0 3 (fontified t)) . t) when completing partially written function beginning with "Pri".

I also get loads of messages similar to LSP :: no object for ident Pri, but not sure this is related to company-lsp.

It seems to be working in other modes, so this may be related to Go and gopls?

Thanks in advance!

storvik avatar Apr 10 '19 09:04 storvik

@storvik

(add-to-list 'company-lsp-filter-candidates '(gopls . nil))

This line will sove your problem.

@tigersoldier I was unable to track down what is causing this behaviour.

yyoncho avatar Apr 10 '19 16:04 yyoncho

Thank you @yyoncho seems to be working perfectly now!

Not sure if this issue should be closed or if you want to keep it open?

storvik avatar Apr 10 '19 18:04 storvik

@storvik it should be open, we need to make it work out of the box.

@tigersoldier the problem is that the firlterText is the remaining part, e. g. "int($1)" (from Print).I am really puzzled how it is all supposed to work...

yyoncho avatar Apr 10 '19 18:04 yyoncho

It seems like gopls is sending back a "progressive" filterText. The client sends a complete request after fmt.P has been typed, so gopls gives results with a filterText that doesn't include the leading P. gopls seems to be assuming (not unreasonably) that subsequent filtering will start from after the P that has already been entered.

@stamblerre do you know any details about filterText? Is it supposed to be progressive like this, or is it possible it is supposed to take into account the entire prefix the user has typed so far? The spec doesn't really make it clear.

I tried to compare to other language servers, but Java and Rust don't seem to send filterText. Does anyone know of another language server that sets filterText?

muirdm avatar Apr 13 '19 20:04 muirdm

@muirrn you may try with clangd.

yyoncho avatar Apr 13 '19 20:04 yyoncho

Sorry for late response. I'll disable client-side filtering for gopls as a quick fix. A better solution will be comparing the filterText with and without the prefix. The best solution is to implement flx filtering.

tigersoldier avatar Apr 20 '19 05:04 tigersoldier

A better solution will be comparing the filterText with and without the prefix. The best solution is to implement flx filtering.

I think that unifing the filtering with what vscode does is a good idea since pretty much everybody is testing only with vscode - https://github.com/Microsoft/vscode/blob/d13f20e019044747729ab0370893e666090299cb/src/vs/editor/contrib/suggest/completionModel.ts . As far as I can see it automatically determines whether to use client side filtering (not sure how).

yyoncho avatar Apr 20 '19 07:04 yyoncho

I'll disable client-side filtering for gopls as a quick fix.

Thanks, that did the trick.

I looked at other clients/servers and opened a bug for gopls golang/go#31552 since gopls is the only one I found that behaves like this. Still not sure if filterText is allowed/intended to work like this.

I think that unifing the filtering with what vscode does

Sounds good to me.

muirdm avatar Apr 20 '19 16:04 muirdm

@tigersoldier we have problems with xml autocompletion.

The prefix is calculated incorrectly. If you have <tag the prefix will be calculated to tag while each of the items will be <tag which will remove all items from the list. At the same time, the workaround with disabling client-side filtering does not work since the xml server does not filter at all.

yyoncho avatar Apr 20 '19 16:04 yyoncho

@tigersoldier I did some experimentation with company-lsp--completion-prefix, I changed it to company-grab-symbol and it seems to fix the issues with clojure and xml. Do you know a language server that won't work with that approach and requires prefix calculation using trigger chars?

yyoncho avatar Apr 29 '19 07:04 yyoncho

@yyoncho I introduced trigger chars calculation because it doesn't work for Java when annotation is involved. The prefix will be @Override instead of Override.

Flex matching is on its way. Stay tuned.

tigersoldier avatar Apr 29 '19 15:04 tigersoldier

I did some debugging and narrowed down how vscode works. VScode fetches the current word(i. e. prefix) depending on the currently defined syntax. Guess what, if you are editing xml and you have <tag the prefix is <tag but when you are editing html using the html language server and you have <tag the prefix is tag. And since everybody is testing only against vscode that this behaviour is inherited by the servers. Note that flex matching wont solve that problem if the word that you have calculated is not the right one.

yyoncho avatar Apr 29 '19 15:04 yyoncho

FYI https://github.com/Microsoft/vscode/blob/a3099d3e10e47d5060bf73beb5c6fd5bae0567a9/src/vs/editor/common/model/textModel.ts#L2017

yyoncho avatar Apr 29 '19 15:04 yyoncho

@yyoncho If we are calculating prefix as <tag, the HTML language server will be broken. However, if we are always removing the trigger character, the prefix may be a substring of the prefix expected by the server (in this case the XML slanguage server). In this case flex matching should work.

tigersoldier avatar Apr 30 '19 05:04 tigersoldier

@tigersoldier I agree that it will work in this case. You know what is the problem - if the word separator is not considered as a trigger character this might cause failure client side. IMO the proper fix is server to return the match instead of letting the client to use some heuristics to guess what the server has used as a prefix...

yyoncho avatar Apr 30 '19 07:04 yyoncho

This issue has been fixed in gopls for a while. We can probably close this issue (and re-enable the client side filtering, if desired).

muirdm avatar Jul 12 '19 16:07 muirdm