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

vscode: restore removed default values #3844

Merged
merged 1 commit into from Apr 4, 2020

Conversation

Veetaha
Copy link
Contributor

@Veetaha Veetaha commented Apr 4, 2020

After refactoring the config we forgot to set defaults for
some properties like workspaceLoaded, callInfo.full, etc.
This commit restored them to being turned on by defult,
as well added defaults for other props to be more explicit
on their defualt value.
cc @matklad

After refactoring the config we forgot to set defaults for
some properties like workspaceLoaded, callInfo.full, etc.
This commit restored them to being turned on by defult,
as well added defaults for other props to be more explicit
on their defualt value.
@matklad
Copy link
Member

matklad commented Apr 4, 2020

I am actually not sure if we need these defaults -- they are set in the server anyway.

The benefit here is that we document the default for the users of VS Code.

The drawback is that we have to maintain a second set of defaults.

@Veetaha
Copy link
Contributor Author

Veetaha commented Apr 4, 2020

This is actually
a fix, since the defaults are implict on the vscode side. So now workspaceLoaded and callinfo.full do
n't actually work

@Veetaha
Copy link
Contributor Author

Veetaha commented Apr 4, 2020

I was surprised, but running with a debugger it is obvious, that vscode sends default falsy values according to the declared json schema. So e.g. all booleans get false values and all arrays get [] values. Thus, we have to override them explicitly on vscode side anyway...

@matklad
Copy link
Member

matklad commented Apr 4, 2020

That's unfortunate....

bors r+

@lnicola
Copy link
Member

lnicola commented Apr 4, 2020

The benefit here is that we document the default for the users of VS Code.

Don't underestimate that benefit :-)

@bors
Copy link
Contributor

bors bot commented Apr 4, 2020

@bors bors bot merged commit 45b9d6d into rust-lang:master Apr 4, 2020
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.

None yet

3 participants