Skip to content
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

Move dynamic registration to initialized #602

Closed
wants to merge 1 commit into from

Conversation

gorkem
Copy link
Contributor

@gorkem gorkem commented Mar 26, 2018

Moves the dynamic registration of the server capabilities
to initialized notification

fixes #601

Signed-off-by: Gorkem Ercan [email protected]

@gorkem gorkem requested review from fbricon and snjeza March 26, 2018 01:15
@fbricon fbricon changed the title Move dynamic registration to initialzed Move dynamic registration to initialized Mar 26, 2018
@@ -164,9 +164,70 @@ public void disconnectClient() {
*/
@Override
public void initialized(InitializedParams params) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logInfo(">> initialized");

toggleCapability(preferenceManager.getPreferences().isSignatureHelpEnabled(), Preferences.SIGNATURE_HELP_ID, Preferences.TEXT_DOCUMENT_SIGNATURE_HELP, SignatureHelpHandler.createOptions());
}
if (preferenceManager.getClientPreferences().isRenameDynamicRegistrationSupported()) {
toggleCapability(preferenceManager.getPreferences().isRenameEnabled(), Preferences.RENAME_ID, Preferences.TEXT_DOCUMENT_RENAME, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (preferenceManager.getPreferences().isRenameEnabled()) {
	registerCapability(Preferences.RENAME_ID, Preferences.TEXT_DOCUMENT_RENAME);
} else {
	unregisterCapability(Preferences.RENAME_ID, Preferences.TEXT_DOCUMENT_RENAME);
}

should have been removed back in 3c87245#diff-836a9345804342fc811e247515539e98R265

It's replaced by the call to toggleCapability

@fbricon
Copy link
Contributor

fbricon commented Mar 26, 2018

Change looks sound. OK to merge once those 2 comments are addressed

Moves the dynamic registration of the server capabilities
to initialized notification

fixes eclipse-jdtls#601

Signed-off-by: Gorkem Ercan <[email protected]>
@fbricon
Copy link
Contributor

fbricon commented Mar 26, 2018

merged as a3fb99c

@fbricon fbricon closed this Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

server is not initialized correctly without configuration call
3 participants