Check for dynamic registration in textDocument/typeDefinition and workspace/executeCommand#1748
Check for dynamic registration in textDocument/typeDefinition and workspace/executeCommand#1748mickaelistria merged 3 commits into
Conversation
…kspace/executeCommand (#1142)
| if (this.getClientCapabilities().isExecuteCommandRegistered()) { | ||
| registerCapability(WORKSPACE_EXECUTE_COMMAND_ID, WORKSPACE_EXECUTE_COMMAND, | ||
| new ExecuteCommandOptions(commands)); | ||
| } |
There was a problem hiding this comment.
Can you please just restore those lines to their initial formatting to avoid non-payload change in the same commit as the fix?
There was a problem hiding this comment.
I'm not sure what you mean by this, do you mean the indentation?
There was a problem hiding this comment.
Yes, we try to keep the indentation the same as much as possible so that the commits are focused on the actual code change (indentiation change make it often harder to read history, we try to process indentation changed when necessary in dedicated commits)
There was a problem hiding this comment.
OK, my bad, I see that the code actually changes!
So I'm going to merge it, I don't think indentation is so critical to delay merging further.
There was a problem hiding this comment.
Yup, added a check :)
There was a problem hiding this comment.
If you see on github you will see that there are some trouble with formatting
There was a problem hiding this comment.
Think that's the indentation fixed now
|
This project requires that you sign the Eclipse Contributor License Agreement to certify you're not intentionally introducing malicious or stolen code. Can you please sign it at https://accounts.eclipse.org/user/eca ? |
Signed |
|
LGTM, thanks @daniellaing ! @fbricon @mickaelistria I let you to merge the PR. |
|
Thanks a lot @daniellaing ! |
4 participants
For #1142
This might not be all the capabilities which are erroneously registered, only the ones I've seen an error for (in my limited use).
Seems to also fix a maven test, 6 fails down to 5. Not sure which one now passes though...