Add support for startDebugging#704
Conversation
|
Can one of the admins verify this patch? |
|
@eclipse-lsp4j-bot run tests |
jonahgraham
left a comment
There was a problem hiding this comment.
The build is failing with this error:
09:06:31 > Task :org.eclipse.lsp4j.debug:generateXtext FAILED
09:06:31 ERROR:ProcessEventArgumentsStartMethod cannot be resolved to a type. (file:/home/jenkins/agent/workspace/lsp4j-github-pullrequests/org.eclipse.lsp4j.debug/src/main/java/org/eclipse/lsp4j/debug/DebugProtocol.xtend line : 486 column : 2)
09:06:31 ERROR:StartDebuggingRequestType cannot be resolved to a type. (file:/home/jenkins/agent/workspace/lsp4j-github-pullrequests/org.eclipse.lsp4j.debug/src/main/java/org/eclipse/lsp4j/debug/DebugProtocol.xtend line : 4299 column : 2)
09:06:31 ERROR:The type ProcessEventArgumentsStartMethod is already defined. (file:/home/jenkins/agent/workspace/lsp4j-github-pullrequests/org.eclipse.lsp4j.debug/src/main/java/org/eclipse/lsp4j/debug/DebugProtocol.xtend line : 499 column : 6)
09:06:31 ERROR:The type ProgressUpdateEventArguments is already defined. (file:/home/jenkins/agent/workspace/lsp4j-github-pullrequests/org.eclipse.lsp4j.debug/src/main/java/org/eclipse/lsp4j/debug/DebugProtocol.xtend line : 607 column : 7)
09:06:31 ERROR:The type ProgressUpdateEventArguments is already defined. (file:/home/jenkins/agent/workspace/lsp4j-github-pullrequests/org.eclipse.lsp4j.debug/src/main/java/org/eclipse/lsp4j/debug/DebugProtocol.xtend line : 4251 column : 7)
09:06:31 ERROR:The type ProcessEventArgumentsStartMethod is already defined. (file:/home/jenkins/agent/workspace/lsp4j-github-pullrequests/org.eclipse.lsp4j.debug/src/main/java/org/eclipse/lsp4j/debug/DebugProtocol.xtend line : 4274 column : 6)
09:06:31 4a251b7 to
0b865c6
Compare
|
@eclipse-lsp4j-bot run tests |
0b865c6 to
a2ae94f
Compare
|
@eclipse-lsp4j-bot run tests |
|
build currently does not run cause auf nexus.eclipse outage |
|
@eclipse-lsp4j-bot run tests |
a2ae94f to
8d6bec9
Compare
|
I had hoped to get to this by now, but alas too many other things to review. Still hoping that @KamasamaK may swoop in and review, but I should get to it soon if he doesn't. |
|
@jonahgraham no big issue; I don't have strong needs for it on short term; of course -like all reviews- the sooner the better, but as I'm also busy with other things, I don't plan to get to LSP4E side of things shortly, so this can wait without harming. |
8d6bec9 to
2f525cf
Compare
jonahgraham
left a comment
There was a problem hiding this comment.
I have made a bunch of changes, each one should be explained with a review comment.
| } | ||
|
|
||
| /** | ||
| * Arguments for <code>startDebugging</code> reverse-request |
There was a problem hiding this comment.
Match existing style, plus "reverse" is not relevant here.
| * or `attach` request. | ||
| */ | ||
| @NonNull | ||
| StartDebuggingRequestType request; |
There was a problem hiding this comment.
For anonymous enums we have used the full type name + field name to name it. e.g OutputEventArgumentsGroup
| } | ||
|
|
||
| @JsonRpcData | ||
| class StartDebuggingResponse { |
There was a problem hiding this comment.
Remove this - we use Void type when there is just an ack response. e.g. there is no ConfigurationDoneResponse
| * Since 1.59 | ||
| */ | ||
| @JsonRequest | ||
| default CompletableFuture<StartDebuggingResponse> startDebugging(StartDebuggingRequestArguments args) { |
| * of the same type as the caller. | ||
| * <p> | ||
| * This request should only be sent if the corresponding client capability | ||
| * <code>supportsStartDebuggingRequest</code> is <code>true</code>. |
There was a problem hiding this comment.
Use links to be consistent with existing code.
| * <p> | ||
| * Since 1.59 | ||
| */ | ||
| Boolean supportsStartDebuggingRequest; |
There was a problem hiding this comment.
This is in the wrong place, should be in InitializeRequestArguments as it is a capability of the client, not of the server.
| /** | ||
| * Client supports the `startDebugging` request. | ||
| * <p> | ||
| * This is an optional property. |
There was a problem hiding this comment.
Mixed indentation on these lines
|
|
||
| @JsonRpcData | ||
| class StartDebuggingResponse { | ||
| } No newline at end of file |
There was a problem hiding this comment.
Missing newline at end of the file
|
@jonahgraham sure, thanks a lot! |
|
The rest of the updates to bring the DAP part of LSP4J up to date are in #716 |
No description provided.