[PATCH] D72622: [clangd] Add a ruler after header in hover

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72622

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/test/hover.test
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1746,6 +1746,7 @@
             HI.NamespaceScope.emplace();
           },
           R"(class foo
+
 documentation
 
 template <typename T, typename C = bool> class Foo {})",
@@ -1769,6 +1770,7 @@
             HI.Definition = "ret_type foo(params) {}";
           },
           R"(function foo → ret_type
+
 - 
 - type
 - type foo
@@ -1787,6 +1789,7 @@
             HI.Definition = "def";
           },
           R"(variable foo : type
+
 Value = value
 
 // In test::bar
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -136,12 +136,12 @@
   EXPECT_EQ(D.asPlainText(), ExpectedText);
 }
 
-TEST(Document, Spacer) {
+TEST(Document, Ruler) {
   Document D;
   D.addParagraph().appendText("foo");
-  D.addSpacer();
+  D.addRuler();
   D.addParagraph().appendText("bar");
-  EXPECT_EQ(D.asMarkdown(), "foo  \n\nbar");
+  EXPECT_EQ(D.asMarkdown(), "foo  \n\n---\nbar");
   EXPECT_EQ(D.asPlainText(), "foo\n\nbar");
 }
 
Index: clang-tools-extra/clangd/test/hover.test
===================================================================
--- clang-tools-extra/clangd/test/hover.test
+++ 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 → void\n\n\nvoid foo()"
 # CHECK-NEXT:    },
 # CHECK-NEXT:    "range": {
 # CHECK-NEXT:      "end": {
@@ -37,7 +37,7 @@
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "contents": {
 # CHECK-NEXT:      "kind": "plaintext",
-# CHECK-NEXT:      "value": "enum foo\n\nenum foo {}"
+# CHECK-NEXT:      "value": "enum foo\n\n\nenum foo {}"
 # CHECK-NEXT:    },
 # CHECK-NEXT:    "range": {
 # CHECK-NEXT:      "end": {
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -550,6 +550,8 @@
     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`
Index: clang-tools-extra/clangd/FormattedString.h
===================================================================
--- clang-tools-extra/clangd/FormattedString.h
+++ clang-tools-extra/clangd/FormattedString.h
@@ -81,8 +81,8 @@
 public:
   /// Adds a semantical block that will be separate from others.
   Paragraph &addParagraph();
-  /// Inserts a vertical space into the document.
-  void addSpacer();
+  /// Inserts a horizontal separator to the document.
+  void addRuler();
   /// Adds a block of code. This translates to a ``` block in markdown. In plain
   /// text representation, the code block will be surrounded by newlines.
   void addCodeBlock(std::string Code, std::string Language = "cpp");
Index: clang-tools-extra/clangd/FormattedString.cpp
===================================================================
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -122,10 +122,16 @@
   return llvm::StringRef(OS.str()).trim().str();
 }
 
-// Puts a vertical space between blocks inside a document.
-class Spacer : public Block {
+// Seperates two blocks with extra spacing. Note that it might render strangely
+// in vscode if the trailing block is a codeblock, see
+// https://github.com/microsoft/vscode/issues/88416 for details.
+class Ruler : public Block {
 public:
-  void renderMarkdown(llvm::raw_ostream &OS) const override { OS << '\n'; }
+  void renderMarkdown(llvm::raw_ostream &OS) const override {
+    // Note that we need an extra new line before the ruler, otherwise we might
+    // make previous block a title instead of introducing a ruler.
+    OS << "\n---\n";
+  }
   void renderPlainText(llvm::raw_ostream &OS) const override { OS << '\n'; }
 };
 
@@ -259,7 +265,7 @@
   return *static_cast<Paragraph *>(Children.back().get());
 }
 
-void Document::addSpacer() { Children.push_back(std::make_unique<Spacer>()); }
+void Document::addRuler() { Children.push_back(std::make_unique<Ruler>()); }
 
 void Document::addCodeBlock(std::string Code, std::string Language) {
   Children.emplace_back(

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits