I nerd sniped myself thinking about how to get formatting into
cohosting, given the limitations Alex ran into doing the relayering for
auto insert, and this is the result. I was going to go further and port
the actual formatting endpoint to cohosting, but that would have ran
into the same issue that Alex did with auto insert, so I figured I'd
wait for that to merge, and put this up in the meantime.
This unblocks the formatting, code action and completion end points from
being ported.
Part of https://github.com/dotnet/razor/issues/10743
Part of https://github.com/dotnet/razor/issues/9519
I **strongly** recommend reviewing commit-at-a-time, as I did this
deliberately in an order, and in order to (hopefully) make reviewing
easier. Though granted, there are a lot of commits.
Fixes an issue found in app building. Thanks @phil-allen-msft!
Also fixes https://github.com/dotnet/razor/issues/9161
To make reviewing easier:
* First commit is entirely mechanical cleanup, renames, etc. and can be
skipped.
* Second commit is the fix.
* Third commit is tests.
* Fourth commit is updating more tests because these days when you ask
VS to build things it doesn't build all of it and I need to get into the
habit of doing a command line build before pushing
Because of the type and file renames, looking at the PR as a whole is
inadvised.
* Factoring out AutoInsertService
* Switch non-cohost endpoint to use new AutoInsertService
* Adding Remote and OOB AutoInsertService classes and OnAutoInsertProviders
* Add common code for capabilities
* Add cohost OnAutoInsert endpoint
* Parameter rename
* Update src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/AutoInsert/AutoClosingTagOnAutoInsertProvider.cs
Co-authored-by: David Wengier <david.wengier@microsoft.com>
* Update src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/AutoInsert/IAutoInsertService.cs
Co-authored-by: David Wengier <david.wengier@microsoft.com>
* Update src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/AutoInsert/InsertTextEdit.cs
Co-authored-by: David Wengier <david.wengier@microsoft.com>
* Fixup after rebase
* More post-build cleanup
* Move common HTML and C# auto-insert trigger chars per CR suggestion
* More cleanup after rebase
* Add IOnAutoInsertTiggerCharacterProvider per CR suggestion
* Removing trigger characters property from RemoteAutoInsertService per CR suggestion
* Switch to using RemoteResponse
* Fixup bad resolve after rebase, and extra whitespace in RazorLangaugeServer
* Complete capabilities check in CohostingOnAutoInsertEndpoing registration
* Change input position type to serializable :LinePosition
* Fixing RemoteInsertTextEdit to use properly annotated (for serialization) types only
* Support for delegating auto-insert to C#
* Fixup after rebase
* Fixup AutoClosingTagOnAutoInsertProviderTest tests
* Fixinfg up CloseTextTagOnAutoInsertProviderTest tests
* Fixing up OnAutoInsertEndpointTest (which also tests the new AutoInsertService)
* Fixing duplicate OnAutoInsert handler registration (bad merge after rebate)
* Fixes to MEF composition issue and capabilities check
- IDocumentMappingService was not needed (and not available via MEF), so removed that
- TextDocument does not implmement VSInternalTextDocumentClientCapabilities
* Fixing incorrect export type
* Minor cleanup per CR suggestions
* Switching parameters to RazorCodeDocument and removing async in a lot of places per CR suggestion
* Fixing build - removing unneeded using
* Fix RemoteAutoInsertService logic to follow existing code (always prefer our own AutoInsertService first)
* Check allowed trigger characters before delegating to other languages/servers
* Plumbing through actual option values we need and using them
* Fixup After Rebase
* Consuming RazorFormattingService in remote OnAutoInsert service
* Fixing exception in RemoteProjectSnapshot.Configuration
Moving GetFormatterCodeDocumentAsync() into IDocumentSnapshot (and implementations of that) to allow eaiser differentiation of behavior in remote (cohosting) case where we don't need to check the flag on Project.Configuration.
Also AddUsingStatementsIfNeeded *always* gets called, even in cases when they are not actually needed, so we can't Debug.Fail there.
* Switch to PreferHtmlInAttributeValuesDocumentPositionInfoStrategy as the original code does
That allows the code insert double-quotes by delegating to HTML language server after attribute name and equals.
* Cleanup usings
* More usings cleanup
* PR feedback - minor cleanup and removal of InsertTextEdit type in favor of VSInternalDocumentOnAutoInsertResponseItem
* PR feedback - removing MEF usage from RemoteAdhocWorkspaceFactory, minor cleanup
* PR feedback
* Type and method renames, minor cleanup per PR feedback
* Renaming interface methods, removing unnecessary usage of FrozenSet
* Use ImmutableArray to store providers
* Switching to bool and out on IAutoInsertService and implementation
* Cleaned up document position info calculation per PR feedback
* Minor cleanup in RemoteAutoInsertService
* Cleanup trigger character calculation in CohostOnAutoInsertEndpoint
* Formatting changes, VsLspFactory usage
* Revert to MEF DI in RemoteAdhocWorkspaceFactory/RemoteRazorFormattingService
* Removing Rolsyn to VS LSP extension methods
* Switch to raw strings in tests and minor whitespace cleanup
* Rename per PR feedback suggestion to better indicate the purpose
* Cleanup formatting code document acquisition per PR feedback
Creating IFormattingCodeDocumentProvider service with seprate LSP and Remote implementations to provide code document appropriate for formatting.
* Removing redundant assert as compiler is already doing the check
* Unnecessary assignment cleanup
* Misc PR feedback cleanup
* Made GetGeneratedOutputAsync an extension method per PR feedback
* Remaining MEF parameter attrivute formatting changes
* Last of the PR feedback.
* Fixing most unit test failures.
Extension methods can't be used for Mock setups, so since I made GetGeneratedOutputAsync() with no parameter an extension method, I had to switch unit tests to use GeGeneratedOutputAsync(It.Any<bool>())
* Fixing last 4 unit test failures
We still had non-parameter GetGeneratedOutputAsycnc in DocumentSnapshot (even though it wasn't in IDocumentSnapshot) which was getting called internally. That was both messy (since there is now no-parameter extension method on IDocumentSnapshot) and was causing issues in tests
---------
Co-authored-by: David Wengier <david.wengier@microsoft.com>
Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2222322
Our use of tildes as replacement characters for C# in Html documents has
never been great, and has caused issues with JS/TS and Html tooling in
the past, but it seems there are scenarios where
large/complicated/specifically formed Razor documents can actually crash
the JS compiler. Seems it has a perf issue where lots of unary negation
(ie, `~`) causes stack size issues.
This PR mitigates the issue by encoding any stretch of C# characters
that are 4 characters or longer as a JS comment (`/*~~~*/`) so that the
compiler will ignore some of the more problematic chunks of Razor files.
There is one case where we couldn't do that, but any pressure relief
should help even if it's not 100%.
This is an automatically generated pull request from release/dev17.12
into main.
Once all conflicts are resolved and all the tests pass, you are free to
merge the pull request. 🐯
## Troubleshooting conflicts
### Identify authors of changes which introduced merge conflicts
Scroll to the bottom, then for each file containing conflicts copy its
path into the following searches:
- https://github.com/dotnet/razor/find/release/dev17.12
- https://github.com/dotnet/razor/find/main
Usually the most recent change to a file between the two branches is
considered to have introduced the conflicts, but sometimes it will be
necessary to look for the conflicting lines and check the blame in each
branch. Generally the author whose change introduced the conflicts
should pull down this PR, fix the conflicts locally, then push up a
commit resolving the conflicts.
### Resolve merge conflicts using your local repo
Sometimes merge conflicts may be present on GitHub but merging locally
will work without conflicts. This is due to differences between the
merge algorithm used in local git versus the one used by GitHub.
``` bash
git fetch --all
git checkout -t upstream/merges/release/dev17.12-to-main
git reset --hard upstream/main
git merge upstream/release/dev17.12
# Fix merge conflicts
git commit
git push upstream merges/release/dev17.12-to-main --force
```
Fixes integration test failures in Find All References.
Roslyns LSP types got much more spec compliant in
https://github.com/dotnet/roslyn/pull/73911 and we were never sending
the `Context` property in our request, so deserialization failed on
their end.
Fixes https://github.com/dotnet/razor/issues/10759
Initially when investigating this I was digging into buffer versions and
race conditions, but now that the editor fix is in, and we are self
versioned, the bug stood out much more obviously: A character offset was
being passed in to a parameter that expected a length 🤦♂️
Sadly none of the `RazorCustomMessageTarget` code is testable. Cohosting
will make this trivial though, as we would simply create a new
`Document` with the provisional change, and drop it on the floor when
we're finished.
This is an automatically generated pull request from release/dev17.12
into main.
Once all conflicts are resolved and all the tests pass, you are free to
merge the pull request. 🐯
## Troubleshooting conflicts
### Identify authors of changes which introduced merge conflicts
Scroll to the bottom, then for each file containing conflicts copy its
path into the following searches:
- https://github.com/dotnet/razor/find/release/dev17.12
- https://github.com/dotnet/razor/find/main
Usually the most recent change to a file between the two branches is
considered to have introduced the conflicts, but sometimes it will be
necessary to look for the conflicting lines and check the blame in each
branch. Generally the author whose change introduced the conflicts
should pull down this PR, fix the conflicts locally, then push up a
commit resolving the conflicts.
### Resolve merge conflicts using your local repo
Sometimes merge conflicts may be present on GitHub but merging locally
will work without conflicts. This is due to differences between the
merge algorithm used in local git versus the one used by GitHub.
``` bash
git fetch --all
git checkout -t upstream/merges/release/dev17.12-to-main
git reset --hard upstream/main
git merge upstream/release/dev17.12
# Fix merge conflicts
git commit
git push upstream merges/release/dev17.12-to-main --force
```
This is an automatically generated pull request from release/dev17.11
into release/dev17.12.
Once all conflicts are resolved and all the tests pass, you are free to
merge the pull request. 🐯
## Troubleshooting conflicts
### Identify authors of changes which introduced merge conflicts
Scroll to the bottom, then for each file containing conflicts copy its
path into the following searches:
- https://github.com/dotnet/razor/find/release/dev17.11
- https://github.com/dotnet/razor/find/release/dev17.12
Usually the most recent change to a file between the two branches is
considered to have introduced the conflicts, but sometimes it will be
necessary to look for the conflicting lines and check the blame in each
branch. Generally the author whose change introduced the conflicts
should pull down this PR, fix the conflicts locally, then push up a
commit resolving the conflicts.
### Resolve merge conflicts using your local repo
Sometimes merge conflicts may be present on GitHub but merging locally
will work without conflicts. This is due to differences between the
merge algorithm used in local git versus the one used by GitHub.
``` bash
git fetch --all
git checkout -t upstream/merges/release/dev17.11-to-release/dev17.12
git reset --hard upstream/release/dev17.12
git merge upstream/release/dev17.11
# Fix merge conflicts
git commit
git push upstream merges/release/dev17.11-to-release/dev17.12 --force
```
This is an automatically generated pull request from release/dev17.12
into main.
Once all conflicts are resolved and all the tests pass, you are free to
merge the pull request. 🐯
## Troubleshooting conflicts
### Identify authors of changes which introduced merge conflicts
Scroll to the bottom, then for each file containing conflicts copy its
path into the following searches:
- https://github.com/dotnet/razor/find/release/dev17.12
- https://github.com/dotnet/razor/find/main
Usually the most recent change to a file between the two branches is
considered to have introduced the conflicts, but sometimes it will be
necessary to look for the conflicting lines and check the blame in each
branch. Generally the author whose change introduced the conflicts
should pull down this PR, fix the conflicts locally, then push up a
commit resolving the conflicts.
### Resolve merge conflicts using your local repo
Sometimes merge conflicts may be present on GitHub but merging locally
will work without conflicts. This is due to differences between the
merge algorithm used in local git versus the one used by GitHub.
``` bash
git fetch --all
git checkout -t upstream/merges/release/dev17.12-to-main
git reset --hard upstream/main
git merge upstream/release/dev17.12
# Fix merge conflicts
git commit
git push upstream merges/release/dev17.12-to-main --force
```
Coherency update: Failed to perform coherency update for one or more
dependencies. Please review the GitHub checks or run `darc
update-dependencies --coherency-only` locally against
darc-main-a20bf2b8-a79a-4f26-a141-1afa2bfa58d0 for more information.
[marker]: <> (Begin:2907dbca-fa2e-42bc-f7dd-08dc0c5b4e6d)
## From https://github.com/dotnet/arcade
- **Subscription**: 2907dbca-fa2e-42bc-f7dd-08dc0c5b4e6d
- **Build**: 20240826.3
- **Date Produced**: August 26, 2024 5:01:25 PM UTC
- **Commit**: e3bdd9a0f2a65fe037ba1adb2261eea48a840fa4
- **Branch**: refs/heads/main
[DependencyUpdate]: <> (Begin)
- **Updates**:
- **Microsoft.SourceBuild.Intermediate.arcade**: [from
9.0.0-beta.24423.2 to 9.0.0-beta.24426.3][1]
- **Microsoft.DotNet.Arcade.Sdk**: [from 9.0.0-beta.24423.2 to
9.0.0-beta.24426.3][1]
[1]: 9159926865...e3bdd9a0f2
[DependencyUpdate]: <> (End)
[marker]: <> (End:2907dbca-fa2e-42bc-f7dd-08dc0c5b4e6d)