[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