Completions while using LSP Mode aren't filtered properly

[lucminah]

Hello! I'm really excited about corfu and am happy to see it grow. Hope I can help in its improvement. The issue I've found is as follows:

When completing and using LSP Mode, filtering seems to apply to the already inserted text, but breaks with what comes after. The order of the candidates seems to be affected while typing, showing first the ones that actually match what's inserted, but non-matching candidates are also shown and not filtered away. Non-LSP completion works fine, and without corfu (with the default *Completions*), the filtering works as expected.

I've reproduced the problem in emacs -q. Apart from the default configuration of straight.el, I've only evaluated what's shown in the scratch buffer in the following demonstration (in which I used Python with pylsp, but the same behavior occurs in other modes such as C/C++ with clangd). Here's a replication of that code:

(straight-use-package 'corfu)
(setq tab-indent-always 'complete)
(corfu-global-mode 1)

(straight-use-package 'lsp-mode)

[minad]

Lsp-mode does not implement a correct completion table, see emacs-lsp/lsp-mode#2970. Maybe that's what leads to issues here.

[minad]

Closing. Should be fixed in lsp-mode, see emacs-lsp/lsp-mode#2970 and emacs-lsp/lsp-mode#2975.

[jdtsmith]

What is the state of using corfu with lsp-mode? Any advice? Orderless filtering/highlighting does not seem to pass through (unlike eglot).

Update — Aha, I see why:

completion-category-defaults is a variable defined in ‘minibuffer.el’.
Its value is ((lsp-capf (styles lsp-passthrough)))

which I suppose I could defeat. But if I just let the lsp server give me whatever it thinks in whatever order, it works OK. Honestly I really miss orderless filtering then.

One thing I notice company does that's great is after a . (in python-mode) it auto-summons completion immediately. I don't think corfu has a predicate that can be setup for reducing auto-prefix length.

[minad]

@jdtsmith LSP should work probably since the fix got merged. Unfortunately it overrides the completion styles (as you figured out) in order to not interfere with LSP filtering. But if you override this setting with orderless, orderless should work too. Please give it a check - this also depends on if LSP caches the candidates within the completion table!

[jdtsmith]

Good thoughts. I ripped out lsp-passthrough:

  (add-hook 'lsp-mode-hook (lambda ()
			     ;; Switch back to corfu and orderless
			     (company-mode 0)
			     (setcdr (cadr (assq 'lsp-capf completion-category-defaults))
				     '(orderless))
			     (setf (caadr ;; Pad before lsp modeline error info
				    (assq 'global-mode-string mode-line-misc-info))
				   " ")))

and found lsp-mode+corfu+orderless does indeed work well in a short test! This of course does the dreaded "post-LSP-contact" filtering, but that is what I want. I use filtering rather than arrowing down almost exclusively: just so much faster especially for lots of completions.

I do really wish I could configure auto popup to happen immediately after a ".", e.g. for deeply nested foo.bar.car.dee attributes. This is company default behavior. Perhaps a special corfu-auto-complete-predicate, or if that's too heavy, simply a list of chars like corfu-auto-complete-immediately-after-chars would do the trick?

BTW, like eglot, lsp-mode also does not "dump its completion cache" when the original string is altered/deleted. I think I'm actually convinced that requesting a new table function is the job of the completion UI in this very specific situation: if the entity at point in the buffer is changed beyond recognition, the completion table should no longer be considered valid and a new one should be requested. But we've already gone around on that a few times :). Note that this is still less dumping (by far) than company does (which his basically on every keystroke).

BTW, if you do hope to attract a good number of lsp users (which seem to be growing in number rapidly), I'd suggest:

1. Getting the lsp-mode authors not to hard-code in company, or at least provide an option to go without it.
2. Implementing a few visual niceties.

Comparing default corfu:

vs. default company:

Main improvement of the latter is the distinctive text styling on the annotation (easy), the right alignment (enabled with company-tooltip-align-annotations) and of course icons (less easy/but also probably less important). For the coloring, perhaps just mentioning a config option like:

(set-face-foreground 'completions-annotations "CornFlowerBlue")

would give people a bit of the company vibe (if they want it). I'd also suggest providing an option to right-align annotations. I mocked these both up and corfu then looks quite nice:

(though less space between annotation and bar would be better). Enjoying corfu very much thanks for all your efforts.

[minad]

• LSP: We should document the LSP configuration in the wiki (lsp/eglot configuration should be documented in the readme #71, PR welcome!). I tried convincing the lsp-mode authors to not hard code so much, but it was already difficult to convince them to not implement a broken completion table. And the current completion table is still not fully correct as I argued in the aforementioned PR. Of course one could still ask them to consider testing against Corfu too. But this shouldn't be my job - if multiple users request them to also consider Corfu, they may be more interested in it.

• Automatic popup after "." in OOP languages. I would like to have this feature, ideally one should investigate how Company implements this. See Auto trigger chars #70, PR welcome.

• Post-filtering/caching/dumping: We seem to agree on that keeping the completion table alive for some time is good for performance. I disagree that the completion table should be dumped, since the table is a lambda it can make the decisions for us. The current design is indicated by the API (and the call chain) we are implementing this on top. Corfu is a Completion in Region FUnction. The capf step happens first, at this point we got our hands on the completion in region table and Corfu takes over in a second step. Rerunning the capf at this point does not make sense, beyond calling the completion-in-region-mode-predicate to check if we are still within the completion field. Corfu takes a bit of freedom here and does not call this predicate by default in order to allow Orderless filtering. But you can configure Corfu to respect the predicate by setting corfu-quit-at-boundary=t.

• Implementing visual niceties. I would like to have right alignment, even by default. I had this implemented once but it wasn't robust. See Option to right-align annotations #18, PR welcome. On the other hand I've got even more critical regarding the icons - I don't like the icons at all that I am seeing in Company. I know these are the vscode icons, but they look meaningless.

• Documenting theme styling. Yes, we could do this in the README. I guess I would also like a PR in this direction. But it would be even better if interested users add Corfu support to their favorite theme. Currently Corfu is designed against the modus-themes and looks well there. These themes are the best in terms of accessibility and compatibility with packages. However people may prefer other themes for aesthetic reasons or a different taste in colors.

So it seems we agree on most of the points. I would like to encourage you and other users to make PRs regarding these features. Corfu's main issue is that it is young and not tested widely against different setups (capfs, programming languages, lsp-servers, themes, ...). Ideally this testing against all the different configurations and different programming languages is crowd sourced - I lack the time for doing this. Furthermore the second issue is that the entire Emacs ecosystem is developed against Company and Company added many extensions, which makes it hard for an alternative which is not entirely following the Company implementation. As I pointed out above, Corfu follows the capf/completion-in-region API quite closely, instead of adapting/retrofitting capf to the frontend as Company is doing.

You may have seen that Doom Emacs adopted Vertico/Consult/Embark/Orderless/Marginalia as their default completion UI. I have some hope that they will also add a Corfu module to their configuration, which will then help us sort out many more issues due to their well made preconfiguration of many packages. However this is more likely to happen if Corfu works well overall and ideally these visual issues should be sorted out first.

[jdtsmith]

Very good thanks for the detailed response. I'll see about PRs for right alignment and trigger characters. Since completions-annotations is the default face, probably just giving people info on how to set it suffices. I see modus does (completions-annotations ((,class :inherit modus-theme-slant :foreground ,cyan-faint))) so already pretty good.

If corfu does get adopted in Doom, that presumably would push the LSP/eglot's of the world to adapt, but I see your point; a bit of chicken and egg problem. I'll see if I can stimulate some movement there.

[minad]

I'll see about PRs for right alignment and trigger characters.

Looking forward to that.

Since completions-annotations is the default face, probably just giving people info on how to set it suffices.

I introduced a corfu-annotations face in 9249404 to allow more flexible styling. This is useful since the popup face does not necessarily have to match the minibuffer/completions-buffer face. I agree that adding a small section to the README documenting these style tweaks would be good. We should probably describe all the faces used by Corfu.

If corfu does get adopted in Doom, that presumably would push the LSP/eglot's of the world to adapt, but I see your point; a bit of chicken and egg problem. I'll see if I can stimulate some movement there.

As I see it, the best way to help is really to document and fix all the issue that occur on a case by case basis. As I said help is very welcome there, since testing all the different setups needs a lot of effort. Furthermore it is different if I only try something quickly to fix an issue in contrast to really working a while with a configuration. So the goal should be that new users don't immediately leave again after finding some issues. If everything works reasonably well, they may stick with it. For me Corfu works quite well, but I am only using it in a tiny subset of the possible scenarios. As it has turned out, due to the variety it is much harder to get Corfu right in comparison to getting something like Vertico right. But I guess the polyculture of minibuffer completion UIs has helped there in contrast to the de-facto Company monopoly. The name fits I guess :-P

[hmelman]

I'm not an lsp user and I only use company rarely (in python via elpy), but looking at the styling above I'll throw out this thought. If the annotation is just the type of the candidate (variable, method, property), rather than seeing those words right-aligned or not, I think I'd rather just see the candidate fontified via the font-lock properties for variable-names and function-names. Is this a config choice? Or append "()" to method candidate names.

[minad]

@hmelman The annotation function is provided by the backend. Corfu currently avoids applying fancy styling on its own. However Company has the extension company-kind which is used by Company to show these ugly icons. One could as well use company-kind to style the candidates accordingly. But for now I'd like not to focus on such superficial polishing.

[jdtsmith]

@hmelman See #38 for some other text-based prefix concepts, which does color-code based on "flavor". I had coded something up and forgotten it. In general aligning things which may have arbitrary display properties as well as including their own attempts at alignment is a challenge. What's needed is an agreement between the supply side of CAPF backends and the presentation side about who's in charge of what. Sticking with "everything is a fixed width character" radically simplifies things, but could be constraining.

[minad]

The idea of company-kind is to have a semantic symbol and the UI is in charge of styling. This is a good idea, but a deviation from how styling is done in completing-read, where for example the annotations are formatted by the backend.

[gagbo]

@jdtsmith In my case (lsp-mode, using Doom Emacs), to get "proper" orderless corfu completion with lsp-mode I had to :

(add-hook 'doom-init-modules-hook      ; This hook runs after shipped Doom code is ran and means "let me override anything that Doom did"
          (lambda ()
            (after! lsp-mode           ; Basically with-eval-after-load
              (setq lsp-completion-provider :none))))

(add-hook 'lsp-mode-hook
          (lambda ()
            (setf (caadr ;; Pad before lsp modeline error info
                   (assq 'global-mode-string mode-line-misc-info))
                  " ")))

(add-hook 'lsp-completion-mode-hook
          (lambda ()
            (setf (alist-get 'lsp-capf completion-category-defaults) '((styles . (orderless))))))

With the :none provider, almost nothing is done in lsp-completion-mode-hook : https://github.com/emacs-lsp/lsp-mode/blob/9865d315123bb0482de358e6905838ba1acb5b45/lsp-completion.el#L754 so I don't need to turn off company-mode, and if I want to avoid nil errors I directly setf the completion-category-defaults

[jdtsmith]

Thanks, right you are. I had opened an issue and got the same advice. I found the name quite counter-intuitive, since completions are certainly still provided by CAPF, just company is not setup if :none. I've suggested some simplifying language. Here's what I'm currently using for orderless, after setting lsp-completion-provider to :none.

  (defun my/lsp-mode-use-orderless ()
    (setf (alist-get 'styles
		     (alist-get 'lsp-capf completion-category-defaults))
	  '(orderless)))