Fix for interpreter selection #14693
Conversation
Codecov Report
@@ Coverage Diff @@
## main #14693 +/- ##
==========================================
- Coverage 64.89% 64.87% -0.02%
==========================================
Files 547 547
Lines 25611 25615 +4
Branches 3619 3619
==========================================
Hits 16619 16619
- Misses 8311 8315 +4
Partials 681 681
Continue to review full report at Codecov.
|
|
Build failure addressed by this PR: #14692 |
src/client/common/configSettings.ts
Outdated
| const keys = Object.entries(settings); | ||
| keys.forEach((e) => { | ||
| const [k, v] = e; | ||
| if (!k.includes('Manager') && !k.includes('Service') && !k.includes('onDid') && !k.includes('to')) { |
There was a problem hiding this comment.
I still don't understand this logic but okay.
There was a problem hiding this comment.
It looks like settings are passed to a message handler, and they need to remove anything on the settings that are not (eventually) either string or a number. The issue itself was that the settings were removed on an instance that we had a reference to else where. The fix I have done now, is that we create a clone of the settings with only the serializeable settings.
There was a problem hiding this comment.
It looks like settings are passed to a message handler, and they need to remove anything on the settings that are not (eventually) either string or a number.
Ya, this is still not the way to remove it. There is also this.workspace. But that's not related to the fix so okay.
There was a problem hiding this comment.
Ideally this should be a deep clone where non-serializable types are removed, may be cloneDeepWith and a customizer, that checks type of value, and if it is not in the serializable group, ignores cloning.
There was a problem hiding this comment.
I did not want to deviate from the original code to prevent any unforeseen side effects.
|
Kudos, SonarCloud Quality Gate passed!
|
* Fix for interpreter selection * Fix linting errors * Minor tweak to property removal
* change log updates * Update gifs * Fix for interpreter selection (#14693) * Fix for interpreter selection * Fix linting errors * Minor tweak to property removal
* Cherry pick from main (#14644) * Update shipped wheels version (#14615) * Update shipped wheels version * News item * Remove redundant files (#14620) * Add extension dependencies at build time (#14636) * Use Node 12.15 in Insiders and Release GitHub Actions (#14641) * Use Node 12.15 on all Insiders and Release GitHub Actions jobs (#14642) Co-authored-by: Joyce Er <[email protected]> * Cherry pic fixes into release for tests. (#14673) * Added FSWatching base class and made related changes (#14605) * Add FSWatching locator base class * Correct glob pattern to not match python3.2whoa * Add documentation of python binary watcher * Fix lint errors * Update ignore list * Add disposable registry * Modify FSWatching Locator * Code reviews * Use string[] * Remove list disposable getter * Fix failing global virtual env watcher tests (#14633) Co-authored-by: Kartik Raj <[email protected]> * Version, change log and cherrypicks for nov release (#14696) * change log updates * Update gifs * Fix for interpreter selection (#14693) * Fix for interpreter selection * Fix linting errors * Minor tweak to property removal * Cherry pick "Bind function to correct this for workspace syms" (#14743) * Fix #14674: Enable overriding "pythonPath" in the launcher Fix #12462: Update launch.json schema to add "python" and remove "pythonPath" Split the "pythonPath" debug property into "python", "debugAdapterPython", and "debugLauncherPython". Do most debug config validation on fully expanded property values via resolveDebugConfigurationWithSubstitutedVariables(). Add fixups for legacy launch.json with "pythonPath". * Point release change log and version update (#14750) * Point release change log and version update * Fix process picker (#14700) * Workaround VSCode bug for process picker * Fix how we pass in icons to VSCode * update change log with cherry pick Co-authored-by: Kartik Raj <[email protected]> Co-authored-by: Joyce Er <[email protected]> Co-authored-by: Kartik Raj <[email protected]> Co-authored-by: Jake Bailey <[email protected]> Co-authored-by: Pavel Minaev <[email protected]>
For #14645
Removing properties from python settings instance caused python path experiment to fail. We should never be removing properties from an instance that could be referenced somewhere else. So shallow-cloning the settings with non-serializable properties removed for the startpage.