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

Reduce noisy errors when viewing git diff on VS code #9407

Merged
merged 3 commits into from
Oct 13, 2023

Conversation

maryamariyan
Copy link
Member

@maryamariyan maryamariyan commented Oct 12, 2023

Summary of the changes

  • Fixes noisy errors generated when viewing git diff of cshtml/razor files on VS code.
    This is a hacky workaround. Would be good to follow this up with a proper fix.

  • The language server project was returning a wrong file path for URIs with git scheme

  • e.g. this could happen when opening git diff on VS code

  • With this hacky fix, we make up a fake git folder to provide for these file paths

  • This makes sure the returned file path for the left hand side of the git diff view, would not collide with existing file paths (original file path for the right hand side)

Fixes #9365

- The language server project was return a wrong file path for uris with git scheme
- e.g. this could happen when opening git diff on VS code

- With this hacky fix, we make up a fake git folder to provide for these file paths
- This makes sure the returned file path for the left hand side of the git diff view,
 would not collide with existing file paths (original file path for the right hand side)

Fixes #9365
@maryamariyan maryamariyan requested a review from a team as a code owner October 12, 2023 15:51
@maryamariyan maryamariyan self-assigned this Oct 13, 2023
@maryamariyan
Copy link
Member Author

maryamariyan commented Oct 13, 2023

The following integration test failures are showing up in the CI build report: (see here, hitting rerun to check again)

- Rename_ComponentAttribute_BoundAttribute
- Rename_ComponentAttribute_FromCSharpInCSharp
- GoToDefinition_ComponentAttribute_CascadingGenericComponentWithConstraints
- GoToDefinition_ComponentAttribute_GenericComponent
- FindAllReferences_ComponentAttribute_FromCSharpInRazor

But because they are annotated with [IdeFact(Skip = "")], I expected them to be skipped so I'm confused as to why they were even showing up in the report.

@davidwengier
Copy link
Contributor

Looks like it's the conditional legs that are failing, so ignoring the skip attribute is expected. Hopefully you are able to ignore them and merge anyway.

@ryzngard looks like PRs created from branches in this repo get the extra legs, but PRs from forks don't. Is that intentional?

@ryzngard
Copy link
Contributor

Looks like it's the conditional legs that are failing, so ignoring the skip attribute is expected. Hopefully you are able to ignore them and merge anyway.

@ryzngard looks like PRs created from branches in this repo get the extra legs, but PRs from forks don't. Is that intentional?

That is not. I'll take a look, but the trigger is set to none so I'm surprised.... I assumed some level of sanity in CI

@ryzngard
Copy link
Contributor

@maryamariyan feel free to merge with the conditional failures, they don't seem to be blocking. If they are for you let me know. I'm following up with infra to resolve this issue.

@maryamariyan
Copy link
Member Author

Thanks @ryzngard merging now

@maryamariyan maryamariyan merged commit 70de468 into main Oct 13, 2023
@maryamariyan maryamariyan deleted the dev/maryamariyan/bugfix-git branch October 13, 2023 20:27
@ghost ghost added this to the Next milestone Oct 13, 2023
333fred added a commit to 333fred/razor that referenced this pull request Oct 19, 2023
* upstream/main: (45 commits)
  Update dependencies from https://github.com/dotnet/arcade build 20231018.2
  Don't report that we're synchronized if the line count is wrong
  Update dependencies from https://github.com/dotnet/arcade build 20231017.7 (dotnet#9435)
  Guess what! I've got a fever, and the only prescription is more usings!
  Replace explicit richCodeNavigationEnvironment in azure-pipelines-richnav.yml
  Change constraint to require that T inherit from TagHelperCollector<T>
  Update dependencies from https://github.com/dotnet/arcade build 20231016.5
  Found a repro, made a test!
  Don't map to ranges that are invalid
  Don't run conditional tests on PRs (dotnet#9416)
  Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20231010.3
  Update dependencies from https://github.com/dotnet/arcade build 20231010.4
  Don't bother looking up ever project if we know where it comes from
  Make sure we carry ProjectContext through to code action resolve
  Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2291545
  Return an incomplete list if the delegated server doesn't respond
  Update dependencies from https://github.com/dotnet/arcade build 20231010.4
  Update dependencies from https://github.com/dotnet/arcade build 20231010.4
  Apply code review feedback for tag helper caching
  Reduce noisy errors when viewing git diff on VS code (dotnet#9407)
  ...
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
@burkeholland
Copy link

I'm seeing the "semanticTokens/range" error here. In my case, I opened a git diff view of the file, and that's where things went sidways. But even after opening the file directly - no diff view - the error persists.

Extension version: v1.4.29

[Error - 8:58:34 AM] [LanguageServerHost] System.ArgumentOutOfRangeException: The requested line number 407 must be less than the number of lines 2. (Parameter 'Line')
   at Microsoft.CodeAnalysis.Text.TextLineCollection.GetPosition(LinePosition position) in /_/src/Compilers/Core/Portable/Text/TextLineCollection.cs:line 67
   at Microsoft.CodeAnalysis.LanguageServer.Handler.SemanticTokens.SemanticTokensHelpers.ComputeSemanticTokensDataAsync(Document document, ImmutableArray`1 spans, Boolean supportsVisualStudioExtensions, ClassificationOptions options, CancellationToken cancellationToken) in /_/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs:line 106
   at Microsoft.CodeAnalysis.LanguageServer.Handler.SemanticTokens.SemanticTokensHelpers.HandleRequestHelperAsync(Document document, ImmutableArray`1 spans, Boolean supportsVisualStudioExtensions, ClassificationOptions options, CancellationToken cancellationToken) in /_/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs:line 69
   at Microsoft.CodeAnalysis.LanguageServer.Handler.SemanticTokens.SemanticTokensHelpers.HandleRequestHelperAsync(IGlobalOptionService globalOptions, SemanticTokensRefreshQueue semanticTokensRefreshQueue, Range[] ranges, RequestContext context, CancellationToken cancellationToken) in /_/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs:line 48
   at Microsoft.CodeAnalysis.LanguageServer.Handler.SemanticTokens.SemanticTokensRangeHandler.HandleRequestAsync(SemanticTokensRangeParams request, RequestContext context, CancellationToken cancellationToken) in /_/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensRangeHandler.cs:line 45
   at Microsoft.CommonLanguageServerProtocol.Framework.QueueItem`3.StartRequestAsync(TRequestContext context, IMethodHandler handler, CancellationToken cancellationToken)
[Error - 8:58:34 AM] Request textDocument/semanticTokens/range failed.
  Message: The requested line number 407 must be less than the number of lines 2. (Parameter 'Line')
  Code: -32000 

@ryzngard
Copy link
Contributor

ryzngard commented Apr 8, 2024

Thanks @burkeholland. Are you seeing these issues only after you have opened a git window? Or are they showing up in other cases?

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.

Razor language server crashes when viewing a git diff
6 participants