Skip to content

Adapt stubPath special-casing to latest changes#11532

Open
rchl wants to merge 3 commits into
microsoft:mainfrom
rchl:fix/stubs-path
Open

Adapt stubPath special-casing to latest changes#11532
rchl wants to merge 3 commits into
microsoft:mainfrom
rchl:fix/stubs-path

Conversation

@rchl

@rchl rchl commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

In #11480 I've removed redundant workspace/configuration requests but I've missed this special-case logic in vscode extension that removes stubPath property if it's at default value.

Without this change, the server would receive the default typings value and complain that the path is not correct (in logs only, I believe).

@rchiodo

rchiodo commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

@rchiodo

rchiodo commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

The section rename is correct but incomplete — the guard and delete inside the loop still assume the old 'python.analysis' shape. Both need to be updated to operate on the nested analysis.stubPath so the default typings value is actually stripped (and a user's custom stubPath is preserved). A middleware test would prevent this section→shape coupling from regressing.

@rchiodo

rchiodo commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

The migration to section python is only half-done. Since the server now reads stubPath nested under analysis, both the user-set guard and the delete need to target analysis.stubPath (guarding against a missing analysis). As written the delete no-ops and the default typings is still forwarded, so the reported log complaint remains.

@rchiodo

rchiodo commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

The section switch to python needs matching updates to the stubPath handling below it: both the user-set guard and the delete still assume the old top-level python.analysis shape, so they now target the wrong key/depth (result[i].stubPath and python.stubPath) instead of result[i].analysis.stubPath / python.analysis.stubPath. As written, the default typings value is still forwarded, so the fix is incomplete.

Comment thread packages/vscode-pyright/src/extension.ts Outdated
Comment thread packages/vscode-pyright/src/extension.ts
@rchiodo

rchiodo commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

The migration correctly forwards section python and targets the nested analysis.stubPath, matching the server's pythonSection?.analysis.stubPath read. Two small, non-blocking suggestions inline: guard the delete with optional chaining and add a middleware regression test for the strip-vs-preserve contract.

@rchiodo rchiodo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@rchiodo rchiodo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@rchiodo

rchiodo commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

@rchiodo

rchiodo commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

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.

2 participants