Autocompletion overwrites following characters #425

snjeza · 2017-11-11T17:11:17Z

Fixes redhat-developer/vscode-java#352 (comment)

Signed-off-by: Snjezana Peco [email protected]

fbricon · 2017-11-13T19:45:36Z

@snjeza care to explain the rationale of this change?

snjeza · 2017-11-14T18:06:00Z

@snjeza care to explain the rationale of this change?

The validationTimer job is a LONG job and starts after 100ms even when we call job.schedule(0).
Also, sometimes VS Code can apply a new completion while resolving another completion.
This patch immediately runs the validation in didChange, runs completion and resolveCompletion in IWorkspaceRunnable.

fbricon · 2017-11-15T00:06:29Z

@snjeza so my understanding of @aeschli's initial delayed validation, was that you don't want to perform validation on each keystroke, but basically wait for the user to be idle and then start showing validation errors. Why did we stop doing that? (I know this predates your change)

snjeza · 2017-11-20T18:20:25Z

@snjeza so my understanding of @aeschli's initial delayed validation, was that you don't want to perform validation on each keystroke, but basically wait for the user to be idle and then start showing validation errors. Why did we stop doing that? (I know this predates your change)

The PR doesn't work and breaks the delayed validation. I have updated it.

You will be able to reproduce the issue if you use a larger project and edit a big class. I have used the che-plugin-openshift-client project and the OpenShiftConnector class (the che project is built from the command line).
You should type the following:

|()

type 'g' and 'e' and apply 'getApiEndpoint'
If you do it fast, you will get an invalid replacement ('getApiEndpoint)' or 'getApiEndpoint(()' instead of 'getApiEndpoint()' ).
The issue happens because the Java LS returns the completion command before it is finished.
VS Code sends the didChange command that breaks the completion.
VS Code sends document/didChange ('g')
Java LS updates a compilation unit
VS Code sends document/completion
Java LS starts a completion, doesn't solve it, but returns a response to the client
VS Code sends document/didChange ('e')
Java LS updates the compilation unit
Java LS solves the completion with a wrong offset and we get a wrong replacement

Also, sometimes code actions throw an exception (usually 'End of File').

The PR solves the issue by waiting for the completion to finish. The PR also sets a timeout of 1 second for the completion.

You can also test the issue by setting the following property:

"editor.quickSuggestionsDelay": 500 // or some higher value

In this case, VS Code sends document/completion after 500ms which is enough that Java LS completes several didChange commands so that you won't be able to reproduce the issue.

fbricon · 2017-11-20T18:45:29Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java

