-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Extract raw string literal auto-insert logic into reusable service #80871
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
Conversation
Co-authored-by: dibarbet <5749229+dibarbet@users.noreply.github.com>
Co-authored-by: dibarbet <5749229+dibarbet@users.noreply.github.com>
Co-authored-by: dibarbet <5749229+dibarbet@users.noreply.github.com>
|
I can review tomorrow |
| namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests.OnAutoInsert; | ||
|
|
||
| [Trait(Traits.Feature, Traits.Features.AutomaticCompletion)] | ||
| public abstract class AbstractOnAutoInsertTests(ITestOutputHelper testOutputHelper) : AbstractLanguageServerProtocolTests(testOutputHelper) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all this is moved
| TryGenerateInitialEmptyRawString(caret.Value, cancellationToken) ?? | ||
| TryGrowInitialEmptyRawString(caret.Value, cancellationToken) ?? | ||
| TryGrowRawStringDelimiters(caret.Value, cancellationToken); | ||
| var root = document.GetRequiredSyntaxRootSynchronously(cancellationToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a problem for me. A core idea in the original code was that it did nothing related to syntax unless it first had verified textually that it was in a reasonable position to do so (like checking if it was around quotes). This prevented parsing the syntax tree for most edits.
I don't want to roll that logic back. Can the service just take the document and defer creating the root until needed?
| var textChange = service.GetTextChangeForQuote(root, text, caret.Value.Position); | ||
|
|
||
| if (textChangeOpt is not TextChange textChange) | ||
| if (textChange is null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like you could keep this code :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/Features/CSharp/Portable/RawStringLiteral/CSharpRawStringLiteralOnAutoInsertService.cs
Show resolved
Hide resolved
CyrusNajmabadi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tentatively approving, since hte overall change is good. just please change to defer syntaxroot acquisition until needed.
Three things I've been tracking from Roslyn: 1. Auto insert support for raw string literals: dotnet/roslyn#80871 2. Line Folding Only support in VS Code: dotnet/roslyn#80955 (fixes #12414) * We already did this in our folding range response anyway 3. Allow FormattingLogTest can use real types, and remove our temporary equivalent

Resolves #68537 by extracting the custom Visual Studio behavior for raw string literal auto-insertion into a workspace service that can be shared between the VS command handler and LSP OnAutoInsert handler.
Problem
Previously, the logic for automatically expanding raw string literals when typing quote characters was hardcoded in the VS-specific command handler (
RawStringLiteralCommandHandler_TypeChar.cs). This made it impossible to provide the same functionality through LSP for other editors.Solution
This PR extracts the raw string literal expansion logic into a new workspace service:
Created
IRawStringLiteralOnAutoInsertServiceinterface in the Features layerTextChangeif raw string expansion should occurnullif no special handling is neededImplemented
CSharpRawStringLiteralOnAutoInsertServicein the C# Features layer""+"→"""""""""$$"""+"→""""$$"""""""$$ content """+"→"""" content """"Updated
RawStringLiteralCommandHandler_TypeChar.csto use the serviceIntegrated with
OnAutoInsertHandler.csImpact
The service follows Roslyn's standard patterns with
[ExportLanguageService]and proper MEF construction.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/repos/dotnet/roslyn/issues/68537curl -s REDACTED(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.