Razor snippets by dibarbet · Pull Request #58857 · dotnet/roslyn

dibarbet · 2022-01-14T04:20:01Z

Implements support for built in snippets for Razor LSP using inline completions. This reads the XML snippet information, applies function results and then converts into an LSP formatted snippet.

While this should work for many XML snippets including custom ones, there are edge cases that I haven't tackled yet (no selection support, no add imports/references support). As such I've restricted this to built in snippets . Support for the remaining will require further iteration on both the lsp client portion and server world.

I will additionally have a followup PR to cache the snippets as we parse from XML.

dibarbet · 2022-01-26T04:56:47Z

+namespace Microsoft.CodeAnalysis.CSharp;
+
+[ExportLanguageService(typeof(SnippetFunctionService), LanguageNames.CSharp), Shared]
+internal class CSharpSnippetFunctionService : SnippetFunctionService


extracted this code from the c# snippet functions
https://github.com/dotnet/roslyn/tree/main/src/VisualStudio/CSharp/Impl/Snippets/SnippetFunctions

dibarbet · 2022-01-26T04:57:26Z

+using Microsoft.CodeAnalysis.Text;
+
+namespace Microsoft.CodeAnalysis;
+internal abstract class SnippetFunctionService : ILanguageService


this class was extracted from https://github.com/dotnet/roslyn/tree/main/src/VisualStudio/Core/Def/Implementation/Snippets/SnippetFunctions and converted into a service to be shared by lsp

dibarbet · 2022-01-26T04:59:10Z

+namespace Microsoft.CodeAnalysis;
+internal class SnippetUtilities
+{
+    public static bool TryGetWordOnLeft(int position, SourceText currentText, ISyntaxFactsService syntaxFactsService, [NotNullWhen(true)] out TextSpan? wordSpan)


extracted from

roslyn/src/VisualStudio/Core/Def/Implementation/Snippets/AbstractSnippetCommandHandler.cs

Line 295 in b0f460c

var endPosition = endPositionInSubjectBuffer.Value.Position;

dibarbet · 2022-01-26T05:02:24Z

+
+namespace Microsoft.CodeAnalysis.LanguageServer.Handler.InlineCompletions;
+
+internal partial class InlineCompletionsHandler


code here was extracted from similar editor side code that reads snippet xml files and modified a bit to support our use case.

If we need to support XML based snippets long term, it would be nice if an API existed on the editor side to retrieve these. We would have to still do some conversion since some snippets are context dependent, but reading and parsing the XML could be left to the editor.

That being said, semantic snippets will probably make this irrelevant for built in c# snippets, and the editor is re-thinking the snippet experience so I don't believe it's worth doing anything right now.

dibarbet · 2022-01-26T05:03:51Z

+            }
+        }
+
+        public ParsedXmlSnippet? Parse()


much of the parsing logic remains the same as the battle-tested editor code, the major change is to convert the snippet into 'SnippetParts' instead of just a pure string with default values.

dibarbet · 2022-01-26T05:05:21Z

+                    // Generate switch cases requires a multi-step snippet interaction, where the snippet is inserted first then the
+                    // client calls back to on commit so that we can generate the cases for the specified switch value.
+                    // This is not yet supported via LSP.
+                    return this;


Didn't realize this is a two step process at the time of writing the LSP api :(
Future work here is to figure out a way for LSP to call back on the 'commit' of a tab stop and ask the server for more edits.

Interesting, is this also handled in the new semantic snippets?

semantic snippets would have to support it in some manner - the cases can only be known when the field is committed

dibarbet · 2022-01-26T05:06:02Z

+/// </summary>
+[ExportRoslynLanguagesLspRequestHandlerProvider, Shared]
+[ProvidesMethod(VSInternalMethods.TextDocumentInlineCompletionName)]
+internal partial class InlineCompletionsHandler : AbstractStatelessRequestHandler<VSInternalInlineCompletionRequest, VSInternalInlineCompletionList>


