[clangd] Rearrange type, returntype and parameters in hover card

kadircet · kadircet · commit 13553d51aa87 · 2020-01-27T16:35:11.000+01:00
Summary: Moves type/returntype into its own line as it is more readable in cases where the type is long. Also gives parameter lists a heading, `Parameters:` to make them stand out. Leaves the `right arrow` instead of `Returns: ` before Return Type to make output more symmetric. ``` function foo Returns: ret_type Parameters: - int x ``` vs ``` function foo 🡺 ret_type Parameters: - int x ``` Reviewers: sammccall, ilya-biryukov Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D72623 (cherry picked from commit 44f9c7a)
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
@@ -510,41 +510,45 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
 markup::Document HoverInfo::present() const {
   markup::Document Output;
   // Header contains a text of the form:
-  // variable `var` : `int`
+  // variable `var`
   //
   // class `X`
   //
-  // function `foo` → `int`
+  // function `foo`
+  //
+  // expression
   //
-  // expression : `int`
   // Note that we are making use of a level-3 heading because VSCode renders
   // level 1 and 2 headers in a huge font, see
   // https://github.com/microsoft/vscode/issues/88417 for details.
   markup::Paragraph &Header = Output.addHeading(3);
   Header.appendText(index::getSymbolKindString(Kind));
   assert(!Name.empty() && "hover triggered on a nameless symbol");
   Header.appendCode(Name);
-  if (ReturnType) {
-    Header.appendText("→");
-    Header.appendCode(*ReturnType);
-  } else if (Type) {
-    Header.appendText(":");
-    Header.appendCode(*Type);
-  }
 
   // Put a linebreak after header to increase readability.
   Output.addRuler();
-  // For functions we display signature in a list form, e.g.:
-  // - `bool param1`
-  // - `int param2 = 5`
-  if (Parameters && !Parameters->empty()) {
-    markup::BulletList &L = Output.addBulletList();
-    for (const auto &Param : *Parameters) {
-      std::string Buffer;
-      llvm::raw_string_ostream OS(Buffer);
-      OS << Param;
-      L.addItem().addParagraph().appendCode(std::move(OS.str()));
+  // Print Types on their own lines to reduce chances of getting line-wrapped by
+  // editor, as they might be long.
+  if (ReturnType) {
+    // For functions we display signature in a list form, e.g.:
+    // 🡺 `x`
+    // Parameters:
+    // - `bool param1`
+    // - `int param2 = 5`
+    Output.addParagraph().appendText("🡺").appendCode(*ReturnType);
+    if (Parameters && !Parameters->empty()) {
+      Output.addParagraph().appendText("Parameters:");
+      markup::BulletList &L = Output.addBulletList();
+      for (const auto &Param : *Parameters) {
+        std::string Buffer;
+        llvm::raw_string_ostream OS(Buffer);
+        OS << Param;
+        L.addItem().addParagraph().appendCode(std::move(OS.str()));
+      }
     }
+  } else if (Type) {
+    Output.addParagraph().appendText("Type: ").appendCode(*Type);
   }
 
   if (Value) {
diff --git a/clang-tools-extra/clangd/test/hover.test b/clang-tools-extra/clangd/test/hover.test
@@ -9,7 +9,7 @@
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "contents": {
 # CHECK-NEXT:      "kind": "plaintext",
-# CHECK-NEXT:      "value": "function foo → void\n\nvoid foo()"
+# CHECK-NEXT:      "value": "function foo\n\n🡺 void\n\nvoid foo()"
 # CHECK-NEXT:    },
 # CHECK-NEXT:    "range": {
 # CHECK-NEXT:      "end": {
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1722,8 +1722,10 @@ template <typename T, typename C = bool> class Foo {})",
             HI.NamespaceScope = "ns::";
             HI.Definition = "ret_type foo(params) {}";
           },
-          R"(function foo → ret_type
+          R"(function foo
 
+🡺 ret_type
+Parameters:
 - 
 - type
 - type foo
@@ -1741,8 +1743,9 @@ ret_type foo(params) {})",
             HI.Type = "type";
             HI.Definition = "def";
           },
-          R"(variable foo : type
+          R"(variable foo
 
+Type: type
 Value = value
 
 // In test::bar
@@ -1763,9 +1766,8 @@ TEST(Hover, PresentHeadings) {
   HoverInfo HI;
   HI.Kind = index::SymbolKind::Variable;
   HI.Name = "foo";
-  HI.Type = "type";
 
-  EXPECT_EQ(HI.present().asMarkdown(), "### variable `foo` \\: `type`");
+  EXPECT_EQ(HI.present().asMarkdown(), "### variable `foo`");
 }
 
 // This is a separate test as rulers behave differently in markdown vs
