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

To maintain consisntency, use requests instead of notifications for changes #666

Closed
matklad opened this issue Jan 21, 2019 · 3 comments
Closed

Comments

@matklad
Copy link
Contributor

matklad commented Jan 21, 2019

We currently use notifications to inform server about document changes. This is unfortunate, because the client has to way to figure out which notifications were actually acknowledged by the server. This leads to a situation where client and the server might disagree about the state of the world without realizing that disagreement exists.

See #663 (comment) for a compelling argument why this is problematic (kudos to @rasika for formulating the example)

See #584 for a general discussion of synchronization issues.

This is obviously a breaking change, so we need to do this in the next major version of the protocol.

@matklad matklad changed the title To maintain consisntency, use requests and not notifications for changes To maintain consisntency, use requests instead of notifications for changes Jan 21, 2019
@dbaeumer
Copy link
Member

See my comment: #663 (comment)

@matklad
Copy link
Contributor Author

matklad commented Jan 23, 2019

I think we should mark it as "backlog" and reconsider when/if we will change the shape of notifications. For example #9 (using the same semantics of TextEdit[] as the rest of the protocol and removing redundant rangeLength) looks like something we might want to fix some day as well.

EDIT: I am also fine with just closing, although, imo, receiving responses won't hurt :)

@dbaeumer dbaeumer added this to the Backlog milestone Jan 25, 2019
@dbaeumer
Copy link
Member

I will close the issue since I don't see us changing this. For now things I stayed away from using notifications unless there is a compelling reason for it (no need for the client to wait for a result).

@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 27, 2020
@dbaeumer dbaeumer removed this from the Backlog milestone Nov 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants