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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 13 10:35:27 PST 2020


sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:553
 
+  // Put a linebreak after header to increase readability.
+  Output.addRuler();
----------------
(When merging with D72623, I'm assuming return type will go below this line?)


================
Comment at: clang-tools-extra/clangd/Hover.cpp:576
     Output.addParagraph().appendText(Documentation);
 
   if (!Definition.empty()) {
----------------
seems to me we'd likely want one above definition too


================
Comment at: clang-tools-extra/clangd/test/hover.test:12
 # 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:    },
----------------
Why are we getting two blank lines here?

(I understand why this patch increases it by one, but don't understand why it was two before. Ideally I think we never want \n\n\n in plaintext)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72622/new/

https://reviews.llvm.org/D72622





More information about the cfe-commits mailing list