Skip to content

[LSP] Temporary workaround for Razor documents returning null tokens#54548

Merged
allisonchou merged 1 commit intodotnet:mainfrom
allisonchou:LSPSemanticTokensWorkaround
Jul 2, 2021
Merged

[LSP] Temporary workaround for Razor documents returning null tokens#54548
allisonchou merged 1 commit intodotnet:mainfrom
allisonchou:LSPSemanticTokensWorkaround

Conversation

@allisonchou
Copy link
Contributor

Temporary workaround for #54547. Mainly just to clean up the noise in logs until a cleaner, long-term solution is added.

@allisonchou allisonchou added the LSP issues related to the roslyn language server protocol implementation label Jul 1, 2021
@allisonchou allisonchou requested a review from a team as a code owner July 1, 2021 22:41
@ghost ghost added the Area-IDE label Jul 1, 2021
RequestContext context,
CancellationToken cancellationToken)
{
// Temporary workaround for https://github.com/dotnet/roslyn/issues/54547:

Choose a reason for hiding this comment

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

@allisonchou allisonchou enabled auto-merge July 1, 2021 22:51
@allisonchou allisonchou merged commit 83571b0 into dotnet:main Jul 2, 2021
@ghost ghost added this to the Next milestone Jul 2, 2021
{
// Temporary workaround for https://github.com/dotnet/roslyn/issues/54547:
// We should eventually go back to throwing here if context.Document is null.
if (context.Document is null)
Copy link
Contributor

Choose a reason for hiding this comment

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

oh... boy do i hate this. IMO this is not ok and we need to revert this PR. This is precisely teh sort of "limp along when we dont' know what is happening" approach that leads to extreme amounts of tech debt and an inability to reason about what's going on.

@CyrusNajmabadi
Copy link
Contributor

Mainly just to clean up the noise in logs

This is not a good reason. The logs are telling us that something has gone critically wrong. It is not ok to solve that by just having our code ignore hte issue. If anything, we should be prioritizing determining the root cause asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE LSP issues related to the roslyn language server protocol implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants