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

Choosing a low idle-timeout time causes a spam of "content changed" errors with Rust Analyzer #7497

Closed
dob9601 opened this issue Jul 1, 2023 · 12 comments · Fixed by #8021
Closed
Labels
C-enhancement Category: Improvements

Comments

@dob9601
Copy link
Contributor

dob9601 commented Jul 1, 2023

While this is arguably intended behaviour, its not documented anywhere and hard to figure out the cause of - especially when it doesn't occur on another machine (because that machine is faster). The errors appear every few seconds while typing and are rather distracting.

It might be worth adding some documentation surrounding this side effect (attached to where idle-timeout is discussed in the docs) so that others who encounter this can figure out the solution faster.
Alternatively, the error message could be suppressed (or at least not shown in the commandline repeatedly) since it conveys no useful information and LSP still behaves fine.

Update: Suppressing the message would be more preferable. Choosing a slower timeout time makes working in other languages with faster language servers painful

@dob9601 dob9601 added the C-enhancement Category: Improvements label Jul 1, 2023
@the-mikedavis the-mikedavis added the A-documentation Area: Documentation improvements label Jul 1, 2023
@pascalkuthe
Copy link
Member

using an idle timeout of 0 is absolutly not recommended (in fact its something we advise against that will cause issues). If you want faster completion you can look at using #8021. I think that should likely fix those errors. If it doesn't fix the issue we will have to look into a retry strategy (RA completion model is a bit at odds with our completeion model because they cancel running completion requests when the file changes again while we assume a snapshot like behaviour, this would probably be fixed once we support incomplete completion requests tough)

@Tudyx
Copy link
Contributor

Tudyx commented Sep 9, 2023

I have exactly the same problem (#8196). I've tried using the new event system #8021 but it didn't work by my side. For me it's even worse, I have to increase the default idle time not to be spammed by the message but as the consequence the editor feel very laggy.

You mentioned that it might be solved if we support incomplete completion requests, could you please give some hint how you would do that. As it's a real pain for me, I want to submit a PR for this.

@dob9601
Copy link
Contributor Author

dob9601 commented Sep 9, 2023

using an idle timeout of 0 is absolutly not recommended (in fact its something we advise against that will cause issues). If you want faster completion you can look at using #8021. I think that should likely fix those errors. If it doesn't fix the issue we will have to look into a retry strategy (RA completion model is a bit at odds with our completeion model because they cancel running completion requests when the file changes again while we assume a snapshot like behaviour, this would probably be fixed once we support incomplete completion requests tough)

Just to clarify - I wasn't using an idle time of 0. I was using a time that happened to sit on the threshold where completions would work fine on my desktop but spit out errors on my laptop

@pascalkuthe
Copy link
Member

pascalkuthe commented Sep 9, 2023

I have exactly the same problem (#8196). I've tried using the new event system #8021 but it didn't work by my side. For me it's even worse, I have to increase the default idle time not to be spammed by the message but as the consequence the editor feel very laggy.

You mentioned that it might be solved if we support incomplete completion requests, could you please give some hint how you would do that. As it's a real pain for me, I want to submit a PR for this.

the event system is a prerequisite for supporting incomplete completion lists. I considered including it with that PR but its already quite large. Fixing this will also not be easy and requires some thaught on how to handle this. The issue is that RA assumes that all editors use the exact same completion system as VSCode and cancels completion requests according to that assumption. I think I may be able to work that into the event system tough.

@pascalkuthe pascalkuthe removed the A-documentation Area: Documentation improvements label Sep 9, 2023
@pascalkuthe
Copy link
Member

This should be fixed by #8021. I pushed some changes so that completion requests are always reastatet when typing more text (which matches vscode).

The "content modified message" actually should not appear anyway since #8021 changes completion errors to log messages (so not shown in the UI anymore). You may have tested an old version of that PR @Tudyx

@pascalkuthe pascalkuthe linked a pull request Sep 9, 2023 that will close this issue
@Tudyx
Copy link
Contributor

Tudyx commented Sep 9, 2023

I was testing with 272e679 which seems to be the most up to date at the moment but I might missed something.
Now I tested with the latest additions (36e70b4) and the events are still shown in the UI by my side. @dob9601 could you give it a try? Maybe I do something wrong

@pascalkuthe
Copy link
Member

pascalkuthe commented Sep 9, 2023

are you using inlay hints? Those were not touched by that PR (to keep it smaller) and are also related to the idle timeout. I don't think its related to completions completions at this point because we just log the error but don't show it in the UI in any way:

https://github.com/pascalkuthe/helix/blob/36e70b4217293fa9d747e87e623dc0430114fcb2/helix-term/src/handlers/completion.rs#L214

@Tudyx
Copy link
Contributor

Tudyx commented Sep 9, 2023

Yes I'm using inlay hints, I will disable them and give it a try tomorrow to see if I still get those errors in the UI

@Tudyx
Copy link
Contributor

Tudyx commented Sep 10, 2023

I confirm than disabling inlay hints fix the spamming both for the UI and the log file. This work for #8021 and also for the master branch.

@dob9601
Copy link
Contributor Author

dob9601 commented Sep 10, 2023

Can also confirm on my machine

I confirm than disabling inlay hints fix the spamming both for the UI and the log file. This work for #8021 and also for the master branch.

@pascalkuthe
Copy link
Member

pascalkuthe commented Sep 10, 2023

Yeah requesting inlay hints takes a while and also causes weird visual artifacts if done often (vscode has a quite high timeout by defaukt).

I do want to change that and was a strong motivator for both #8021 and #6447 (the word based mapping behavior there is needed for mapping inlay hints).

However with #8021 you can simoly remove any custom idle-timeout setting from your config. With that PR the idle timeout does not affect completions anymore so setting it to a high value would fix thus issue without side effects.

@Tudyx
Copy link
Contributor

Tudyx commented Sep 10, 2023

I did switch to #8021, reactivate inlay hints and remove the idle-timeout setting and it worked like a charm! Thank you, you're the boss

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants