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

[BUG] Changes to python.analysis.diagnosticSeverityOverrides are completely overwritten at startup #105

Open
Jzooor opened this issue Dec 7, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@Jzooor
Copy link

Jzooor commented Dec 7, 2022

Describe the bug
When the extension starts up, the value for python.analysis.diagnosticSeverityOverrides in settings.json is completely overwritten, losing any user customized settings.

To Reproduce
Steps to reproduce the behavior:

  1. Open settings.json
  2. Add "reportShadowedImports": "none" to "python.analysis.diagnosticSeverityOverrides"
    e.g.
    "python.analysis.diagnosticSeverityOverrides": {
        "reportMissingModuleSource": "none",
        "reportShadowedImports": "none"
    },
  1. Close and re-open workspace.
  2. See that "python.analysis.diagnosticSeverityOverrides" no longer has the added value from step 2.

Expected behavior
All user customizations to "python.analysis.diagnosticSeverityOverrides" in settings.json should be kept (extra options, and changes to extensions desired values).

Desktop (please complete the following information):

  • OS: Win10
  • Version VS Code v1.73.1, CircuitPython extension v0.1.19

Additional context
I want to add (and keep) "reportShadowedImports": "none" because Pylance complains that code.py overrides the stdlib module code.

@Jzooor Jzooor added the bug Something isn't working label Dec 7, 2022
@Jzooor
Copy link
Author

Jzooor commented Dec 7, 2022

I gather the reason is that the activate function in extension.ts is just wholesale setting the value of "python.analysis.diagnosticSeverityOverrides" to be

{
    "reportMissingModuleSource": "none"
}

Which wipes out anything else in the settings.json file.

I don't know javascript or the VS extensions API very well, but I will try to see if I can figure out how to merge what the extension wants to be in the settings with what already exists there.

@wmerkens
Copy link

wmerkens commented Feb 4, 2023

I found if I created a .workplace file the settings.json does not get reverted in that section anymore but then there is a chance when a bundle has updated the date in the path is not updated.

@f0lie
Copy link

f0lie commented Feb 13, 2023

I have a .workplace file too to support multiple different projects but the extension still overwrites my overrides.

From what I can tell, the offending line is this:

vscode.workspace.getConfiguration().update("python.analysis.diagnosticSeverityOverrides",

I am not a ts developer but it seems like it's a simple fix to check if the setting doesn't exist and append it to the config.

I found that you can set

"python.analysis.diagnosticSeverityOverrides" : {
  "reportShadowedImports": "none"
}

at your user settings json and it seems like it doesn't overwrite it. From what I can tell from the code, it is overwriting workspace settings which are not the same as user settings.

This unfortunately has the impact of changing all of your python projects unrelated to circuitpython ones. You might end up forgetting about it later.

@stefan-sherwood
Copy link

Not a solution to the bug reported here per se but you can probably solve your original problem by instead renaming code.py to main.py. Per the documentation, this obviates a need to disable the warnings you're trying to avoid.

jrwagz added a commit to jrwagz/vscode-circuitpython that referenced this issue May 13, 2023
This should ensure that only the config key we care about is updated, and not every other config that the user has set.

As mentioned in joedevivo#105
@jrwagz
Copy link

jrwagz commented May 13, 2023

I have this same issue, and I am not a TypeScript developer, but I am a python developer. I took a crack at the changes required, and would like somebody to review and approve:

#120

I don't know how to properly test the changes, so please review carefully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants