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 intomasterfrom
brackets
Closed

Fix settings retrieval#2113
MikhailArkhipov wants to merge 1 commit intomasterfrom
brackets

Conversation

@MikhailArkhipov
Copy link
Copy Markdown

Something changed (in the core extension?) so all didChangeConfiguration come with empty settings object. Switching to explicit settings retrieval.

var rootSection = token?["settings"];
var pythonSection = rootSection?["python"];
var pythonSection = await GetPythonConfigurationAsync(cancellationToken);
if (pythonSection == null) {
Copy link
Copy Markdown
Member

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
Copy Markdown
Author

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
Copy Markdown
Author

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
Copy Markdown
Member

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...

@MikhailArkhipov
Copy link
Copy Markdown
Author

In favor of microsoft/vscode-python#13511

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants