Fix LSP didChange parameters #4978 #5002
Conversation
|
Can one of the admins verify this patch? |
| return text; | ||
| } | ||
|
|
||
| public DocumentHandle getDocument() { |
There was a problem hiding this comment.
getDocumentHandle() maybe better name for this method we can avoid getDocument().getDocument() in this case
There was a problem hiding this comment.
I left it like that for consistency with "DocumentReadyEvent"
There was a problem hiding this comment.
Look like we already have similar event https://github.com/eclipse/che/blob/master/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/editor/events/DocumentChangeEvent.java#L16-L16
There was a problem hiding this comment.
Ah, DocumentChanging is sent before the document is changed. It is necessary to compute the coordinates BEFORE the document changes. Both events are necessary. Maybe rename the other event to "DocumentChanged"?
There was a problem hiding this comment.
@tsmaeder Any update here?
There was a problem hiding this comment.
As I said, both events are necessary. I was trying to get your opinion on the rename.
There was a problem hiding this comment.
DocumentChanged is ok for me
benoitf
changed the title
benoitf
changed the title
tsmaeder
force-pushed
the
4978_fix_did_change_parameters
branch
from
c04376e
to
16137b5
Compare
|
Can one of the admins verify this patch? |
Signed-off-by: Thomas Mäder <tmader@redhat.com>
parameters Signed-off-by: Thomas Mäder <tmader@redhat.com>
Signed-off-by: Thomas Mäder <tmader@redhat.com>
tsmaeder
force-pushed
the
4978_fix_did_change_parameters
branch
from
16137b5
to
938c9c8
Compare
Signed-off-by: Thomas Mäder <tmader@redhat.com>
tsmaeder
force-pushed
the
4978_fix_did_change_parameters
branch
from
938c9c8
to
bf33dcf
Compare
| document.getDocumentHandle().getDocEventBus().addHandler(DocumentChangingEvent.TYPE, new DocumentChangingHandler() { | ||
| @Override | ||
| public void onDocumentChanging(DocumentChangingEvent event) { | ||
| lastEventStart= event.getDocument().getDocument().getPositionFromIndex(event.getOffset()); |
There was a problem hiding this comment.
I beg you please format these code lines!
| @@ -70,7 +78,24 @@ private void fireDocumentChangeEvent(final ModelChangedEventOverlay param) { | |||
|
|
|||
| String text = editorOverlay.getModel().getText(startOffset, startOffset + addedCharCount); | |||
|
|
|||
| final DocumentChangeEvent event = new DocumentChangeEvent(this, | |||
| final DocumentChangedEvent event = new DocumentChangedEvent(this, | |||
|
|
||
| String text = editorOverlay.getModel().getText(startOffset, startOffset + addedCharCount); | ||
|
|
||
| final DocumentChangingEvent event = new DocumentChangingEvent(this, |
|
ci-build |
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/3100/ |
|
@tsmaeder |
* Add document changing listener support Signed-off-by: Thomas Mäder <tmader@redhat.com> * Use document changin notification to compute correct didChange parameters Signed-off-by: Thomas Mäder <tmader@redhat.com> * Make sure to use the proper document instance. Signed-off-by: Thomas Mäder <tmader@redhat.com> * Renamed DocumentChangeEvent->DocumentChangedEvent Signed-off-by: Thomas Mäder <tmader@redhat.com>
This PR adds support for "documentChanging" events and uses the event to properly compute line numbers in "textDocument/didChange" notifications to LSP language servers. This only affects language servers that request incremental "didChange" messages. The language servers currently in use in Che use full text changes.
This is a fix for #4978
Changelog
Fix didChange notification line numbers for incremental update policy.
This is a bugfix only.