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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:121
 std::string renderBlocks(llvm::ArrayRef<std::unique_ptr<Block>> Children,
-                         void (Block::*RenderFunc)(llvm::raw_ostream &) const) {
+                         const RenderType RT) {
   std::string R;
----------------
Having thought about this a bit more:

I think it'd be clearer to apply the ruler-filtering "symbolically", dropping those blocks before attempting rendering. Need isRuler() I guess.
Then something like

```
vector<Block*> FilteredBlocks;
for (auto& C : Children) {
  if (C->isRuler() && (FilteredBlocks.empty() || FilteredBlocks.back()->isRuler()))
    continue;
  FilteredBlocks.push_back(C.get());
}
while (!FilteredBlocks.empty() && FilteredBlocks.back()->isRuler())
  FilteredBlocks.pop_back();
```

Whereas I can't see a good way to apply the plaintext-newline rule symbolically, textually seems reasonable:
```
llvm::erase_if(Text, [&](Char &C) { return StringRef(Text.data(), &C - Text.data()).endswith("\n\n"); });
```


================
Comment at: clang-tools-extra/clangd/FormattedString.h:36
 
+  /// Padding information to imitate spacing while rendering for plaintext. As
+  /// markdown renderers should already add appropriate padding between
----------------
This is a bit weird layering-wise. I think it'd be cleaner just to do a substitution on the rendered output.
Still a hack, but we've got that either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72622