-
Notifications
You must be signed in to change notification settings - Fork 780
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
Multiple text document changes when setting the entire content #289
Conversation
It's unclear how a language server should react when multiple changes to the text document are sent, and one or more set the content of the entire document. The convention so far among the language servers I've seen has been to apply the changes from the bottom of the document to the top. However, there is no obvious ordering for the whole-document changes. Should we apply the changes in the order that they are received (so that earlier whole-document changes are overridden by later whole-document changes), except that within consecutive subsequences of range-changes we apply them in bottom-to-top order? This is a non-trivial amount of complexity for a situation that probably never arises in practice. Most likely the editor will either set the entire text of the document, or set ranges for the changes to the document (where, in some cases, the range may be the whole document). This commit clarifies this by forbidding whole-document changes in conjunction with other changes. It's also in line with the idea of forbidding overlapping edits as per #279.
This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request. |
@arxanas forbidding this is not necessary since the content changes describe single state moves. So if you have two content changes c1 and c2 for a document in state S10 then c1 move the document to S11 and c2 to S12. So the array of content changes should simply be applied from I updated the documentation accordingly. |
@dbaeumer: Is there discussion of that anywhere else? I fear that this is incompatible with how most editors are currently treating this: as a vehicle to support features like multiple cursors. In those cases, one can make changes to multiple places in the document simultaneously during the course of normal editing. What is the point of supporting multiple changes in a single I assumed that the point of having multiple changes in a Does VS Code have any multiple-cursor feature, and what does it do in that case? cc @ljw1004: the corresponding commit can be found at fcb32f9 |
The only reason here is performance / batching. Regarding multi cursor: the assumption is that the server doesn't need to know anything about multi cursor. For example if there are two cursor and the user types a we generate a change with two content changes but treat them as individual states. IMO there is no difference whether this would be one or two state changes. The client could even simulate one state change by setting the right version number in the The important thing is that the content changes must be applied in order without the need for adjusting any positions which IMO is the easiest model for a server. |
In the order given in the message, or from the end of the file to the front? |
Actually, it doesn't really matter. Since the edits do not overlap, the order in which you apply them shouldn't matter.... however... The caveat is that you have to think of all position references in the edits in terms of the coordinates of the original document to which all these edits are being applied. This means that if you have changes in the beginning of the document then, if you apply them first, then you have to adjust all the positions of the other edits, as the coordinates have been changed by the earlier edits. Now... if you sort the edits based on position and then apply them starting from the end of the document, then you don't have to worry about adjusting the other edit positions because they are unaffected. So, basically, you can think of the sorting as only a nice / easy / efficient way to implement applying a Multi-change. |
@kdvolder: I think that's not what @dbaeumer is saying, although this was my initial assumption. I believe the clarification of the spec is that you simply apply them in the order that they're received, without adjusting any positions, and you get the right results. |
@arxanas I think you are right. For a document change event, the changes in the array are to be applied in order, and if position shift around because of that, I'd assume the client doesn't have worry about that because the next change is meant to be applied to the result of the previous change. But... the order matters. I would also assume that changes can be overlapping in this case. What I was talking about was this: The similarity between document edits and document change events had gotten me all confused. Change events are sequential and have a definite ordering to them. Whereas DocumentEdits on the other hand are non-overlapping and their order shouldn't affect the semantics. |
Well, if the "adapted in order" interpretation is correct than I can revert emacs-lsp/lsp-mode@3d230b7 and go back to batching up changes for emacs. |
I renamed a variable in VS Code and I received the following two changes. Applying them in order is certainly fine given that the first modification is on line 10 and the second is on line 9 so no offsets have to be recalculated here upon application of the first change. Seems like I revert my language server's sorting code and open bugs against LSP clients that aren't sending changes in this order. [
{
"range": {
"start": {
"line": 10,
"character": 1
},
"end": {
"line": 10,
"character": 4
}
},
"rangeLength": 3,
"text": "var2"
},
{
"range": {
"start": {
"line": 9,
"character": 1
},
"end": {
"line": 9,
"character": 4
}
},
"rangeLength": 3,
"text": "var2"
}
] |
@kdvolder your final conclusion is correct. If you need further clarification please let me know. |
It's unclear how a language server should react when multiple changes to the text document are sent, and one or more set the content of the entire document.
The convention so far among the language servers I've seen has been to apply the changes from the bottom of the document to the top. However, there is no obvious ordering for the whole-document changes. Should we apply the changes in the order that they are received (so that earlier whole-document changes are overridden by later whole-document changes), except that within consecutive subsequences of range-changes we apply them in bottom-to-top order?
This is a non-trivial amount of complexity for a situation that probably never arises in practice. Most likely the editor will either set the entire text of the document, or set ranges for the changes to the document (where, in some cases, the range may be the whole document). This commit clarifies this by forbidding whole-document changes in conjunction with other changes. It's also in line with the idea of forbidding overlapping edits as per #279.