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

Auto mode #92

Closed
xendk opened this issue Nov 27, 2021 · 14 comments
Closed

Auto mode #92

xendk opened this issue Nov 27, 2021 · 14 comments

Comments

@xendk
Copy link

xendk commented Nov 27, 2021

Corfu looks very promising, but I've run into a few problems early on.

Auto-mode is extremely slow for me. Even though it's configured with the 0.2 sec delay, when typing (exec in an elisp buffer it takes 3 seconds before the popup appears. However, if I move about and return to the same position (to get rid of the popup), running completion-at-point makes the popup appear instantaneously.

In the scratch buffer I get no popup at all, though completion-at-point still works.

Corfu doesn't know how to highlight odd matches:
image
Somehow company manages to get it right:
image
(and can filter out the non-matching, but I seem to recall having to set some odd variable, but bugger me if I can remember).

I'm a big fan of consult, embark and marginalia.

@minad
Copy link
Owner

minad commented Nov 27, 2021

Auto-mode is extremely slow for me. Even though it's configured with the 0.2 sec delay, when typing (exec in an elisp buffer it takes 3 seconds before the popup appears.

This is probably an issue with your configuration. Try to reproduce this with a minimal configuration, starting from emacs -Q. The issue is probably that other modes eat up computation time, which starves Corfu. Corfu uses run-with-idle-timer. I had a similar report where flyspell was the problem. Maybe you use an aggressive flymake, flycheck or other linter setting? You may want to profile your setup. On my setup corfu-auto in elisp buffers is reasonably fast.

Corfu doesn't know how to highlight odd matches:

What do you mean by odd matches? It seems that filtering does not work at all. I assume you use lsp-mode, maybe an outdated version? @jdtsmith told me that on his setup lsp-mode works well, but it requires some tweaks.

I'm a big fan of consult, embark and marginalia.

Thanks!

@xendk
Copy link
Author

xendk commented Nov 27, 2021

I had a similar report where flyspell was the problem.

Oh, I'm using flyspell. What was the solution? I have no idea how to profile this.

What do you mean by odd matches?

See the mockMessageCallback? How can that match getC, it doesn't even have a t (unless it's matching the t in protected which is a bit odd). But look at how company highlights changeSubscription. Corfu seems to just highlight the initial characters, which would be fine if lsp only returned suggestions that matches the prefix.

I think the configuration I can't seem to remember was something that made company re-filter the reply from lsp. Which would explain how it could figure out what matched where. I can't see how Corfu could know how the lsp server thought that a given candidate matched the prefix.

@minad
Copy link
Owner

minad commented Nov 27, 2021

Oh, I'm using flyspell. What was the solution? I have no idea how to profile this.

First try if it helps if you disable flyspell. Then one can look into possible solutions. But I've heard often that flyspell is too costly, so one may want to enable it only occasionally. If flyspell is not the problem, try to disable all kinds of different modes you have enabled. Some of which may add post-command-hooks and costly timers which then compete with Corfu. Although I wonder why Company does not have this issue.

See the mockMessageCallback? How can that match getC...

Yes. The filtering does not work. Do you use the newest lsp-mode and the newest Corfu? There was a bug in lsp-mode, but this should be fixed by now.

@jdtsmith
Copy link
Collaborator

jdtsmith commented Nov 27, 2021

Looks to me like what has happened is that corfu completion popped up on $this->get, just before you added the C. So the system is not filtering results correctly (you can test that by typing getC quicker). What completion style(s) are you using? Here's my lsp-mode completions with pyright server and corfu (which popup instantaneously):

image

Check out kind-icon if you like the icons. I use the "orderless" completion style, which is strange but strangely amazing. If you are using the default lsp-passthrough style, you have discovered that it doesn't do much of anything, and that lsp-mode sadly does not recompute candidates from scratch correctly in this case. It works with company since company tends to drop completions on any new input and asks for new ones. I find local filtering to be much faster.

@minad I know you were involved in getting lsp-passthrough setup, does that seem like a bug with that simple style?

Update: just tested a new lsp-mode with it's lsp-passthrough set, and I can confirm a complete lack of client-side filtering (it does at least sort them!).

@minad
Copy link
Owner

minad commented Nov 27, 2021

@jdtsmith Oh right, lsp-passthrough defeats the filtering. I forgot this, I wasn't deeply involved into the lsp-mode fix. The user has to overwrite the completion style.

This should be documented at some point, seee #71 (comment). We should document both configurations, one with builtin completion styles basic/substring/flex and one with orderless. Can you paste your config here, @jdtsmith? The part where you adjust the completion style.


See #41 (comment)

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

@galeo
Copy link
Contributor

galeo commented Nov 27, 2021

First try if it helps if you disable flyspell.

I can confirm that disable flyspell would solve the auto problem. I only turn on flyspell for comments and strings in prog mode. The corfu auto mode can not work properly and will lead to big delay.

@minad minad closed this as completed in 9ffdbae Nov 27, 2021
@minad minad reopened this Nov 27, 2021
@minad
Copy link
Owner

minad commented Nov 27, 2021

Okay, I fixed the issue with flyspell-mode by using run-at-time instead of run-with-idle-timer. See 9ffdbae. This will make Corfu more deterministic, but also potentially more costly.

I hope we can also fix @xendk's remaining lsp-mode issue, which is caused by the completion style configuration.

@jdtsmith
Copy link
Collaborator

Check the wiki for a new section on configuring corfu with lsp-mode. I've come to the conclusion that lsp-mode really works best with a style like orderless, so that's what's shown there. Other modes can work too (even basic), but since lsp-servers have their own "style" ideas of what to complete, if you want to see most/all of their results, you need a flexible style.

@minad
Copy link
Owner

minad commented Nov 27, 2021

@jdtsmith Thank you! This is a big help. I have two question:

  1. I wonder how well orderless filtering works, if you enter "foo bar". Does lsp-mode drop the candidates at some point? Then the server is asked again, this time with "foo bar" in the buffer, which will not yield results? If this is the case one would need a cape-capf-with-cache/cape-capf-retainer, which does not ask the table again for candidates as soon as you enter a space as separator.
  2. Since orderless filters the candidates, does it make sense to enable the orderless-flex matching style? The server may return flex filtered candidates, so without orderless-flex we could lose candidates.

@jdtsmith
Copy link
Collaborator

jdtsmith commented Nov 27, 2021

  1. Nope it holds onto candidates after space, so you can filter to your heart's content with as many terms as you like.
  2. Was just working on that!

In the config example, I have now setup orderless to use flex-matching for its first search term. This means you get absolutely positively every candidate the server returned.

I honestly regard this as optional, since many people I think would regard those flex-matched "candidates" the server has supplied as basically clutter not very related to their search term. So normal orderless "style" is perfectly generous for me (and faster). I mean, how likely was it I meant np.ar -> np.FLOATING_POINT_SUPPORT?

image

Not very!

@minad
Copy link
Owner

minad commented Nov 27, 2021

@jdtsmith I expanded the wiki documentation slightly, such that we have three configuration examples, from basic to more advanced. I think it is helpful to give simpler examples first and I don't want to give the impression that Cape is strictly needed for this to work. Please take a look! Thank you again for your help - your documentation is well written!

@xendk If you find time, please give it another try! See the documentation in the Wiki. The slowness issue in conjuction with flex mode should also be fixed if you use the newest Corfu version from git. I am looking forward to your feedback, if it indeed works well now.

Closing. The issues should be fixed and documented. If there are further questions please let us know.

@minad minad closed this as completed Nov 27, 2021
@jdtsmith
Copy link
Collaborator

Looks good, I fixed a few typos. But I see you added pattern in the dispatcher: (and (eq index 0) 'orderless-flex pattern))? The orderless dispatchers are allowed to return nil, in which case later dispatchers on the list will be used.

@minad
Copy link
Owner

minad commented Nov 27, 2021

I removed the pattern, this was also a typo.

@xendk
Copy link
Author

xendk commented Nov 27, 2021

Okay, I fixed the issue with flyspell-mode

Excellent! This made me notice that my cursor doesn't blink while flyspell is doing its thing. It's rather a lot of resources it's using, innit? Hopefully someone will look into that some day.

After updating lsp and setting the style to basic, things looks right. Thanks @minad and @jdtsmith Now on to fiddling with stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants