-
Notifications
You must be signed in to change notification settings - Fork 336
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
[Property Editor] Check for registered LSP service methods before calling them #8788
Conversation
@@ -59,6 +61,18 @@ class EditorClient extends DisposableController | |||
_supportsOpenDevToolsPage = isRegistered; | |||
_supportsOpenDevToolsForceExternal = | |||
capabilities?[Field.supportsForceExternal] == true; | |||
} else if (method == LspMethod.editArgument.methodName) { |
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.
nit: a switch statement may be easier to read than this long chain of if / elses
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.
It would be but unfortunately these are all getters, triggering the error: "Extension methods can't be used in constant expressions." (I'm guessing that is why this was written as a bunch of if statements in the first place)
It's not a big deal, but I think it may have been better to hold off on this - the reason for the prefix was in case we needed to make any breaking changes, we could be sure there wasn't a version of PE/Server out there that try to communicate with different versions of the protocol (unless they were experimental versions). (I think the API is fairly stable and changes will probably be compatible, although I'm not certain how we'll handle removing arguments yet) |
Ah got it! I think we can remove the non-experimental endpoints for now, and then add them back in when ready. I'll send out a change. |
Fixes #8715
Work towards #1948
editArgument
andeditableArguments
service methods to be registered on the LSP service before loading the property editor.