the meat and potatoes of the actual new logic (that wasn't copied from the editor)

dibarbet · 2022-01-26T05:07:30Z

+        var expansion = new ExpansionTemplate(matchingSnippet);
+
+        // Parse the snippet XML
+        var parsedSnippet = expansion.Parse();


the parsed snippet needs to be cached. Am working on that, but will do as a separate PR to reduce the size here

Note that we cannot cache the actual LSP snippet, as it is context dependent (class names / formatting), but we can cache the snippet parts we read from XML.

We also may want to build specific error handling around this given it's reading user content. Also might make sense to do the same with RetrieveSnippetFromXml above given that reads content from disk

I think I'm OK with the amount of error handling in there right now. currently we do some basic checks ot make sure the file exists. But otherwise if there are malformed snippets, imho it is ok to let those exceptions bubble up to the queue and let them be automatically reported and logged.

Happy to hear other thoughts though.

I did find a few cases I should probably handle actually, ~~so will update~~ updated

dibarbet · 2022-01-26T05:11:19Z

-
-namespace Microsoft.VisualStudio.LanguageServices.CSharp.Snippets.SnippetFunctions
-{
-    internal sealed class SnippetFunctionClassName : AbstractSnippetFunctionClassName


the logic here can now live in a single impl for each language since the language specific logic is now in the snippetfunctionservice (and children)

dibarbet · 2022-01-26T05:19:29Z

+    /// C:\Program Files\Microsoft Visual Studio\2022\VS_INSTANCE\VC#\Snippets\1033\Visual C#
+    /// These are currently the only snippets supported.
+    /// </summary>
+    public static ImmutableHashSet<string> BuiltInSnippets = ImmutableHashSet.Create(


I've limited these to built in snippets for now. While the parsing could probably handle most custom snippets, there are a few things that would not work, namely snippets with additional references (requires lsp response to support additional text edits) and selection (currently snippets are only invocable via tab-tab, so selection does not make sense in that context).

Depending on the direction that the editor takes xml snippets and where we go with semantic snippets, we can potentially open this up to more with a few improvements on the lsp api.

NTaylorMullen

This is super exciting and great work! I imagine the path you're taking here would also be used by our friends over on the XAML language service side too given it could potentially enable custom snippets too. Quite flexible.

I had some overarching concerns around the snippet parsing tech but reaching out offline about that.

NTaylorMullen · 2022-01-26T18:01:28Z

+                    // Generate switch cases requires a multi-step snippet interaction, where the snippet is inserted first then the
+                    // client calls back to on commit so that we can generate the cases for the specified switch value.
+                    // This is not yet supported via LSP.
+                    return this;


Interesting, is this also handled in the new semantic snippets?

NTaylorMullen · 2022-01-26T18:03:37Z

+
+namespace Microsoft.CodeAnalysis.LanguageServer.Handler.InlineCompletions;
+
+internal partial class InlineCompletionsHandler


Do you typically utilize partial classes heavily in Roslyn? They typically fall over in some source linked debug scenarios + make understanding implementation details a bit more tedious. Would it be awful to pull all the XML snippet tech into a service that the primary InlineCompletionsHandler utilizes?

partial classes are not uncommon in roslyn. It sounds like source link debug scenarios need to be more resilient ;)

I don't mind either way really

NTaylorMullen · 2022-01-26T18:09:29Z

+        var expansion = new ExpansionTemplate(matchingSnippet);
+
+        // Parse the snippet XML
+        var parsedSnippet = expansion.Parse();


We also may want to build specific error handling around this given it's reading user content. Also might make sense to do the same with RetrieveSnippetFromXml above given that reads content from disk

mgoertz-msft · 2022-01-27T16:56:25Z

This is super exciting and great work! I imagine the path you're taking here would also be used by our friends over on the XAML language service side too given it could potentially enable custom snippets too. Quite flexible.

Totally agree! I haven't fully wrapped my mind around how to utilize this for custom snippets in XAML but I'm excited to see that the foundation has been laid.

NTaylorMullen · 2022-01-27T17:54:16Z

/cc @jimmylewis @ToddGrun if you're curious on snippets 😄

NTaylorMullen

It's happpppennnningggg!@!!!

ghost added the Area-IDE label Jan 14, 2022

dibarbet force-pushed the razor_snippets branch from 56417e2 to 3517692 Compare January 14, 2022 04:21

runfoapp Bot mentioned this pull request Jan 14, 2022

Test failure in Microsoft.CodeAnalysis.CSharp.Emit.UnitTests.dll #57560

Closed

dibarbet added 2 commits January 19, 2022 23:53

Upgrade protocol version

21a0631

Support built in XML snippets via LSP inline completion

dceba2a

dibarbet force-pushed the razor_snippets branch from 9f05f30 to 2a64a09 Compare January 20, 2022 07:53

Add support for snippet functions

50931d2

dibarbet force-pushed the razor_snippets branch from 2a64a09 to 50931d2 Compare January 22, 2022 09:29

dibarbet added 4 commits January 24, 2022 20:42

Simplify snippet formatting

9e3211f

Cleanup bits and pieces

4869cd6

Add unit tests

7a24ec1

small cleanup

12728e2

dibarbet mentioned this pull request Jan 26, 2022

Add support for invoking C# inline completions dotnet/razor#6012

Merged

enable snippets in completion

c37ead8

dibarbet marked this pull request as ready for review January 26, 2022 04:55

dibarbet requested review from a team as code owners January 26, 2022 04:55

dibarbet added the LSP issues related to the roslyn language server protocol implementation label Jan 26, 2022

dibarbet commented Jan 26, 2022

View reviewed changes

dibarbet requested review from NTaylorMullen and akhera99 January 26, 2022 05:21

JoeRobich reviewed Jan 26, 2022

View reviewed changes

Comment thread src/EditorFeatures/Core/Implementation/LanguageServer/VisualStudioInProcLanguageServer.cs Outdated

NTaylorMullen reviewed Jan 26, 2022

View reviewed changes

dibarbet added 2 commits January 27, 2022 17:36

review feedback

7501b4f

Explicitly catch/report exceptions during xml snippet parsing

9be6faa

dibarbet force-pushed the razor_snippets branch from eaa0bb0 to 9be6faa Compare January 28, 2022 02:04

NTaylorMullen approved these changes Jan 28, 2022

View reviewed changes

dibarbet added 2 commits January 27, 2022 18:27

Merge remote-tracking branch 'upstream/main' into razor_snippets

49df3d2

fix build warnings

33dc880

dibarbet merged commit 3f5adda into dotnet:main Feb 2, 2022

dibarbet deleted the razor_snippets branch February 2, 2022 20:39

ghost added this to the Next milestone Feb 2, 2022

RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 2022


		namespace Microsoft.CodeAnalysis.LanguageServer.Handler.InlineCompletions;

		internal partial class InlineCompletionsHandler

Conversation

dibarbet commented Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dibarbet Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dibarbet Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NTaylorMullen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mgoertz-msft commented Jan 27, 2022

Uh oh!

NTaylorMullen commented Jan 27, 2022

Uh oh!

NTaylorMullen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dibarbet commented Jan 14, 2022 •

edited

Loading

dibarbet Jan 26, 2022 •

edited

Loading

dibarbet Jan 28, 2022 •

edited

Loading