[PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:138
+    // No need to pad from previous blocks, as they should end with a new line.
+    OS << renderCodeBlock(Contents, Language) << '\n';
+  }
----------------
sammccall wrote:
> can you inline renderCodeBlock here, or modify it to just compute the marker?
> It doesn't make sense to have this function just to delegate to one with a different interface (and a copy)
changed it to only return `Marker`


================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:142
+  void renderPlainText(llvm::raw_ostream &OS) const override {
+    // In plaintext we want one empty line before and after codeblocks.
+    OS << '\n' << Contents << "\n\n";
----------------
sammccall wrote:
> This means if we have two consecutive code blocks, they'll be separated by two blank lines...
> 
> worth at least a fixme.
> 
> (I guess the "right thing" here is to have `virtual unsigned verticalMargin() { return 0; }`, and have the document renderer insert max(first.verticalMargin(),second.verticalMargin()) newlines in between). Probably not worth it
adding a testcase and a fixme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71414