[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

172

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;
 };
 
+/// Prints a list of documents while prepending "- " marker. Also indents the
+/// documents.
+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;
 
+  /// Causes every line of the document to be indented by 2 spaces. Except the
+  /// first line, it is containers responsibility to adjust padding for first
+  /// line.
+  void indent() { Indent = true; }
+
 private:
   std::vector<std::unique_ptr<Block>> Children;
+  bool Indent = false;
 };
-
 } // namespace markup
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/FormattedString.cpp

std::string canonicalizeSpaces(std::string Input) {
   }
   // Get rid of extra spaces, -1 is for the trailing space introduced with last
   // word.
   Input.resize(WritePtr - Input.begin() - 1);
   return Input;
 }
 
 std::string renderBlocks(llvm::ArrayRef<std::unique_ptr<Block>> Children,
-                         void (Block::*RenderFunc)(llvm::raw_ostream &) const) {
+                         void (Block::*RenderFunc)(llvm::raw_ostream &) const,
+                         bool Indent) {
   std::string R;
   llvm::raw_string_ostream OS(R);
   for (auto &C : Children)
     ((*C).*RenderFunc)(OS);
-  return llvm::StringRef(OS.str()).trim().str();
+  llvm::StringRef Trimmed(OS.str());
+  Trimmed = Trimmed.trim();
+  if (!Indent)
+    return Trimmed.str();
+
+  // 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.
+  std::string IndentedR;
+  // We'll add 2 spaces after each new line.
+  IndentedR.reserve(Trimmed.size() + Trimmed.count('\n') * 2);
+  for (char C : Trimmed) {
+    IndentedR += C;
+    if (C == '\n')
+      IndentedR.append("  ");
+  }
+  return IndentedR;
 }
 
 // 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'; }
 };

 
 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();
 }
 
 std::string Block::asPlainText() const {
   std::string R;
   llvm::raw_string_ostream OS(R);
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)
+    OS << "- " + 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)
+    OS << "- " + 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();
+  Documents.back().indent();
+  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);
+  return renderBlocks(Children, &Block::renderMarkdown, Indent);
 }
 
 std::string Document::asPlainText() const {
-  return renderBlocks(Children, &Block::renderPlainText);
+  return renderBlocks(Children, &Block::renderPlainText, Indent);
+}
+
+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

TEST(Paragraph, NewLines) {
   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");
+  D.addBulletList().addItem().addParagraph().appendText("item");
+  D.addParagraph().appendText("baz");
   EXPECT_EQ(D.asMarkdown(), R"md(foo
 ```cpp
 test
 ```
-bar)md");
+bar
+- item
+
+baz)md");
   EXPECT_EQ(D.asPlainText(), R"pt(foo
 
 test
 
-bar)pt");
+bar
+- item
+baz)pt");
 }
 
 TEST(Document, Spacer) {
   Document D;
   D.addParagraph().appendText("foo");
   D.addSpacer();
   D.addParagraph().appendText("bar");
   EXPECT_EQ(D.asMarkdown(), "foo\n\nbar");
```)md");
   EXPECT_EQ(D.asPlainText(), R"pt(foo
   bar
   baz
 
 
 foo)pt");
 }
 
+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