Skip to content

Idle timer for lsp-on-change #239

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

Closed
wants to merge 1 commit into from

Conversation

topisani
Copy link

Accumulate the change-event parameters in lsp-on-change and limit them with an idle timeout. Also make sure these events are then sent before sending any other requests/notifications. lsp--send-did-change does the actual sending if any changes are lined up.

This was sparked over at emacs-lsp/lsp-ui#45, and seems to be the best solution overall, since the didChange events are sent extremely frequently by default. This is a nice place to do performance tuning.

We are still testing it a bit with cquery, but it would be great if someone could try it out with some other servers!

@alanz
Copy link
Contributor

alanz commented Jan 19, 2018

We used to have this, but the problem is that there is no way to combine multiple changes into a single change message that is meaningful to the LSP server.

@alanz
Copy link
Contributor

alanz commented Jan 19, 2018

See #112

@yyoncho
Copy link
Member

yyoncho commented Oct 2, 2018

@alanz

We used to have this, but the problem is that there is no way to combine multiple changes into a single
change message that is meaningful to the LSP server.

Why do we need to merge them in single document change message? Can we collect the N changes and send N document change notifications without the need to merge them into single one.

@alanz
Copy link
Contributor

alanz commented Oct 2, 2018

Can we collect the N changes and send N document change notifications without the need to merge them into single one.

@yyoncho what is the benefit of this running on a timer? Vs sending them as they happen, as now.

@yyoncho
Copy link
Member

yyoncho commented Oct 2, 2018

@alanz Hm, I believe that you are right - it won't benefit a lot(theoretically if the server has delay it will increase the chance to skip rebuilt). I think that the changes can be reavaluated to work as they are performed on the original document as you have mentioned in #112

@yyoncho
Copy link
Member

yyoncho commented Oct 17, 2018

Denying the PR as it is as per #239 (comment) .

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

Successfully merging this pull request may close these issues.

3 participants