Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

corfu for variables not working in php-mode with serenata ls #3817

Closed
2 of 3 tasks
xendk opened this issue Nov 20, 2022 · 3 comments · May be fixed by #3825
Closed
2 of 3 tasks

corfu for variables not working in php-mode with serenata ls #3817

xendk opened this issue Nov 20, 2022 · 3 comments · May be fixed by #3825
Labels

Comments

@xendk
Copy link
Contributor

xendk commented Nov 20, 2022

Thank you for the bug report

  • I am using the latest version of lsp-mode related packages.
  • I checked FAQ and Troubleshooting sections
  • You may also try reproduce the issue using clean environment using the following command: M-x lsp-start-plain

Bug description

When using corfu as completion frontend rather than company, assuming a defined $varname, completing $var doesn't work. Typing the var prefix without the $ does work, and adds the missing $.

@minad says it must be a lsp-mode issue.

Steps to reproduce

Haven't been able to make one yet. lsp-start-plain didn't make me wiser as it only have company (which works).

Expected behavior

That $varname can be completed by typing $var rather than var.

Which Language Server did you use?

PHP, serenata

OS

Linux

Error callstack

No response

Anything else?

No response

@xendk
Copy link
Contributor Author

xendk commented Nov 24, 2022

I've figured it out.

I'd followed minads instructions for setting up corfu with lsp-mode, which configures the completion style for lsp-capf.

The problem with that is that lsp-mode configures its own lsp-passthrough style, which the above configuration then overwrites.

The reason why it breaks variable completion with the serenata language server (might not be the only one, but that what I have available) without lsp-passthrough, is that when completing $var, the server returns a completion with the label var_name, but
what's really inserted is $var_name. So when orderless (and other) completion styles get a candidate of var_name and see a prefix of $var they filter it out.

Removing the configuration from the wiki doesn't help, as that kills all variable filtering and just presents a popup with all the variables (makes sense, corfu lets the completion style handle filtering, and lsp-completion-passthrough-all-completions just
parses everything through). I guess company must do its own post-filtering for it to work.

This is not limited to corfu, vanilla completion-at-point has the same problems, as would any other completion front-end that doesn't know about lsp-mode.

I've read up on the PR #2975 and the issue #2970, which is what introduced lsp-passthrough, and it's clear there's different opinions on the importance of keeping to standard capf, and being able to take advantage of what the language server actually provides.

I've come up with a possible solution, see #3825

@minad
Copy link

minad commented Nov 24, 2022

I guess company must do its own post-filtering for it to work.

Company restarts the Capf on every keypress, such that the language server is called again on every keypress. Therefore the passthrough style works for Company. Corfu works differently. It calls the Capf once, obtains the completion table and caches it. Then the candidates are filtered using the completions style. In case you want to emulate the Company behavior you can use the cape-capf-buster, which busts the cache and recalls the Capf on every keypress.

The advantages of the Corfu behavior are efficiency thanks to the cache and the flexibility offered by the completion styles. You can for example use Orderless filtering on the cached completion table. The downside is that you don't get immediate updates from the Lsp server, but you could use the cape-capf-buster if you prefer that and if you can live with the lower completion performance.

@xendk
Copy link
Contributor Author

xendk commented Feb 12, 2024

Closing this as the problem seems limited to the one server, which is minimally maintained these days anyway.

@xendk xendk closed this as completed Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants