Fix problems with a python kernel not handling json with numbers#12293
Conversation
| // tslint:disable-next-line: no-var-requires no-require-imports | ||
| const NamedRegexp = require('named-js-regexp') as typeof import('named-js-regexp'); | ||
|
|
||
| import { JSONObject } from '@phosphor/coreutils'; |
There was a problem hiding this comment.
Do we really need this type? Why not just use any or {} or similar.
There was a problem hiding this comment.
Yeah I guess not. Any is essentially the same thing
DonJayamanne
left a comment
There was a problem hiding this comment.
I'd try to remove the use of @phosphor/coreutils.
| // Scrub the environment of the specmodel to make sure it has allowed values (they all must be strings) | ||
| // See this issue here: https://github.com/microsoft/vscode-python/issues/11749 | ||
| if (specModel.env) { | ||
| specModel = cleanEnvironment(specModel); |
There was a problem hiding this comment.
Why not just pass the specModel.env instead of passing specModel, at the end of the day that's what needs to be cleane dup.
There was a problem hiding this comment.
Ignore the above comment, I know now why its required.
| // See this issue here: https://github.com/microsoft/vscode-python/issues/11749 | ||
| const keys = Object.keys(copy.env); | ||
| keys.forEach((k) => { | ||
| if (copy.env) { |
There was a problem hiding this comment.
I don' think this check is required.
|
Kudos, SonarCloud Quality Gate passed!
|
For #11749
Essentially if you have this:
A kernel will fail on startup saying non string environment is not allowed. THis might only happen with 3.7 and earlier, but easier to just prevent this.