[clangd] Introduce codeblocks

[kadircet]

Summary

Follow-up to the patch D71248

[merge_guards_bot]

clang-format: pass.

Build artifacts: console-log.txt, diff.json

[sammccall]

clang-tools-extra/clangd/FormattedString.cpp

137

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)

141

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

[sammccall]

Otherwise LG

[kadircet]

• Address comments

clang-tools-extra/clangd/FormattedString.cpp

137

changed it to only return Marker

141

adding a testcase and a fixme

[merge_guards_bot]

Unit tests: pass. 60768 tests passed, 0 failed and 726 were skipped.

clang-format: pass.

Build artifacts: console-log.txt, CMakeCache.txt, test-results.xml, diff.json

[thakis]

clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

126

Older (but still supported) gccs can't handle multiline raw strings in macro arguments, see e.g. http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio

I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days, please watch bots and your inbox after committing.

clang-tools-extra/clangd/FormattedString.h

 /// A format-agnostic representation for structured text. Allows rendering into
 /// markdown and plaintext.
 class Document {
 public:
   /// Adds a semantical block that will be separate from others.
   Paragraph &addParagraph();
   /// Inserts a vertical space into the document.
   void addSpacer();
+  /// Adds a block of code. This translates to a ``` block in markdown. In plain
+  /// text representation, the code block will be surrounded by newlines.
+  void addCodeBlock(std::string Code, std::string Language = "cpp");
 
   std::string asMarkdown() const;
   std::string asPlainText() const;
 
 private:
   std::vector<std::unique_ptr<Block>> Children;
 };
 
 } // namespace markup
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/FormattedString.cpp

std::string renderInlineBlock(llvm::StringRef Input) {
   // Markdown render should ignore first and last space if both are there. We
   // add an extra pair of spaces in that case to make sure we render what the
   // user intended.
   if (llvm::StringRef(R).startswith(" ") && llvm::StringRef(R).endswith(" "))
     return "` " + std::move(R) + " `";
   return "`" + std::move(R) + "`";
 }
 
-/// Render \p Input as markdown code block with a specified \p Language. The
-/// result is surrounded by >= 3 backticks. Although markdown also allows to use
-/// '~' for code blocks, they are never used.
-std::string renderCodeBlock(llvm::StringRef Input, llvm::StringRef Language) {
+/// Get marker required for \p Input to represent a markdown codeblock. It
+/// consists of at least 3 backticks(`). Although markdown also allows to use
+/// tilde(~) for code blocks, they are never used.
+std::string getMarkerForCodeBlock(llvm::StringRef Input) {
   // Count the maximum number of consecutive backticks in \p Input. We need to
   // start and end the code block with more.
   unsigned MaxBackticks = 0;
   unsigned Backticks = 0;
   for (char C : Input) {
     if (C == '`') {
       ++Backticks;
       continue;
     }
     MaxBackticks = std::max(MaxBackticks, Backticks);
     Backticks = 0;
   }
   MaxBackticks = std::max(Backticks, MaxBackticks);
   // Use the corresponding number of backticks to start and end a code block.
-  std::string BlockMarker(/*Repeat=*/std::max(3u, MaxBackticks + 1), '`');
-  return BlockMarker + Language.str() + "\n" + Input.str() + "\n" + BlockMarker;
+  return std::string(/*Repeat=*/std::max(3u, MaxBackticks + 1), '`');
 }
 
 // Trims the input and concatanates whitespace blocks into a single ` `.
 std::string canonicalizeSpaces(std::string Input) {
   // Goes over the string and preserves only a single ` ` for any whitespace
   // chunks, the rest is moved to the end of the string and dropped in the end.
   auto WritePtr = Input.begin();
   llvm::SmallVector<llvm::StringRef, 4> Words;

 
 // Puts a vertical space between blocks inside a document.
 class Spacer : public Block {
 public:
   void renderMarkdown(llvm::raw_ostream &OS) const override { OS << '\n'; }
   void renderPlainText(llvm::raw_ostream &OS) const override { OS << '\n'; }
 };
 
+class CodeBlock : public Block {
+public:
+  void renderMarkdown(llvm::raw_ostream &OS) const override {
+    std::string Marker = getMarkerForCodeBlock(Contents);
+    // No need to pad from previous blocks, as they should end with a new line.
+    OS << Marker << Language << '\n' << Contents << '\n' << Marker << '\n';
+  }
+
+  void renderPlainText(llvm::raw_ostream &OS) const override {
+    // In plaintext we want one empty line before and after codeblocks.
+    OS << '\n' << Contents << "\n\n";
+  }
+
+  CodeBlock(std::string Contents, std::string Language)
+      : Contents(std::move(Contents)), Language(std::move(Language)) {}
+
+private:
+  std::string Contents;
+  std::string Language;
+};
 } // namespace
 
 std::string Block::asMarkdown() const {
   std::string R;
   llvm::raw_string_ostream OS(R);
   renderMarkdown(OS);
   return llvm::StringRef(OS.str()).trim().str();
 }

 
 Paragraph &Document::addParagraph() {
   Children.push_back(std::make_unique<Paragraph>());
   return *static_cast<Paragraph *>(Children.back().get());
 }
 
 void Document::addSpacer() { Children.push_back(std::make_unique<Spacer>()); }
 
+void Document::addCodeBlock(std::string Code, std::string Language) {
+  Children.emplace_back(
+      std::make_unique<CodeBlock>(std::move(Code), std::move(Language)));
+}
+
 std::string Document::asMarkdown() const {
   return renderBlocks(Children, &Block::renderMarkdown);
 }
 
 std::string Document::asPlainText() const {
   return renderBlocks(Children, &Block::renderPlainText);
 }
 } // namespace markup
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/Hover.cpp