-				proposals.addAll(collector.getCompletionItems());
+				IProgressMonitor subMonitor = new NullProgressMonitor() {
+					private final static int TIMEOUT = 1000;
+					private long endTime;


can we call it timeLimit?

fbricon · 2017-11-20T18:46:59Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java

@@ -77,8 +86,27 @@
 		if (offset >-1 && !monitor.isCanceled()) {
 			IBuffer buffer = unit.getBuffer();
 			if (buffer != null && buffer.getLength() >= offset) {
-				unit.codeComplete(offset, collector, monitor);
-				proposals.addAll(collector.getCompletionItems());
+				IProgressMonitor subMonitor = new NullProgressMonitor() {


can't you wrap the outer monitor so that if the user cancels it, the submonitor is cancelled too?

Signed-off-by: Snjezana Peco <[email protected]>

snjeza · 2017-11-20T19:44:28Z

@fbricon I have updated the PR.

fbricon · 2017-11-20T20:39:31Z

This makes code completion super snappy again, nice work @snjeza!

puremourning · 2017-12-27T21:28:45Z

Does the 1s timeout affect resolveCompletion? I am seeing a regression in my test where the javadoc is not returned on a resolve request and git bisect (and manual testing) blames this commit.

My client (for better or, more likely worse) attempts to resolve every completion item as we have no way to resolve based on what the user is looking at (details not interesting).

The test case is File (https://github.com/puremourning/ycmd-1/blob/java-language-server/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java), line 16 requesting completion after the .. It expects the resolved completion item testywesty to return a documentation item of Test in the west.

here's a snippet log (full log in gist here https://gist.github.com/anonymous/2329af7bff6bc5a6cfc51dfbddcd27b5) :

This commit:

2017-12-27 21:07:42,101 - DEBUG - TX: Sending message: b'Content-Length: 290\r\n\r\n{"id": "2", "jsonrpc": "2.0", "method": "textDocument/completion", "params": {"position": {"character": 32, "line": 15}, "textDocument": {"uri": "file:///Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}}}'


  2017-12-27 21:07:42,160 - DEBUG - RX: Received message: b'{"jsonrpc":"2.0","id":"2","result":{"isIncomplete":false,"items":[{"label":"\xc3\xa5test : boolean","kind":5,"detail":"Te  st.T\xc3\xa9stClass","sortText":"999999034","insertText":"\xc3\xa5test","data":{"decl_signature":"Lcom.youcompleteme.Test$T\xc3\xa9stClass;","name":"\xc3\xa5test","pid":"0","rid":  "0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"label":"testywesty : Str  ing","kind":5,"detail":"Test.T\xc3\xa9stClass","sortText":"999999034","insertText":"testywesty","data":{"decl_signature":"Lcom.youcompleteme.Test$T\xc3\xa9stClass;","name":"testyw  esty","pid":"1","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"la  bel":"a_test : int","kind":5,"detail":"Test.T\xc3\xa9stClass","sortText":"999998554","insertText":"a_test","data":{"decl_signature":"Lcom.youcompleteme.Test$T\xc3\xa9stClass;","na  me":"a_test","pid":"2","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"  }},{"label":"wait(long timeout, int nanos) : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"wait","data":{"decl_signature":"Ljava.lang.Object;","signature":  "(JI)V","name":"wait","pid":"3","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/T  est.java"}},{"label":"wait(long timeout) : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"wait","data":{"decl_signature":"Ljava.lang.Object;","signature":"(  J)V","name":"wait","pid":"4","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test  .java"}},{"label":"wait() : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"wait","data":{"decl_signature":"Ljava.lang.Object;","signature":"()V","name":"wai  t","pid":"5","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"label  ":"toString() : String","kind":3,"detail":"Object","sortText":"999999035","insertText":"toString","data":{"decl_signature":"Ljava.lang.Object;","signature":"()Ljava.lang.String;",  "name":"toString","pid":"6","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.  java"}},{"label":"notifyAll() : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"notifyAll","data":{"decl_signature":"Ljava.lang.Object;","signature":"()V","n  ame":"notifyAll","pid":"7","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.j  ava"}},{"label":"notify() : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"notify","data":{"decl_signature":"Ljava.lang.Object;","signature":"()V","name":"n  otify","pid":"8","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"l  abel":"hashCode() : int","kind":3,"detail":"Object","sortText":"999998555","insertText":"hashCode","data":{"decl_signature":"Ljava.lang.Object;","signature":"()I","name":"hashCode  ","pid":"9","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"label"  :"getClass() : Class\\u003c?\\u003e","kind":3,"detail":"Object","sortText":"999999035","insertText":"getClass","data":{"decl_signature":"Ljava.lang.Object;","signature":"()Ljava.l  ang.Class\\u003c*\\u003e;","name":"getClass","pid":"10","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project  /src/com/youcompleteme/Test.java"}},{"label":"equals(Object obj) : boolean","kind":3,"detail":"Object","sortText":"999999035","insertText":"equals","data":{"decl_signature":"Ljava  .lang.Object;","signature":"(Ljava.lang.Object;)Z","name":"equals","pid":"11","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/  simple_eclipse_project/src/com/youcompleteme/Test.java"}}]}}'

...

  2017-12-27 21:07:42,199 - DEBUG - TX: Sending message: b'Content-Length: 472\r\n\r\n{"id": "4", "jsonrpc": "2.0", "method": "completionItem/resolve", "params": {"data": {"decl_sig  nature": "Lcom.youcompleteme.Test$T\\u00e9stClass;", "name": "testywesty", "pid": "1", "rid": "0", "uri": "file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/ja  va/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}, "detail": "Test.T\\u00e9stClass", "insertText": "testywesty", "kind": 5, "label": "testywesty : String", "sor  tText": "999999034"}}'

...

  2017-12-27 21:07:42,203 - DEBUG - RX: Received message: b'{"jsonrpc":"2.0","id":"4","result":{"label":"testywesty : String","kind":5,"detail":"Test.T\xc3\xa9stClass","sortText":"9  99999034","insertText":"testywesty","insertTextFormat":1,"textEdit":{"range":{"start":{"line":15,"character":32},"end":{"line":15,"character":32}},"newText":"testywesty"}}}'

Commit prior to this one:


  2017-12-27 21:24:15,740 - DEBUG - TX: Sending message: b'Content-Length: 290\r\n\r\n{"id": "2", "jsonrpc": "2.0", "method": "textDocument/completion", "params": {"position": {"cha  racter": 32, "line": 15}, "textDocument": {"uri": "file:///Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcomplete  me/Test.java"}}}'

..

  2017-12-27 21:24:15,949 - DEBUG - RX: Received message: b'{"jsonrpc":"2.0","id":"2","result":{"isIncomplete":false,"items":[{"label":"\xc3\xa5test : boolean","kind":5,"detail":"Te  st.T\xc3\xa9stClass","sortText":"999999034","insertText":"\xc3\xa5test","data":{"decl_signature":"Lcom.youcompleteme.Test$T\xc3\xa9stClass;","name":"\xc3\xa5test","pid":"0","rid":  "0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"label":"testywesty : Str  ing","kind":5,"detail":"Test.T\xc3\xa9stClass","sortText":"999999034","insertText":"testywesty","data":{"decl_signature":"Lcom.youcompleteme.Test$T\xc3\xa9stClass;","name":"testyw  esty","pid":"1","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"la  bel":"a_test : int","kind":5,"detail":"Test.T\xc3\xa9stClass","sortText":"999998554","insertText":"a_test","data":{"decl_signature":"Lcom.youcompleteme.Test$T\xc3\xa9stClass;","na  me":"a_test","pid":"2","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"  }},{"label":"wait(long timeout, int nanos) : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"wait","data":{"decl_signature":"Ljava.lang.Object;","signature":  "(JI)V","name":"wait","pid":"3","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/T  est.java"}},{"label":"wait(long timeout) : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"wait","data":{"decl_signature":"Ljava.lang.Object;","signature":"(  J)V","name":"wait","pid":"4","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test  .java"}},{"label":"wait() : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"wait","data":{"decl_signature":"Ljava.lang.Object;","signature":"()V","name":"wai  t","pid":"5","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"label  ":"toString() : String","kind":3,"detail":"Object","sortText":"999999035","insertText":"toString","data":{"decl_signature":"Ljava.lang.Object;","signature":"()Ljava.lang.String;",  "name":"toString","pid":"6","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.  java"}},{"label":"notifyAll() : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"notifyAll","data":{"decl_signature":"Ljava.lang.Object;","signature":"()V","n  ame":"notifyAll","pid":"7","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.j  ava"}},{"label":"notify() : void","kind":3,"detail":"Object","sortText":"999999115","insertText":"notify","data":{"decl_signature":"Ljava.lang.Object;","signature":"()V","name":"n  otify","pid":"8","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"l  abel":"hashCode() : int","kind":3,"detail":"Object","sortText":"999998555","insertText":"hashCode","data":{"decl_signature":"Ljava.lang.Object;","signature":"()I","name":"hashCode  ","pid":"9","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}},{"label"  :"getClass() : Class\\u003c?\\u003e","kind":3,"detail":"Object","sortText":"999999035","insertText":"getClass","data":{"decl_signature":"Ljava.lang.Object;","signature":"()Ljava.l  ang.Class\\u003c*\\u003e;","name":"getClass","pid":"10","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/simple_eclipse_project  /src/com/youcompleteme/Test.java"}},{"label":"equals(Object obj) : boolean","kind":3,"detail":"Object","sortText":"999999035","insertText":"equals","data":{"decl_signature":"Ljava  .lang.Object;","signature":"(Ljava.lang.Object;)Z","name":"equals","pid":"11","rid":"0","uri":"file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/java/testdata/  simple_eclipse_project/src/com/youcompleteme/Test.java"}}]}}'

...

  2017-12-27 21:24:16,004 - DEBUG - TX: Sending message: b'Content-Length: 472\r\n\r\n{"id": "4", "jsonrpc": "2.0", "method": "completionItem/resolve", "params": {"data": {"decl_sig  nature": "Lcom.youcompleteme.Test$T\\u00e9stClass;", "name": "testywesty", "pid": "1", "rid": "0", "uri": "file:/Users/ben/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/tests/ja  va/testdata/simple_eclipse_project/src/com/youcompleteme/Test.java"}, "detail": "Test.T\\u00e9stClass", "insertText": "testywesty", "kind": 5, "label": "testywesty : String", "sor  tText": "999999034"}}'

...

  2017-12-27 21:24:16,055 - DEBUG - RX: Received message: b'{"jsonrpc":"2.0","id":"4","result":{"label":"testywesty : String","kind":5,"detail":"Test.T\xc3\xa9stClass","documentatio  n":"Test in the west ","sortText":"999999034","insertText":"testywesty","insertTextFormat":1,"textEdit":{"range":{"start":{"line":15,"character":32},"end":{"line":15,"character":3  2}},"newText":"testywesty"}}}'

As you can see, in the previous commit, documentation is correct, but not supplied in this commit.

snjeza requested review from gorkem and fbricon November 11, 2017 17:11

snjeza force-pushed the issue-352a branch from d2e6e8e to 9f68677 Compare November 14, 2017 18:05

snjeza force-pushed the issue-352a branch from 9f68677 to 373096b Compare November 20, 2017 18:17

fbricon reviewed Nov 20, 2017

View reviewed changes

Autocompletion overwrites following characters

c3977e9

Signed-off-by: Snjezana Peco <[email protected]>

snjeza force-pushed the issue-352a branch from 373096b to c3977e9 Compare November 20, 2017 19:40

fbricon merged commit ace8c79 into eclipse-jdtls:master Nov 20, 2017

puremourning mentioned this pull request Dec 27, 2017

Documentation not included in completion resolve since PR 425 #509

Closed

fbricon mentioned this pull request Jan 9, 2018

Documentation not included in completion resolve since PR 425 #512

Merged

snjeza mentioned this pull request Oct 5, 2019

Is the delay on diagnostics results intentional? #1198

Open

snjeza mentioned this pull request Dec 8, 2021

Diagnostics from changes to build configuration not reflected in open source files #1964

Merged

testforstephen mentioned this pull request Apr 19, 2023

No need to run completion handler in async thread pool #2605

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autocompletion overwrites following characters #425

Autocompletion overwrites following characters #425

snjeza commented Nov 11, 2017

fbricon commented Nov 13, 2017

snjeza commented Nov 14, 2017 •

edited

Loading

fbricon commented Nov 15, 2017

snjeza commented Nov 20, 2017

fbricon Nov 20, 2017

fbricon Nov 20, 2017

snjeza commented Nov 20, 2017

fbricon commented Nov 20, 2017

puremourning commented Dec 27, 2017

Autocompletion overwrites following characters #425

Autocompletion overwrites following characters #425

Conversation

snjeza commented Nov 11, 2017

fbricon commented Nov 13, 2017

snjeza commented Nov 14, 2017 • edited Loading

fbricon commented Nov 15, 2017

snjeza commented Nov 20, 2017

fbricon Nov 20, 2017

Choose a reason for hiding this comment

fbricon Nov 20, 2017

Choose a reason for hiding this comment

snjeza commented Nov 20, 2017

fbricon commented Nov 20, 2017

puremourning commented Dec 27, 2017

snjeza commented Nov 14, 2017 •

edited

Loading