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

fix(windows): clear diagnostics errors #584

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mxschmitt
Copy link
Member

@mxschmitt mxschmitt commented Dec 31, 2024

Whats happening?

The issue is that we write errors to a Map with "Node.js like paths" so that the drive-letter is uppercase but we delete them out of the map by using the VSCode workspace change event, which are VSCode like paths, which have always lowercase drive letters.

This PR wraps when calling set inside this._vscode.Uri.file(path).fsPath which normalises and is what we already do for other places when we interact with locations from the reporter API.

for (const requestedFile of requestedFiles)
this._errorByFile.deleteAll(requestedFile);

Resolves microsoft/playwright#34146
Resolves microsoft/playwright#33671

Copy link
Contributor

@dgozman dgozman left a comment

Choose a reason for hiding this comment

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

Please add a test. Overall, I don't like how we sprinkle this pattern in some places, but not consistently. We need to figure out a nice boundary where we convert between Node.js paths and VSCode paths and stick to that, hopefully enforced by TypeScript. Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants