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

Update feature branch #10652

Merged
merged 207 commits into from
Jul 19, 2024
Merged

Update feature branch #10652

merged 207 commits into from
Jul 19, 2024

Conversation

marcarro
Copy link
Contributor

Summary of the changes

Update feature branch to keep up with current changes in the main branch.

Fixes:

tmat and others added 30 commits April 29, 2022 15:22
# Conflicts:
#	src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormatter.cs
#	src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/FormattingLanguageServerClient.cs
#	src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/FormattingTestBase.cs
Since we were talking in the working group meeting, thought I'd give it
a go and see if it passes
# Conflicts:
#	src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormatter.cs
#	src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpOnTypeFormattingPass.cs
#	src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/FormattingLanguageServerClient.cs
#	src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/TestRazorFormattingService.cs
* Removing unnecessary Razor Legacy Editor feature flag

AFAIK it was introduced for new LSP editor rollout several years ago before we had Tools/Options/HTML Editor/Advanced/Use legacy Razor Editor option. Now that rollout is over, and there is a public non-preview mechanism for users to switch to the old editor if needed, the feature flag is no longer necessary and creates confusion (and extra code to test/maintain).

* Fixing a unit test (test matrix)

* Fixing spacing per CR suggestion

Co-authored-by: David Wengier <david.wengier@microsoft.com>

---------

Co-authored-by: David Wengier <david.wengier@microsoft.com>
This PR is a style-only clean up of our code actions handlers and
related code. Definitely could have gone further (more collection
pooling, returning concrete types etc.) but I wanted to try to have it
serve as an indication for what we consider our current idiomatic coding
style* and to avoid conflicts with the extract to component work.



<sub>* Current style is subject to change at any time, and are not well
documented. To quote a great Australian film, [it's just the
vibe](https://www.youtube.com/watch?v=nMuh33BMZYY).</sub>
* Update dependencies from https://github.com/dotnet/arcade build 20240702.2

Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk
 From Version 8.0.0-beta.24352.1 -> To Version 9.0.0-beta.24352.2

* Install .NET 8 runtime

* Update dependencies from https://github.com/dotnet/arcade build 20240702.2

Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk
 From Version 8.0.0-beta.24352.1 -> To Version 9.0.0-beta.24352.2

* Disable BuildWithNetFrameworkHostedCompiler

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jan Jones <janjones@microsoft.com>
phil-allen-msft and others added 23 commits July 18, 2024 08:09
* Update name away from now-former nation name

* Update packages to modern versions

* Change package version
…est/Cohost/CohostEndpointTestBase.cs

Co-authored-by: Dustin Campbell <dustin@teamcampbell.org>
Part of #9519 and
#10603
Requires dotnet/roslyn#74402

Removes a little more dodginess in the cohosting tests by actually using
the `RazorPinnedSolutionInfoWrapper` for solution checksums, just like
the real OOP services.
We have tests with baselines that have trailing whitespaces. Our `trimTrailingWhitespace` setting means that those will get modified automatically, breaking those tests. To fix that, I implemented a vscode feature to avoid triming inside regex and strings, so let's use that.
Requires dotnet/roslyn#74280, and won't even
compile without it.
Part of #9519

This brings signature help to Cohosting. It's a pretty simply PR, for a
pretty simple endpoint, as we just delegate, and there is no translation
of delegated info. The interesting part here is that we use
`System.Text.Json` for the remote signature help service, because it
makes more sense to take advantage of the existing Json converters for
the potential complexity of the `SignatureHelp` result type.
Having two sln files in the root directory means that `dotnet` CLI commands get confused about what sln to use and need to have it specified. To fix, we just move this solution file into eng, since it's only used for full builds anyways.
Part of #9519 and
#10603

After this, all cohost endpoints have test coverage 😁
* Add tests

* Fix attribute parsing recovery

* Update new baselines

* Fix expected missing source mapping

* Update pre-existing baselines

* Simplify code
I recommend reviewing this commit-by-commit.

I took a look at logging to address an integration test issue where we
should ignore any exceptions that occur during logging because xUnit's
`ITestOutputHelper` is no longer available. Previously, we had swallowed
these exceptions across all unit tests as well. However, in unit tests,
these exceptions can be an early warning sign of other problems.

While addressing integration tests, I ended up taking a more thorough
look at logging and ended up re-implementing the log message formatting
to reduce string allocations. In addition, I cleaned up a fair number of
redundant "no-op" loggers and did an audit of all ILoggerProviders to
determine which ones needed to implement IDisposable.
@marcarro marcarro requested review from a team as code owners July 19, 2024 20:24
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.