std::string printDefinition(const Decl *D) {
   Policy.IncludeTagDefinition = false;
   Policy.SuppressTemplateArgsInCXXConstructors = true;
   D->print(OS, Policy);
   OS.flush();
   return Definition;
 }
 
 void printParams(llvm::raw_ostream &OS,
-                        const std::vector<HoverInfo::Param> &Params) {
   for (size_t I = 0, E = Params.size(); I != E; ++I) {
     if (I)
       OS << ", ";
     OS << Params.at(I);
   }
 }
 
 std::vector<HoverInfo::Param>
if (NamespaceScope) {
     if (!LocalScope.empty())
       P.appendCode(llvm::StringRef(LocalScope).drop_back(2));
     else if (NamespaceScope->empty())
       P.appendCode("global namespace");
     else
       P.appendCode(llvm::StringRef(*NamespaceScope).drop_back(2));
   }
 
-  Output.addSpacer();
   if (!Definition.empty()) {
-    Output.addParagraph().appendCode(Definition);
+    Output.addCodeBlock(Definition);
   } else {
     // Builtin types
-    Output.addParagraph().appendCode(Name);
+    Output.addCodeBlock(Name);
   }
 
   if (!Documentation.empty())
     Output.addParagraph().appendText(Documentation);
   return Output;
 }
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,

clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

TEST(Render, Escaping) {
   P.appendCode("`foo");
   EXPECT_EQ(P.asMarkdown(), "` ``foo `");
   P = Paragraph();
   P.appendCode("foo`");
   EXPECT_EQ(P.asMarkdown(), "` foo`` `");
   P = Paragraph();
   P.appendCode("`foo`");
   EXPECT_EQ(P.asMarkdown(), "` ``foo`` `");
+
+  // Code blocks might need more than 3 backticks.
+  Document D;
+  D.addCodeBlock("foobarbaz `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "```cpp\n"
+                            "foobarbaz `\nqux\n"
+                            "```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ``\nqux");
+  EXPECT_THAT(D.asMarkdown(), "```cpp\n"
+                              "foobarbaz ``\nqux\n"
+                              "```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ```\nqux");
+  EXPECT_EQ(D.asMarkdown(), "````cpp\n"
+                            "foobarbaz ```\nqux\n"
+                            "````");
+  D = Document();
+  D.addCodeBlock("foobarbaz ` `` ``` ```` `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "`````cpp\n"
+                            "foobarbaz ` `` ``` ```` `\nqux\n"
+                            "`````");
 }
 
 TEST(Paragraph, SeparationOfChunks) {
   // This test keeps appending contents to a single Paragraph and checks
   // expected accumulated contents after each one.
   // Purpose is to check for separation between different chunks.
   Paragraph P;
 
TEST(Paragraph, NewLines) {
   P.appendCode(" \n foo\nbar \n ");
   EXPECT_EQ(P.asMarkdown(), R"md(foo bar `foo bar`)md");
   EXPECT_EQ(P.asPlainText(), R"pt(foo bar foo bar)pt");
 }
 
 TEST(Document, Separators) {
   Document D;
   D.addParagraph().appendText("foo");
+  D.addCodeBlock("test");
   D.addParagraph().appendText("bar");
-  EXPECT_EQ(D.asMarkdown(), "foo\nbar");
-  EXPECT_EQ(D.asPlainText(), "foo\nbar");
+  EXPECT_EQ(D.asMarkdown(), R"md(foo
+```cpp
+test
+```
+bar)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+
+test
+
+bar)pt");
 }
 
 TEST(Document, Spacer) {
   Document D;
   D.addParagraph().appendText("foo");
   D.addSpacer();
   D.addParagraph().appendText("bar");
   EXPECT_EQ(D.asMarkdown(), "foo\n\nbar");
   EXPECT_EQ(D.asPlainText(), "foo\n\nbar");
 }
 
+TEST(CodeBlock, Render) {
+  Document D;
+  // Code blocks preserves any extra spaces.
+  D.addCodeBlock("foo\n  bar\n  baz");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz)pt");
+  D.addCodeBlock("foo");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```
+```cpp
+foo
+```)md");
+  // FIXME: we shouldn't have 2 empty lines in between. A solution might be
+  // having a `verticalMargin` method for blocks, and let container insert new
+  // lines according to that before/after blocks.
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz
+
+
+foo)pt");
+}
+
 } // namespace
 } // namespace markup
 } // namespace clangd
 } // namespace clang