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.72k stars 861 forks source link

lsp-completion-at-point: looking back a few more chars for trigger-char #4428

Closed kiennq closed 2 months ago

kiennq commented 2 months ago

Fix #4419

jcs090218 commented 2 months ago

We are using company-specific variables. Should we handle it for users who don't use company-mode?

kiennq commented 2 months ago

We are using company-specific variables. Should we handle it for users who don't use company-mode?

I guess no. This is company-specific property anyway. Using this with corfu has no issue.

guillaumebrunerie commented 2 months ago

It does break with corfu for me, I get (void-variable company-minimum-prefix-length) at every character I type.

guillaumebrunerie commented 2 months ago

Ok, I managed to fix locally it by replacing

(defvar company-minimum-prefix-length)

by

(defvar company-minimum-prefix-length 1)

(i.e. actually providing a value to that variable, not sure if that value matters)

But I think it is pretty weird to have a defvar here, as the variable created by it will be global anyway (if I understand correctly). Why not use boundp instead to check if the variable exists, and do something else instead?

Also I guess that if we load company-mode after lsp-mode, then company-mode won’t be able to provide its own default value and docstring to the defvar'd variable?

jcs090218 commented 2 months ago

cc @kiennq

kiennq commented 2 months ago

Ok, I managed to fix locally it by replacing

(defvar company-minimum-prefix-length)

by

(defvar company-minimum-prefix-length 1)

(i.e. actually providing a value to that variable, not sure if that value matters)

But I think it is pretty weird to have a defvar here, as the variable created by it will be global anyway (if I understand correctly). Why not use boundp instead to check if the variable exists, and do something else instead?

Also I guess that if we load company-mode after lsp-mode, then company-mode won’t be able to provide its own default value and docstring to the defvar'd variable?

Oh, I forgot the case where company-mode doesn't exist at all. Will check using boundp instead

kiennq commented 2 months ago

Btw, to be clear, this issue only happens in lsp-mode and not other is because we're the only one who doesn't trust the completion boundary gotten by symbol at point of Emacs. The boundary should be provided by the lsp server and we use a heuristic to determine that after gotten the data from the server. That also means the boundary/prefix for each candidate is different.