search

emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.79k stars 889 forks source link

Auto-completion doesn't seem to work when `company-minimum-prefix-length` <= 1 #2624

Closed anonimitoraf closed 2 years ago

anonimitoraf commented 3 years ago

Describe the bug Auto-completion when:

To Reproduce Sample project: https://github.com/anonimitoraf/issue-reproduction-clojure

Open src/clojure_sandbox/core.clj and with different values for company-minimum-prefix-length, try to auto-complete (a/so)

Expected behavior company-minimum-prefix-length set to 0 or 1 should suggest auto-completion entries

Which Language Server did you use clojure-lsp

OS 5.9.16-1-MANJARO (via uname -r)

ericdallo commented 3 years ago

@kiennq or @yyoncho do you know why this happens?

yyoncho commented 3 years ago

I did short debug session on that and I was unable to find out the reason. I confirm the issue is happening only with Clojure, it works fine with the other servers I tested with.

ericdallo commented 3 years ago

@yyoncho odd, the request is being made correctly to the server?

yyoncho commented 3 years ago

I don't see requests at all.

ericdallo commented 3 years ago

Got it, so indeed lsp-mode is not requesting for some reason, really curious

yyoncho commented 3 years ago

most likely it is related to prefix calculation, @kiennq knows much better than me on that.

anonimitoraf commented 3 years ago

Hey @yyoncho @ericdallo thanks for giving this a glance

I can also check it out, any good starting points? Files to look at, for example

yyoncho commented 3 years ago

@anonimitoraf thank you for the interest but for this particular issue I believe that @kiennq is the best person to investigate because the completion code is not very intuitive due to the complexity of the emacs completion-at-point facilities. But still, if you want to give it a try - start from lsp-completion-at-point.

kiennq commented 3 years ago

@anonimitoraf Did it work if you set lsp-completion-no-cache to t?

anonimitoraf commented 3 years ago

@kiennq I see. Yes with lsp-completion-no-cache set to t, company-min-length of 1 works :D

Although with company-min-length of 0, it only pops up after I've typed a letter or 2. When I try to open up the completion box manually, it tells me "No completion found"

I'm thinking that while it might be nice to open up the completion box with 0 chars typed, for the sake of "discoverability", it sounds like this could potentially be inefficient (because: LOT of candidates)

Side question: Is setting lsp-completion-no-cacheto t all the more inefficient? What happens when it is set to t?

kiennq commented 3 years ago

Also can you post the trace io log here? Changing prefix-length is working in my env though. I would not recommend setting lsp-completion-no-cache since it will not using cache, and will make the typing slower.

When I try to open up the completion box manually, it tells me "No completion found"

This seems to be clojure language server limitation. Setting min prefix length to 0 is not that different from 1 since company will kick in only after you type something (means the length already >= 1). Also some character is guarded and may not trigger company (( for example), so with min length of 0, the completion will be trigger likely after word separation characters (spaces) only. Combine that with language server limitation, it will likely make you see no different at all.

Note that, just type ( and trigger the completion at point (manually, since it will not be triggered automaticially) will make server returns no result. That value will be cached and make the subsequence typing with same prefix at the same position will not query the language server. It's expected behavior.

ericdallo commented 3 years ago

Yeah, I can confirm server is not returning any results for a min length of 0, this could be fixed on server side indeed :)

ericdallo commented 3 years ago

Fixed the completion with 0 length on server side @anonimitoraf :) https://github.com/clojure-lsp/clojure-lsp/commit/fac945d1d452295adeac04f768d1b37fea0f3e70

yyoncho commented 3 years ago

@ericdallo is there anything to address on the client side after your server fix?

ericdallo commented 3 years ago

I think the issue with the 1 length not working without change the cache on completion? Let me try it

ericdallo commented 3 years ago

I tested with company-prefix-length of 1 and it worked when idle as expected

anonimitoraf commented 3 years ago

@kiennq Ah, I see. Makes sense.

And damn @ericdallo awesome. Thanks so much! I'll have a read through your commits out of curiosity

yyoncho commented 2 years ago

seems like this issue is already fixed.