Merge pull request #69893 from dibarbet/hover_fixes

Fix escaping and wrapping in vscode hover
dotnet · Sep 13, 2023 · f868ca1 · f868ca1
2 parents 938295b + d385fab
commit f868ca1
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 22 deletions.
diff --git a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs
@@ -892,15 +892,17 @@ static string GetCodeBlockLanguageName(string language)
 
             static string GetStyledText(TaggedText taggedText, bool isInCodeBlock)
             {
-                var text = isInCodeBlock ? taggedText.Text : s_markdownEscapeRegex.Replace(taggedText.Text, @"\$1");
+                var isCode = isInCodeBlock || taggedText.Style is TaggedTextStyle.Code;
+                var text = isCode ? taggedText.Text : s_markdownEscapeRegex.Replace(taggedText.Text, @"\$1");
 
                 // For non-cref links, the URI is present in both the hint and target.
                 if (!string.IsNullOrEmpty(taggedText.NavigationHint) && taggedText.NavigationHint == taggedText.NavigationTarget)
                     return $"[{text}]({taggedText.NavigationHint})";
 
                 // Markdown ignores spaces at the start of lines outside of code blocks, 
-                // so to get indented lines we replace the spaces with these.
-                if (!isInCodeBlock)
+                // so we replace regular spaces with non-breaking spaces to ensure structural space is retained.
+                // We want to use regular spaces everywhere else to allow the client to wrap long text.
+                if (!isCode && taggedText.Tag is TextTags.Space or TextTags.ContainerStart)
                     text = text.Replace(" ", "&nbsp;");
 
                 return taggedText.Style switch

diff --git a/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionResolveTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionResolveTests.cs
@@ -250,13 +250,13 @@ void M()
 void A.AMethod(int i)
 ```
   
-A&nbsp;cref&nbsp;A\.AMethod\(int\)  
-**strong&nbsp;text**  
-_italic&nbsp;text_  
-<u>underline&nbsp;text</u>  
+A cref&nbsp;A\.AMethod\(int\)  
+**strong text**  
+_italic text_  
+<u>underline text</u>  
   
-•&nbsp;Item&nbsp;1\.  
-•&nbsp;Item&nbsp;2\.  
+•&nbsp;Item 1\.  
+•&nbsp;Item 2\.  
   
 [link text](https://google.com)";
 

diff --git a/src/Features/LanguageServer/ProtocolUnitTests/Hover/HoverTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/Hover/HoverTests.cs
@@ -245,20 +245,20 @@ public async Task TestGetHoverAsync_UsingMarkupContent(bool mutatingLspWorkspace
 void A.AMethod(int i)
 ```
   
-A&nbsp;cref&nbsp;A\.AMethod\(int\)  
-**strong&nbsp;text**  
-_italic&nbsp;text_  
-<u>underline&nbsp;text</u>  
+A cref&nbsp;A\.AMethod\(int\)  
+**strong text**  
+_italic text_  
+<u>underline text</u>  
   
-•&nbsp;Item&nbsp;1\.  
-•&nbsp;Item&nbsp;2\.  
+•&nbsp;Item 1\.  
+•&nbsp;Item 2\.  
   
 [link text](https://google.com)  
   
-Remarks&nbsp;are&nbsp;cool&nbsp;too\.  
+Remarks are cool too\.  
   
 {FeaturesResources.Returns_colon}  
-&nbsp;&nbsp;a&nbsp;string  
+&nbsp;&nbsp;a string  
   
 {FeaturesResources.Exceptions_colon}  
 &nbsp;&nbsp;System\.NullReferenceException  
@@ -375,13 +375,13 @@ public async Task TestGetHoverAsync_UsingMarkupContentProperlyEscapes(bool mutat
 void A.AMethod(int i)
 ```
   
-Some&nbsp;\{curly\}&nbsp;\[braces\]&nbsp;and&nbsp;\(parens\)  
+Some \{curly\} \[braces\] and \(parens\)  
 \#Hashtag  
-1&nbsp;\+&nbsp;1&nbsp;\-&nbsp;1  
+1 \+ 1 \- 1  
 Period\.  
 Exclaim\!  
-**strong\\\*\*&nbsp;text**  
-_italic\_&nbsp;\*\*text\*\*_  
+**strong\\\*\* text**  
+_italic\_ \*\*text\*\*_  
 [closing\] link](https://google.com)  
 ";
 
@@ -391,6 +391,43 @@ void A.AMethod(int i)
             Assert.Equal(expectedMarkdown, results.Contents.Value.Fourth.Value);
         }
 
+        [Theory, CombinatorialData]
+        public async Task TestGetHoverAsync_UsingMarkupContentDoesNotEscapeCode(bool mutatingLspWorkspace)
+        {
+            var markup =
+@"class A
+{
+    /// <summary>
+    /// <c>
+    /// if (true) {
+    ///     Console.WriteLine(""hello"");
+    /// }
+    /// </c>
+    /// </summary>
+    void {|caret:AMethod|}(int i)
+    {
+    }
+}";
+            var clientCapabilities = new LSP.ClientCapabilities
+            {
+                TextDocument = new LSP.TextDocumentClientCapabilities { Hover = new LSP.HoverSetting { ContentFormat = new LSP.MarkupKind[] { LSP.MarkupKind.Markdown } } }
+            };
+            await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, clientCapabilities);
+            var expectedLocation = testLspServer.GetLocations("caret").Single();
+
+            var expectedMarkdown = @"```csharp
+void A.AMethod(int i)
+```
+  
+`if (true) { Console.WriteLine(""hello""); }`  
+";
+
+            var results = await RunGetHoverAsync(
+                testLspServer,
+                expectedLocation).ConfigureAwait(false);
+            Assert.Equal(expectedMarkdown, results.Contents.Value.Fourth.Value);
+        }
+
         private static async Task<LSP.Hover> RunGetHoverAsync(
             TestLspServer testLspServer,
             LSP.Location caret,

diff --git a/src/Features/Lsif/GeneratorTest/HoverTests.vb b/src/Features/Lsif/GeneratorTest/HoverTests.vb
@@ -75,7 +75,7 @@ class C
 void C.M()
 ```
 
-Doc&nbsp;Comment  "
+Doc Comment  "
                 Case Else
                     Throw TestExceptionUtilities.UnexpectedValue(code)
             End Select