[clangd] Introduce bulletlists

[merge_guards_bot]

clang-format: pass.

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

[sammccall]

clang-tools-extra/clangd/FormattedString.h

65

This is a class, not a function - comment should focus on what it *is*.

95

Why is it the responsibility of this class to keep track of its indentation within the parent?
This seems like it should be a parameter to the render functions, rather than state.

BTW, it appears to be legal to write lists as:

-
  this is the
  first item
-
  second item

Which is much easier/more regular to generate, because it doesn't require Document to special-case the first line.

[kadircet]

• Move indentation logic into bulletlist.

clang-tools-extra/clangd/FormattedString.h

95

moved indentation logic into bullet list.

but that wouldn't look nice when rendering plain text? also I wouldn't call that special casing, as it just generates trimmed output, like rest of the APIs

[merge_guards_bot]

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

clang-format: pass.

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

[sammccall]

clang-tools-extra/clangd/FormattedString.cpp

156

assert that there's no trailing newlines?

clang-tools-extra/clangd/FormattedString.h

75

"items" rather than "documents" would capture the semantics better I think

92

can we document that it *doesn't* render its own trailing newlines? This is a divergence from Block and more important to the implementation now

clang-tools-extra/clangd/FormattedString.h

struct Chunk {
     } Kind = PlainText;
     std::string Contents;
     /// Language for code block chunks. Ignored for other chunks.
     std::string Language;
   };
   std::vector<Chunk> Chunks;
 };
 
+/// Represents a sequence of one or more documents. Knows how to print them in a
+/// list like format, e.g. by prepending with "- " and indentation.
+class BulletList : public Block {
+public:
+  void renderMarkdown(llvm::raw_ostream &OS) const override;
+  void renderPlainText(llvm::raw_ostream &OS) const override;
+
+  class Document &addItem();
+
+private:
+  std::vector<class Document> Documents;
+};
+
 /// 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");
 
+  BulletList &addBulletList();
+
   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

public:
 
   CodeBlock(std::string Contents, std::string Language)
       : Contents(std::move(Contents)), Language(std::move(Language)) {}
 
 private:
   std::string Contents;
   std::string Language;
 };
+
+// Inserts two spaces after each `\n` to indent each line. First line is not
+// indented.
+std::string indentLines(llvm::StringRef Input) {
+  std::string IndentedR;
+  // We'll add 2 spaces after each new line.
+  IndentedR.reserve(Input.size() + Input.count('\n') * 2);
+  for (char C : Input) {
+    IndentedR += C;
+    if (C == '\n')
+      IndentedR.append("  ");
+  }
+  return IndentedR;
+}
 } // namespace
 
 std::string Block::asMarkdown() const {
   std::string R;
   llvm::raw_string_ostream OS(R);
   renderMarkdown(OS);
   return llvm::StringRef(OS.str()).trim().str();
 }
void Paragraph::renderPlainText(llvm::raw_ostream &OS) const {
   llvm::StringRef Sep = "";
   for (auto &C : Chunks) {
     OS << Sep << C.Contents;
     Sep = " ";
   }
   OS << '\n';
 }
 
+void BulletList::renderMarkdown(llvm::raw_ostream &OS) const {
+  for (auto &D : Documents) {
+    // Instead of doing this we might prefer passing Indent to children to get
+    // rid of the copies, if it turns out to be a bottleneck.
+    OS << "- " << indentLines(D.asMarkdown()) << '\n';
+  }
+  // We need a new line after list to terminate it in markdown.
+  OS << '\n';
+}
+
+void BulletList::renderPlainText(llvm::raw_ostream &OS) const {
+  for (auto &D : Documents) {
+    // Instead of doing this we might prefer passing Indent to children to get
+    // rid of the copies, if it turns out to be a bottleneck.
+    OS << "- " << indentLines(D.asPlainText()) << '\n';
+  }
+}
+
 Paragraph &Paragraph::appendText(std::string Text) {
   Text = canonicalizeSpaces(std::move(Text));
   if (Text.empty())
     return *this;
   Chunks.emplace_back();
   Chunk &C = Chunks.back();
   C.Contents = std::move(Text);
   C.Kind = Chunk::PlainText;
   return *this;
 }
 
 Paragraph &Paragraph::appendCode(std::string Code) {
   Code = canonicalizeSpaces(std::move(Code));
   if (Code.empty())
     return *this;
   Chunks.emplace_back();
   Chunk &C = Chunks.back();
   C.Contents = std::move(Code);
   C.Kind = Chunk::InlineCode;
   return *this;
 }
 
+class Document &BulletList::addItem() {
+  Documents.emplace_back();
+  return Documents.back();
+}
+
 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);
 }
+
+BulletList &Document::addBulletList() {
+  Children.emplace_back(std::make_unique<BulletList>());
+  return *static_cast<BulletList *>(Children.back().get());
+}
 } // namespace markup
 } // namespace clangd
 } // namespace clang

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

```)md";
   bar
   baz
 
 
 foo)pt";
   EXPECT_EQ(D.asPlainText(), ExpectedPlainText);
 }
 
+TEST(BulletList, Render) {
+  BulletList L;
+  // Flat list
+  L.addItem().addParagraph().appendText("foo");
+  EXPECT_EQ(L.asMarkdown(), "- foo");
+  EXPECT_EQ(L.asPlainText(), "- foo");
+
+  L.addItem().addParagraph().appendText("bar");
+  EXPECT_EQ(L.asMarkdown(), R"md(- foo
+- bar)md");
+  EXPECT_EQ(L.asPlainText(), R"pt(- foo
+- bar)pt");
+
+  // Nested list, with a single item.
+  Document &D = L.addItem();
+  // First item with foo\nbaz
+  D.addParagraph().appendText("foo");
+  D.addParagraph().appendText("baz");
+
+  // Nest one level.
+  Document &Inner = D.addBulletList().addItem();
+  Inner.addParagraph().appendText("foo");
+
+  // Nest one more level.
+  BulletList &InnerList = Inner.addBulletList();
+  // Single item, baz\nbaz
+  Document &DeepDoc = InnerList.addItem();
+  DeepDoc.addParagraph().appendText("baz");
+  DeepDoc.addParagraph().appendText("baz");
+  EXPECT_EQ(L.asMarkdown(), R"md(- foo
+- bar
+- foo
+  baz
+  - foo
+    - baz
+      baz)md");
+  EXPECT_EQ(L.asPlainText(), R"pt(- foo
+- bar
+- foo
+  baz
+  - foo
+    - baz
+      baz)pt");
+
+  // Termination
+  Inner.addParagraph().appendText("after");
+  EXPECT_EQ(L.asMarkdown(), R"md(- foo
+- bar
+- foo
+  baz
+  - foo
+    - baz
+      baz
+    
+    after)md");
+  EXPECT_EQ(L.asPlainText(), R"pt(- foo
+- bar
+- foo
+  baz
+  - foo
+    - baz
+      baz
+    after)pt");
+}
+
 } // namespace
 } // namespace markup
 } // namespace clangd
 } // namespace clang