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

Only get first document's highlights #2424

Merged
merged 1 commit into from
Jul 27, 2022
Merged

Conversation

333fred
Copy link
Contributor

@333fred 333fred commented Jul 27, 2022

For files that are included in multiple projects (such as via Compile Include directives), we currently attempt to retrieve semantic highlights for every document. However, this means that highlight spans will have overlapping results from every project, and the service will crash. Further, we have no way of actually selecting what project to display the file as, so retrieving any but the first document doesn't serve the user. Therefore, we remove the loop, and just look through the first document found.

Fixes #2421.
Fixes #2050.

For files that are included in multiple projects (such as via `Compile Include` directives), we currently attempt to retrieve semantic highlights for every document. However, this means that highlight spans will have overlapping results from every project, and the service will crash. Further, we have no way of actually selecting what project to display the file as, so retrieving any but the first document doesn't serve the user. Therefore, we remove the loop, and just look through the first document found.
@333fred 333fred requested a review from JoeRobich July 27, 2022 03:40
@333fred
Copy link
Contributor Author

333fred commented Jul 27, 2022

Note: I would highly recommend reviewing without whitespace changes, the diff is significantly smaller.

@filipw filipw enabled auto-merge July 27, 2022 08:26
@filipw
Copy link
Member

filipw commented Jul 27, 2022

thank you!

@333fred
Copy link
Contributor Author

333fred commented Jul 27, 2022

I'm going to force merge this. @filipw @JoeRobich, we need to update our mac leg, as 10.15 is deprecated, and they are intentionally starting to fail jobs. actions/runner-images#5583

@333fred 333fred disabled auto-merge July 27, 2022 15:58
@333fred 333fred merged commit d249964 into OmniSharp:master Jul 27, 2022
Copy link
Member

@JoeRobich JoeRobich left a comment

Choose a reason for hiding this comment

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

👍

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.

Semantic Highlighting Failures SemanticHighlighing Exception in dotnet/roslyn/...ArrayBuilder.cs
3 participants