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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
