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

Avoid URI serialization issues in override completion by passing full text document identifier #72260

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

dibarbet
Copy link
Member

@dibarbet dibarbet commented Feb 24, 2024

An issue was caught by the vscode-csharp integration tests where override completion was broken with the latest roslyn on Windows. I'm not sure exactly when this regressed, but possibly related to changes made for xaml support.

I wasn't able to track down the exact cause - but on Windows the URI sent from the server in the completion resolve command is getting serialized as just the local path (e.g. not including the file:/// part). The vscode client is expecting a URI string, and when it tries to deserialize, it fails.

This inconsistency in serialization is likely because there's no explicit serializer for the Uri type defined, so it ends up being up to the whims of the serialization library. Instead, we should just pass the TextDocumentIdentifier which already defines an explicit and consistent URI serializer.

Requires client side change here - dotnet/vscode-csharp#6920

@dibarbet dibarbet requested a review from a team as a code owner February 24, 2024 09:38
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 24, 2024
@dibarbet dibarbet merged commit 3059a07 into dotnet:main Feb 25, 2024
27 checks passed
@dibarbet dibarbet deleted the fix_uri_override branch February 25, 2024 01:26
@jjonescz jjonescz added this to the 17.10 P2 milestone Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants