Merge pull request swiftlang#1183 from ahoppen/pretty-hover

Make hover popover prettier

Author: ahoppen

Sources/SourceKitLSP/Swift/CommentXML.swift

@@ -77,7 +77,7 @@ private struct XMLToMarkdown {
       newlineIfNeeded(count: 2)
       out += "```swift\n"
       toMarkdown(node.children)
-      out += "\n```\n\n---\n"
+      out += "\n```\n"
 
     case "Name", "USR", "Direction":
       break

Sources/SourceKitLSP/Swift/SwiftLanguageService.swift

@@ -569,46 +569,55 @@ extension SwiftLanguageService {
     let cursorInfoResults = try await cursorInfo(uri, position..<position).cursorInfo
 
     let symbolDocumentations = cursorInfoResults.compactMap { (cursorInfo) -> String? in
-      guard let name: String = cursorInfo.symbolInfo.name else {
-        // There is a cursor but we don't know how to deal with it.
-        return nil
-      }
-
-      /// Prepend backslash to `*` and `_`, to prevent them
-      /// from being interpreted as markdown.
-      func escapeNameMarkdown(_ str: String) -> String {
-        return String(str.flatMap({ ($0 == "*" || $0 == "_") ? ["\\", $0] : [$0] }))
-      }
-
-      var result = escapeNameMarkdown(name)
       if let documentation = cursorInfo.documentation {
+        var result = ""
         if let annotatedDeclaration = cursorInfo.annotatedDeclaration {
           let markdownDecl =
             orLog("Convert XML declaration to Markdown") {
               try xmlDocumentationToMarkdown(annotatedDeclaration)
             } ?? annotatedDeclaration
-          result += "\n\(markdownDecl)"
+          result += "\(markdownDecl)\n"
         }
         result += documentation
+        return result
       } else if let doc = cursorInfo.documentationXML {
-        result += """
-
+        return """
           \(orLog("Convert XML to Markdown") { try xmlDocumentationToMarkdown(doc) } ?? doc)
           """
       } else if let annotated: String = cursorInfo.annotatedDeclaration {
-        result += """
-
+        return """
           \(orLog("Convert XML to Markdown") { try xmlDocumentationToMarkdown(annotated) } ?? annotated)
           """
+      } else {
+        return nil
       }
-      return result
     }
 
     if symbolDocumentations.isEmpty {
       return nil
     }
 
-    let joinedDocumentation = symbolDocumentations.joined(separator: "\n# Alternative result\n")
+    let joinedDocumentation: String
+    if let only = symbolDocumentations.only {
+      joinedDocumentation = only
+    } else {
+      let documentationsWithSpacing = symbolDocumentations.enumerated().map { index, documentation in
+        // Work around a bug in VS Code that displays a code block after a horizontal ruler without any spacing
+        // (the pixels of the code block literally touch the ruler) by adding an empty line into the code block.
+        // Only do this for subsequent results since only those are preceeded by a ruler.
+        let prefix = "```swift\n"
+        if index != 0 && documentation.starts(with: prefix) {
+          return prefix + "\n" + documentation.dropFirst(prefix.count)
+        } else {
+          return documentation
+        }
+      }
+      joinedDocumentation = """
+        ## Multiple results
+
+        \(documentationsWithSpacing.joined(separator: "\n\n---\n\n"))
+        """
+    }
 
     return HoverResponse(
       contents: .markupContent(MarkupContent(kind: .markdown, value: joinedDocumentation)),
@@ -865,7 +874,7 @@ extension SwiftLanguageService {
       // Instead of returning an error, return empty results.
       logger.error(
         """
-        Loading diagnostic failed with the following error. Returning empty diagnostics. 
+        Loading diagnostic failed with the following error. Returning empty diagnostics.
         \(error.forLogging)
         """
       )

Tests/SourceKitLSPTests/HoverTests.swift

@@ -26,12 +26,10 @@ final class HoverTests: XCTestCase {
       struct 1️⃣S {}
       """,
       expectedContent: """
-        S
         ```swift
         struct S
         ```
 
-        ---
         This is a doc comment for S.
 
         Details.
@@ -70,21 +68,51 @@ final class HoverTests: XCTestCase {
       _ = 1️⃣Foo()
       """,
       expectedContent: """
-        Foo
+        ## Multiple results
+
         ```swift
         struct Foo
         ```
 
+
         ---
 
-        # Alternative result
-        init()
         ```swift
+
         init()
         ```
 
+        """
+    )
+  }
+
+  func testMultiCursorInfoResultsHoverWithDocumentation() async throws {
+    try await assertHover(
+      """
+      /// A struct
+      struct Foo {
+        /// The initializer
+        init() {}
+      }
+      _ = 1️⃣Foo()
+      """,
+      expectedContent: """
+        ## Multiple results
+
+        ```swift
+        struct Foo
+        ```
+
+        A struct
+
         ---
 
+        ```swift
+
+        init()
+        ```
+
+        The initializer
         """
     )
   }
@@ -96,12 +124,10 @@ final class HoverTests: XCTestCase {
       func 1️⃣test(_ a: Int, _ b: Int) { }
       """,
       expectedContent: ##"""
-        test(\_:\_:)
         ```swift
         func test(_ a: Int, _ b: Int)
         ```
 
-        ---
         this is **bold** documentation
         """##
     )
@@ -114,12 +140,10 @@ final class HoverTests: XCTestCase {
       func 1️⃣*%*(lhs: String, rhs: String) { }
       """,
       expectedContent: ##"""
-        \*%\*(\_:\_:)
         ```swift
         func *%* (lhs: String, rhs: String)
         ```
 
-        ---
         this is *italic* documentation
         """##
     )
@@ -135,12 +159,10 @@ final class HoverTests: XCTestCase {
       func 1️⃣eatApple() {}
       """,
       expectedContent: """
-        eatApple()
         ```swift
         func eatApple()
         ```
 
-        ---
         Eat an apple
 
         - Precondition: Must have an apple

Tests/SourceKitLSPTests/LocalSwiftTests.swift

@@ -755,8 +755,6 @@ final class LocalSwiftTests: XCTestCase {
       func foo(_ bar: Baz)
       ```
 
-      ---
-
       """
     )
     XCTAssertEqual(
@@ -770,8 +768,6 @@ final class LocalSwiftTests: XCTestCase {
       func foo() -> Bar
       ```
 
-      ---
-
       """
     )
     XCTAssertEqual(
@@ -805,8 +801,6 @@ final class LocalSwiftTests: XCTestCase {
       func replacingOccurrences<Target, Replacement>(of target: Target, with replacement: Replacement, options: String.CompareOptions = default, range searchRange: Range<String.Index>? = default) -> String where Target : StringProtocol, Replacement : StringProtocol
       ```
 
-      ---
-
       """
     )
   }
@@ -823,8 +817,6 @@ final class LocalSwiftTests: XCTestCase {
       var foo
       ```
 
-      ---
-
       """
     )
 
@@ -839,8 +831,6 @@ final class LocalSwiftTests: XCTestCase {
       var foo
       ```
 
-      ---
-
       """
     )
     XCTAssertEqual(
@@ -854,8 +844,6 @@ final class LocalSwiftTests: XCTestCase {
       var foo
       ```
 
-      ---
-
       """
     )
 
@@ -882,8 +870,6 @@ final class LocalSwiftTests: XCTestCase {
       var foo
       ```
 
-      ---
-
       """
     )
 
@@ -1064,8 +1050,6 @@ final class LocalSwiftTests: XCTestCase {
       ```swift
       struct String
       ```
-
-      ---
       A Unicode s
 
       ### Discussion
@@ -1147,8 +1131,6 @@ final class LocalSwiftTests: XCTestCase {
       ```swift
       struct S
       ```
-
-      ---
       ### Discussion
 
       ```swift