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

Incremental synchronization support #3762

Closed
lnicola opened this issue Mar 28, 2020 · 2 comments · Fixed by #4153
Closed

Incremental synchronization support #3762

lnicola opened this issue Mar 28, 2020 · 2 comments · Fixed by #4153
Labels
E-has-instructions Issue has some instructions and pointers to code to get started good first issue

Comments

@lnicola
Copy link
Member

lnicola commented Mar 28, 2020

Spawned off #3758.

Some LSP clients (ahem, Gnome Builder) don't respect our choice for full text synchronization, and it wouldn't hurt to reduce the amount of text transferred.

The text change notification comes in here. params.content_changes contains all changes, but right now we only process one of them. Before passing the changes to the VFS layer, we need to convert from a line/column to byte offset representation. The latter is implemented as the TextRange type, and a conversion method is available here.

@lnicola lnicola added E-has-instructions Issue has some instructions and pointers to code to get started good first issue labels Mar 28, 2020
@matklad
Copy link
Member

matklad commented Mar 29, 2020

Another potential complication is that we store files with normalized line endings (\n), so we need to account for that. I think just normalizing the text from TextDocumentContentChangeEvent would do the trick, as ranges are ending-agnosic.

@lnicola

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-has-instructions Issue has some instructions and pointers to code to get started good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants