-
Notifications
You must be signed in to change notification settings - Fork 449
Support linked correction proposals through LSP snippet syntax #3229
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
Support linked correction proposals through LSP snippet syntax #3229
Conversation
0c8e0a6 to
93cbd4b
Compare
...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionResolveHandler.java
Outdated
Show resolved
Hide resolved
rgrunber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor things to fix.
...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionResolveHandler.java
Outdated
Show resolved
Hide resolved
...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionResolveHandler.java
Outdated
Show resolved
Hide resolved
...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionResolveHandler.java
Outdated
Show resolved
Hide resolved
...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionResolveHandler.java
Outdated
Show resolved
Hide resolved
...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionResolveHandler.java
Outdated
Show resolved
Hide resolved
2a1a4bb to
5c817e7
Compare
rgrunber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ready for merging after we release. We've tested quite a bit and there's some subsequent changes that would depend on this (not to mention maybe upstreaming the snippet edit class into lsp4j, applying your RHS type-mismatch, and expanding the set of things that can correctly use this.
|
retest this please. |
| } | ||
|
|
||
| public boolean isWorkspaceSnippetEditSupported() { | ||
| return Boolean.parseBoolean(extendedClientCapabilities.getOrDefault("snippetEditSupport", "false").toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to use the standard workspaceEdit.snippetEditSupport capability here to make this work out of the box for clients supporting the 3.18 spec addition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but it needs to be added to https://github.com/eclipse-lsp4j/lsp4j/blob/main/org.eclipse.lsp4j/src/main/java/org/eclipse/lsp4j/Protocol.xtend#L59 . @hopehadfield started this at eclipse-lsp4j/lsp4j#847 .
Fixes redhat-developer/vscode-java#3686
redhat-developer/vscode-java#3730