Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Fix settings retrieval #2113

Closed
MikhailArkhipov wants to merge 1 commit into from
Closed

Fix settings retrieval #2113

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/LanguageServer/Impl/LanguageServer.Configuration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,14 @@ public async Task DidChangeConfiguration(JToken token, CancellationToken cancell
using (await _prioritizer.ConfigurationPriorityAsync(cancellationToken)) {
Debug.Assert(_initialized);

var settings = new LanguageServerSettings();

// https://github.com/microsoft/python-language-server/issues/915
// If token or settings are missing, assume defaults.
var rootSection = token?["settings"];
var pythonSection = rootSection?["python"];
var pythonSection = await GetPythonConfigurationAsync(cancellationToken);
if (pythonSection == null) {
Copy link
Member

@jakebailey jakebailey Aug 18, 2020

Choose a reason for hiding this comment

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

Why didn't this check work before? I'd think that if it was sending a null set of settings, we'd be able to see that and skip entirely.

Copy link
Author

@MikhailArkhipov MikhailArkhipov Aug 18, 2020

Choose a reason for hiding this comment

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

didChangeConfiguration is supposed to send object that contains changed settings. Root is settings. It started coming empty like

{
   settings {
   }
}

Copy link
Author

@MikhailArkhipov MikhailArkhipov Aug 18, 2020

Choose a reason for hiding this comment

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

Let me check in my R extension... I tried to comment out what you suggested in core ext but it didn't fix it. This change is more of a quick fix, I don't know actual reason things changed.

Copy link
Member

@jakebailey jakebailey Aug 18, 2020

Choose a reason for hiding this comment

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

Empty would certainly mean empty (since we're getting the whole object), but it's definitely strange that it'd be sending empty unless my hack to the core extension to send null is being converted somewhere in the chain to {}...

Now that I look, that hack only appears in the node code, so commenting it out wouldn't really do anything, yeah...

return;
}

var settings = new LanguageServerSettings();
var autoComplete = pythonSection["autoComplete"];
settings.completion.showAdvancedMembers = GetSetting(autoComplete, "showAdvancedMembers", true);
settings.completion.addBrackets = GetSetting(autoComplete, "addBrackets", false);
Expand Down