[clangd] Introduce paragraph, the first part of new rendering structs

[kadircet]

Summary

Initial patch for new rendering structs in clangd.

Splitting implementation into smaller chunks, for a full view of the API see D71063.

[merge_guards_bot]

Build result: pass - 60655 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[kadircet]

• More comments

[merge_guards_bot]

Build result: pass - 60655 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[sammccall]

Main thing here is I'm fairly sure that code blocks shouldn't be chunks in a paragraph (or I'm misunderstanding what a paragraph is)

clang-tools-extra/clangd/FormattedString.cpp

98

is this equivalent to

SmallVector<StringRef> Words;
llvm::SplitString(Input, Words);
return llvm::join(Input, " ");

?

(That would trim leading/trailing whitespace, but I suspect we actually want that)

160–161

Can we simplify the whitespace model? A lot of this is hacks around code blocks, which shouldn't really be here.

What about:

• the contents of each chunk never have leading/trailing ws (it's stripped when adding)
• there's always a space between consecutive chunks

clang-tools-extra/clangd/FormattedString.h

24–25

I don't think this comment captures what this class is about now. (Rewriting the class but leaving the comment intact is suspicious!)

Comment should allude to how blocks make up a document - a RenderableBlock holds rich text and knows how to lay it out. Blocks can be stacked to form a document, etc.

25–27

nit: I wouldn't bother with both "renderable" prefix and the markup namespace, up to you

29

Or just asMarkdown, up to you

29

are you sure we want to create all the temporary strings?
Consider void renderMarkdown(llvm::raw_ostream&), and keep std::string asMarkdown() on Document

31

renderfortests has gone. What's the plan for testing Hover rendering? (i.e. HoverInfo -> Document)
I guess asserting the plain text and markdown might be enough...

(Did we remove *all* the tests for present()in D70911, and I didn't notice? We should restore some based on synthetic HoverInfo)

36

A code block doesn't go in a paragraph, as it's a block itself.

"A paragraph contains a logical line of rich text. It may be wrapped for display"

43–44

nit: while here, I do think inline block is misleading - this corresponds to display: inline, not display:inline-block. "inline span"?

43–44

just appendCode?

62

semantic -> format-agnostic? This doesn't capture semantics.

formattable strings -> rich text, or structured text?

[kadircet]

• Move code blocks out of Paragraph
• Provide a way to control vertical spacing
• Address comments around naming

[kadircet]

clang-tools-extra/clangd/FormattedString.cpp

98

yes that's exactly what we want, thanks!
not just joining them though, since we could still merge them in-place.

clang-tools-extra/clangd/FormattedString.h

31

yes we I was planning to check using the plaintext output with some "synthetic" hoverinfo. Currently only lit tests are checking for the behavior of present.
there's also changes to hover.cpp in this patch to preserve current output. happy to add some tests before moving forward, but didn't want to add any as they
will change drastically with the new rendering layout.

36

sending out codeblocks in a new patch

[merge_guards_bot]

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

clang-format: pass.

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

[kadircet]

• More tests

[merge_guards_bot]

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

clang-format: pass.

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

[sammccall]

Design question around newlines :-) Otherwise looks good.

clang-tools-extra/clangd/FormattedString.cpp

98

I'm really not sure saving the allocation is worth the code here, but up to you.

118

(why not just pass in the fptr?)

122

I'm not sure this common codepath for plain-text and markdown will survive in the long run:

• state around indentation and wrapping is likely to differ
• separating blocks with one newline may not be the right thing in each case

130

for markdown, this means consecutive paragraphs will be rendered as a single paragraph with line breaks in it. intended?

139

this deserves a comment - the effect is produced by the surrounding newlines.
But if we change the blocks to include their whitespace it'll be more obvious.

clang-tools-extra/clangd/FormattedString.h

29

Do these include trailing newline(s), or not?

Based on offline discussion, they currently do not, but this causes some problems e.g. list followed by paragraph would get merged.

Option a) have Blocks include required trailing newlines (e.g. \n\n after lists in markdown, \n after paragraphs), and have Document trim trailing newlines.
Option b) have Document look at the type of the elements above/below and insert space appropriately.

Personally prefer option A because it encapsulates behavior better in the classes (less dyn_cast) and I think reflects the grammar better.

clang-tools-extra/clangd/Hover.cpp

460

This is a regression at the moment - complicated definitions are clang-formatted, and we'll canonicalize the whitespace.
(The behavior of the library is correct, we just need code block for this).

[kadircet]

clang-tools-extra/clangd/FormattedString.cpp

122

I suppose this will survive in the long run now, as it only prints blocks, without separation, and trims in the end.

130

yes this was intended, but as each block takes care of its own indentation now, this is no longer an issue.

clang-tools-extra/clangd/FormattedString.h

29

taking option A

clang-tools-extra/clangd/Hover.cpp

460

not planning to land until we've got the complete implementation, including lists and codeblocks.

[kadircet]

• Move separation logic from container to blocks
• Add two concrete APIs to block for getting strings and update tests to use those.
• Rename methods

[merge_guards_bot]

Unit tests: fail. 60762 tests passed, 1 failed and 726 were skipped.

failed: Clang.CodeGenCXX/runtime-dllstorage.cpp

clang-format: pass.

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

[sammccall]

clang-tools-extra/clangd/FormattedString.cpp

158–159

this is worth a comment - we translate Paragraphs into markdown lines, not markdown paragraphs

192

push_back (and below)

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

20–21

dead?

95

I'd consider writing this as
[&] { Para.appendText("after"); } to make it clearer what's being tested.

Are you sure the table test is clearer than straight-line code incrementally building the paragrap/asserting here?

[kadircet]

• Address comments

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

95

this test was huge in the beginning and having a straight line of appends were making it really hard to reason with. but it seems leaner now, as it doesn't have codeblocks anymore.
switching back to linear testing.

[merge_guards_bot]

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

clang-format: pass.

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

[aganea]

clang-tools-extra/clangd/FormattedString.cpp

109

There's an issue at this line, the iterator might go past the end of the string (if running the Document.Separators test). This fails when running a Debug build with VS 2019. The code fails with this assert (in xstring):

_STL_VERIFY(_Unfancy(_Ptr) < _Mycont->_Myptr() + _Mycont->_Mysize, "cannot increment string iterator past end");

I changed the code such as:

clangd.patch2 KBDownload

(sorry, Phabricator throws an exception when uploading this patch as a differential)

I can commit it if you're fine with the change.

[kadircet]

clang-tools-extra/clangd/FormattedString.cpp

109

thanks for reporting this!

sent out rG3346cecd4c0c960377b441606b6382a684daf061

clang-tools-extra/clangd/ClangdLSPServer.cpp

Server->findHover(Params.textDocument.uri.file(), Params.position,
                       if (!*H)
                         return Reply(llvm::None);
 
                       Hover R;
                       R.contents.kind = HoverContentFormat;
                       R.range = (*H)->SymRange;
                       switch (HoverContentFormat) {
                       case MarkupKind::PlainText:
-                        R.contents.value = (*H)->present().renderAsPlainText();
+                        R.contents.value = (*H)->present().asPlainText();
                         return Reply(std::move(R));
                       case MarkupKind::Markdown:
-                        R.contents.value = (*H)->present().renderAsMarkdown();
+                        R.contents.value = (*H)->present().asMarkdown();
                         return Reply(std::move(R));
                       };
                       llvm_unreachable("unhandled MarkupKind");
                     });
 }
 
 void ClangdLSPServer::onTypeHierarchy(
     const TypeHierarchyParams &Params,

clang-tools-extra/clangd/FormattedString.h

 //===--- FormattedString.h ----------------------------------*- C++-*------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // A simple intermediate representation of formatted text that could be
 // converted to plaintext or markdown.
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FORMATTEDSTRING_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FORMATTEDSTRING_H
 
+#include "llvm/Support/raw_ostream.h"
+#include <memory>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
+namespace markup {
 
-/// A structured string representation that could be converted to markdown or
-/// plaintext upon requrest.
-class FormattedString {
+/// Holds text and knows how to lay it out. Multiple blocks can be grouped to
+/// form a document.
+class Block {
 public:
+  virtual void asMarkdown(llvm::raw_ostream &OS) const = 0;
+  virtual void asPlainText(llvm::raw_ostream &OS) const = 0;
+
+  virtual ~Block() = default;
+};
+
+/// Represents parts of the markup that can contain strings, like inline code,
+/// code block or plain text.
+/// One must introduce different paragraphs to create separate blocks.
+class Paragraph : public Block {
+public:
+  void asMarkdown(llvm::raw_ostream &OS) const override;
+  void asPlainText(llvm::raw_ostream &OS) const override;
+
   /// Append plain text to the end of the string.
-  void appendText(std::string Text);
-  /// Append a block of C++ code. This translates to a ``` block in markdown.
-  /// In a plain text representation, the code block will be surrounded by
-  /// newlines.
-  void appendCodeBlock(std::string Code, std::string Language = "cpp");
-  /// Append an inline block of C++ code. This translates to the ` block in
-  /// markdown.
-  void appendInlineCode(std::string Code);
-
-  std::string renderAsMarkdown() const;
-  std::string renderAsPlainText() const;
-  std::string renderForTests() const;
+  Paragraph &appendText(std::string Text);
+
+  /// Append inline code, this translates to the ` block in markdown.
+  Paragraph &appendCode(std::string Code);
 
 private:
-  enum class ChunkKind {
-    PlainText,       /// A plain text paragraph.
-    CodeBlock,       /// A block of code.
-    InlineCodeBlock, /// An inline block of code.
-  };
   struct Chunk {
-    ChunkKind Kind = ChunkKind::PlainText;
+    enum {
+      PlainText,
+      InlineCode,
+    } Kind = PlainText;
     std::string Contents;
     /// Language for code block chunks. Ignored for other chunks.
     std::string Language;
   };
   std::vector<Chunk> Chunks;
 };
 
+/// 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();
+
+  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

 //===--- FormattedString.cpp --------------------------------*- C++-*------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "FormattedString.h"
 #include "clang/Basic/CharInfo.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/raw_ostream.h"
 #include <cstddef>
+#include <memory>
 #include <string>
+#include <vector>
 
 namespace clang {
 namespace clangd {
+namespace markup {
 
 namespace {
 /// Escape a markdown text block. Ensures the punctuation will not introduce
 /// any of the markdown constructs.
-static std::string renderText(llvm::StringRef Input) {
+std::string renderText(llvm::StringRef Input) {
   // Escaping ASCII punctiation ensures we can't start a markdown construct.
   constexpr llvm::StringLiteral Punctuation =
       R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
 
   std::string R;
   for (size_t From = 0; From < Input.size();) {
     size_t Next = Input.find_first_of(Punctuation, From);
     R += Input.substr(From, Next - From);
     if (Next == llvm::StringRef::npos)
       break;
     R += "\\";
     R += Input[Next];
 
     From = Next + 1;
   }
   return R;
 }
 
 /// Renders \p Input as an inline block of code in markdown. The returned value
 /// is surrounded by backticks and the inner contents are properly escaped.
-static std::string renderInlineBlock(llvm::StringRef Input) {
+std::string renderInlineBlock(llvm::StringRef Input) {
   std::string R;
   // Double all backticks to make sure we don't close the inline block early.
   for (size_t From = 0; From < Input.size();) {
     size_t Next = Input.find("`", From);
     R += Input.substr(From, Next - From);
     if (Next == llvm::StringRef::npos)
       break;
     R += "``"; // double the found backtick.
 
     From = Next + 1;
   }
   // If results starts with a backtick, add spaces on both sides. The spaces
   // are ignored by markdown renderers.
   if (llvm::StringRef(R).startswith("`") || llvm::StringRef(R).endswith("`"))
     return "` " + std::move(R) + " `";
   // 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.
-static std::string renderCodeBlock(llvm::StringRef Input,
-                                   llvm::StringRef Language) {
+std::string renderCodeBlock(llvm::StringRef Input, llvm::StringRef Language) {
   // 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;
 }
 
-} // namespace
-
-void FormattedString::appendText(std::string Text) {
-  Chunk C;
-  C.Kind = ChunkKind::PlainText;
-  C.Contents = Text;
-  Chunks.push_back(C);
+// 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;
+  llvm::SplitString(Input, Words);
+  if (Words.empty())
+    return "";
+  // Go over each word and and add it to the string.
+  for (llvm::StringRef Word : Words) {
+    llvm::for_each(Word, [&WritePtr](const char C) { *WritePtr++ = C; });
+    // Separate from next block.
+    *WritePtr++ = ' ';
+  }
+  // Get rid of extra spaces, -1 is for the trailing space introduced with last
+  // word.
+  Input.resize(WritePtr - Input.begin() - 1);
+  return Input;
+}
+
+enum RenderType {
+  PlainText,
+  Markdown,
+};
+std::string renderBlocks(llvm::ArrayRef<std::unique_ptr<Block>> Children,
+                         RenderType RT) {
+  llvm::StringRef Sep = "";
+  std::string R;
+  llvm::raw_string_ostream OS(R);
+  auto RenderFunc = RT == PlainText ? &Block::asPlainText : &Block::asMarkdown;
+  for (auto &C : Children) {
+    OS << Sep;
+    Sep = "\n";
+    ((*C).*RenderFunc)(OS);
+  }
+  return OS.str();
 }
 
-void FormattedString::appendCodeBlock(std::string Code, std::string Language) {
-  Chunk C;
-  C.Kind = ChunkKind::CodeBlock;
-  C.Contents = std::move(Code);
-  C.Language = std::move(Language);
-  Chunks.push_back(std::move(C));
-}
+// Puts a vertical space between blocks inside a document.
+class Spacer : public Block {
+public:
+  void asMarkdown(llvm::raw_ostream &OS) const override {}
+  void asPlainText(llvm::raw_ostream &OS) const override {}
+};
 
-void FormattedString::appendInlineCode(std::string Code) {
-  Chunk C;
-  C.Kind = ChunkKind::InlineCodeBlock;
-  C.Contents = std::move(Code);
-  Chunks.push_back(std::move(C));
-}
+} // namespace
 
-std::string FormattedString::renderAsMarkdown() const {
-  std::string R;
-  auto EnsureWhitespace = [&R]() {
-    // Adds a space for nicer rendering.
-    if (!R.empty() && !isWhitespace(R.back()))
-      R += " ";
-  };
-  for (const auto &C : Chunks) {
+void Paragraph::asMarkdown(llvm::raw_ostream &OS) const {
+  llvm::StringRef Sep = "";
+  for (auto &C : Chunks) {
+    OS << Sep;
     switch (C.Kind) {
-    case ChunkKind::PlainText:
-      if (!C.Contents.empty() && !isWhitespace(C.Contents.front()))
-        EnsureWhitespace();
-      R += renderText(C.Contents);
-      continue;
-    case ChunkKind::InlineCodeBlock:
-      EnsureWhitespace();
-      R += renderInlineBlock(C.Contents);
-      continue;
-    case ChunkKind::CodeBlock:
-      if (!R.empty() && !llvm::StringRef(R).endswith("\n"))
-        R += "\n";
-      R += renderCodeBlock(C.Contents, C.Language);
-      R += "\n";
-      continue;
+    case Chunk::PlainText:
+      OS << renderText(C.Contents);
+      break;
+    case Chunk::InlineCode:
+      OS << renderInlineBlock(C.Contents);
+      break;
     }
-    llvm_unreachable("unhanlded ChunkKind");
+    Sep = " ";
   }
-  return R;
 }
 
-std::string FormattedString::renderAsPlainText() const {
-  std::string R;
-  auto EnsureWhitespace = [&]() {
-    if (R.empty() || isWhitespace(R.back()))
-      return;
-    R += " ";
-  };
-  Optional<bool> LastWasBlock;
-  for (const auto &C : Chunks) {
-    bool IsBlock = C.Kind == ChunkKind::CodeBlock;
-    if (LastWasBlock.hasValue() && (IsBlock || *LastWasBlock))
-      R += "\n\n";
-    LastWasBlock = IsBlock;
+void Paragraph::asPlainText(llvm::raw_ostream &OS) const {
+  llvm::StringRef Sep = "";
+  for (auto &C : Chunks) {
+    OS << Sep << C.Contents;
+    Sep = " ";
+  }
+}
 
-    switch (C.Kind) {
-    case ChunkKind::PlainText:
-      EnsureWhitespace();
-      R += C.Contents;
-      break;
-    case ChunkKind::InlineCodeBlock:
-      EnsureWhitespace();
-      R += C.Contents;
-      break;
-    case ChunkKind::CodeBlock:
-      R += C.Contents;
-      break;
-    }
-    // Trim trailing whitespace in chunk.
-    while (!R.empty() && isWhitespace(R.back()))
-      R.pop_back();
-  }
-  return R;
+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;
+
+Paragraph &Document::addParagraph() {
+  Children.emplace_back(std::make_unique<Paragraph>());
+  return *static_cast<Paragraph *>(Children.back().get());
 }
 
-std::string FormattedString::renderForTests() const {
-  std::string R;
-  for (const auto &C : Chunks) {
-    switch (C.Kind) {
-    case ChunkKind::PlainText:
-      R += "text[" + C.Contents + "]";
-      break;
-    case ChunkKind::InlineCodeBlock:
-      R += "code[" + C.Contents + "]";
-      break;
-    case ChunkKind::CodeBlock:
-      if (!R.empty())
-        R += "\n";
-      R += llvm::formatv("codeblock({0}) [\n{1}\n]\n", C.Language, C.Contents);
-      break;
-    }
-  }
-  while (!R.empty() && isWhitespace(R.back()))
-    R.pop_back();
-  return R;
+void Document::addSpacer() {
+  Children.emplace_back(std::make_unique<Spacer>());
+
+std::string Document::asMarkdown() const {
+  return renderBlocks(Children, RenderType::Markdown);
+
+std::string Document::asPlainText() const {
+  return renderBlocks(Children, RenderType::PlainText);
 }
+} // namespace markup
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/Hover.h

struct HoverInfo {
   /// Set for functions, lambdas and macros with parameters.
   llvm::Optional<std::vector<Param>> Parameters;
   /// Set for all templates(function, class, variable).
   llvm::Optional<std::vector<Param>> TemplateParameters;
   /// Contains the evaluated value of the symbol if available.
   llvm::Optional<std::string> Value;
 
   /// Produce a user-readable information.
-  FormattedString present() const;
+  markup::Document present() const;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const HoverInfo::Param &);
 inline bool operator==(const HoverInfo::Param &LHS,
                        const HoverInfo::Param &RHS) {
   return std::tie(LHS.Type, LHS.Name, LHS.Default) ==
          std::tie(RHS.Type, RHS.Name, RHS.Default);
 }
 

clang-tools-extra/clangd/Hover.cpp

 //===--- Hover.cpp - Information about code at the cursor location --------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Hover.h"
 
 #include "AST.h"
 #include "CodeCompletionStrings.h"
 #include "FindTarget.h"
+#include "FormattedString.h"
 #include "Logger.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/DeclTemplate.h"
if (auto Formatted =
           tooling::applyAllReplacements(HI->Definition, Replacements))
     HI->Definition = *Formatted;
 
   HI->SymRange = getTokenRange(AST.getSourceManager(),
                                AST.getLangOpts(), SourceLocationBeg);
   return HI;
 }
 
-FormattedString HoverInfo::present() const {
-  FormattedString Output;
+markup::Document HoverInfo::present() const {
+  markup::Document Output;
   if (NamespaceScope) {
-    Output.appendText("Declared in");
+    auto &P = Output.addParagraph();
+    P.appendText("Declared in");
     // Drop trailing "::".
     if (!LocalScope.empty())
-      Output.appendInlineCode(llvm::StringRef(LocalScope).drop_back(2));
+      P.appendCode(llvm::StringRef(LocalScope).drop_back(2));
     else if (NamespaceScope->empty())
-      Output.appendInlineCode("global namespace");
+      P.appendCode("global namespace");
     else
-      Output.appendInlineCode(llvm::StringRef(*NamespaceScope).drop_back(2));
+      P.appendCode(llvm::StringRef(*NamespaceScope).drop_back(2));
   }
 
+  Output.addSpacer();
   if (!Definition.empty()) {
-    Output.appendCodeBlock(Definition);
+    Output.addParagraph().appendCode(Definition);
   } else {
     // Builtin types
-    Output.appendCodeBlock(Name);
+    Output.addParagraph().appendCode(Name);
   }
 
   if (!Documentation.empty())
-    Output.appendText(Documentation);
+    Output.addParagraph().appendText(Documentation);
   return Output;
 }
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
                               const HoverInfo::Param &P) {
   std::vector<llvm::StringRef> Output;
   if (P.Type)
     Output.push_back(*P.Type);

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

 //===-- FormattedStringTests.cpp ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "FormattedString.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
+namespace markup {
 namespace {
 
-TEST(FormattedString, Basic) {
-  FormattedString S;
-  EXPECT_EQ(S.renderAsPlainText(), "");
-  EXPECT_EQ(S.renderAsMarkdown(), "");
-
-  S.appendText("foobar  ");
-  S.appendText("baz");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foobar  baz");
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-                                  "foobar\n"
-                                  "```\n");
-}
-
-TEST(FormattedString, CodeBlocks) {
-  FormattedString S;
-  S.appendCodeBlock("foobar");
-  S.appendCodeBlock("bazqux", "javascript");
-  S.appendText("after");
-
-  std::string ExpectedText = R"(foobar
-
-bazqux
-
-after)";
-  EXPECT_EQ(S.renderAsPlainText(), ExpectedText);
-  std::string ExpectedMarkdown = R"md(```cpp
-foobar
-```
-```javascript
-bazqux
-```
-after)md";
-  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  S.appendInlineCode("bazqux");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
-
-  S = FormattedString();
-  S.appendText("foo");
-  S.appendInlineCode("bar");
-  S.appendText("baz");
-
-  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
+enum RenderKind {
+  PlainText,
+  Markdown,
+};
+std::string renderBlock(const Block &B, RenderKind RK) {
+  std::string R;
+  llvm::raw_string_ostream OS(R);
+  switch (RK) {
+  case PlainText:
+    B.asPlainText(OS);
+    break;
+  case Markdown:
+    B.asMarkdown(OS);
+    break;
+  }
+  return OS.str();
 }
 
-TEST(FormattedString, Escaping) {
+MATCHER_P(HasMarkdown, MD, "") { return renderBlock(arg, Markdown) == MD; }
+MATCHER_P(HasPlainText, PT, "") { return renderBlock(arg, PlainText) == PT; }
+
+TEST(Render, Escaping) {
   // Check some ASCII punctuation
-  FormattedString S;
-  S.appendText("*!`");
-  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+  Paragraph P;
+  P.appendText("*!`");
+  EXPECT_THAT(P, HasMarkdown("\\*\\!\\`"));
 
   // Check all ASCII punctuation.
-  S = FormattedString();
+  P = Paragraph();
   std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
   // Same text, with each character escaped.
   std::string EscapedPunctuation;
   EscapedPunctuation.reserve(2 * Punctuation.size());
   for (char C : Punctuation)
     EscapedPunctuation += std::string("\\") + C;
-  S.appendText(Punctuation);
-  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+  P.appendText(Punctuation);
+  EXPECT_THAT(P, HasMarkdown(EscapedPunctuation));
 
   // In code blocks we don't need to escape ASCII punctuation.
-  S = FormattedString();
-  S.appendInlineCode("* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
-  S = FormattedString();
-  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-                                  "#define FOO\n* foo !+ bar * baz\n"
-                                  "```\n");
+  P = Paragraph();
+  P.appendCode("* foo !+ bar * baz");
+  EXPECT_THAT(P, HasMarkdown("`* foo !+ bar * baz`"));
 
   // But we have to escape the backticks.
-  S = FormattedString();
-  S.appendInlineCode("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foo``bar``baz`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-                                  "foo`bar`baz\n"
-                                  "```\n");
+  P = Paragraph();
+  P.appendCode("foo`bar`baz");
+  EXPECT_THAT(P, HasMarkdown("`foo``bar``baz`"));
 
   // Inline code blocks starting or ending with backticks should add spaces.
-  S = FormattedString();
-  S.appendInlineCode("`foo");
-  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo `");
-  S = FormattedString();
-  S.appendInlineCode("foo`");
-  EXPECT_EQ(S.renderAsMarkdown(), "` foo`` `");
-  S = FormattedString();
-  S.appendInlineCode("`foo`");
-  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo`` `");
-
-  // Should also add extra spaces if the block stars and ends with spaces.
-  S = FormattedString();
-  S.appendInlineCode(" foo ");
-  EXPECT_EQ(S.renderAsMarkdown(), "`  foo  `");
-  S = FormattedString();
-  S.appendInlineCode("foo ");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foo `");
-  S = FormattedString();
-  S.appendInlineCode(" foo");
-  EXPECT_EQ(S.renderAsMarkdown(), "` foo`");
-
-  // Code blocks might need more than 3 backticks.
-  S = FormattedString();
-  S.appendCodeBlock("foobarbaz `\nqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-                                  "foobarbaz `\nqux\n"
-                                  "```\n");
-  S = FormattedString();
-  S.appendCodeBlock("foobarbaz ``\nqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-                                  "foobarbaz ``\nqux\n"
-                                  "```\n");
-  S = FormattedString();
-  S.appendCodeBlock("foobarbaz ```\nqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "````cpp\n"
-                                  "foobarbaz ```\nqux\n"
-                                  "````\n");
-  S = FormattedString();
-  S.appendCodeBlock("foobarbaz ` `` ``` ```` `\nqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "`````cpp\n"
-                                  "foobarbaz ` `` ``` ```` `\nqux\n"
-                                  "`````\n");
-}
-
-TEST(FormattedString, MarkdownWhitespace) {
-  // Whitespace should be added as separators between blocks.
-  FormattedString S;
-  S.appendText("foo");
-  S.appendText("bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo bar");
-
-  S = FormattedString();
-  S.appendInlineCode("foo");
-  S.appendInlineCode("bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foo` `bar`");
-
-  // However, we don't want to add any extra whitespace.
-  S = FormattedString();
-  S.appendText("foo ");
-  S.appendInlineCode("bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
-
-  S = FormattedString();
-  S.appendText("foo\n");
-  S.appendInlineCode("bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
-
-  S = FormattedString();
-  S.appendInlineCode("foo");
-  S.appendText(" bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
-
-  S = FormattedString();
-  S.appendText("foo");
-  S.appendCodeBlock("bar");
-  S.appendText("baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
+  P = Paragraph();
+  P.appendCode("`foo");
+  EXPECT_THAT(P, HasMarkdown("` ``foo `"));
+  P = Paragraph();
+  P.appendCode("foo`");
+  EXPECT_THAT(P, HasMarkdown("` foo`` `"));
+  P = Paragraph();
+  P.appendCode("`foo`");
+  EXPECT_THAT(P, HasMarkdown("` ``foo`` `"));
+}
+
+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.
+  struct Test {
+    enum {
+      PlainText,
+      Code,
+    } Kind;
+    llvm::StringRef Contents;
+    llvm::StringRef ExpectedMarkdown;
+    llvm::StringRef ExpectedPlainText;
+  } Tests[] = {
+      {
+          Test::PlainText,
+          "after",
+          "after",
+          "after",
+      },
+      {
+          Test::Code,
+          "foobar",
+          "after `foobar`",
+          "after foobar",
+      },
+      {
+          Test::PlainText,
+          "bat",
+          "after `foobar` bat",
+          "after foobar bat",
+      },
+  };
+  Paragraph P;
+  for (const auto &T : Tests) {
+    switch (T.Kind) {
+    case Test::Code:
+      P.appendCode(T.Contents);
+      break;
+    case Test::PlainText:
+      P.appendText(T.Contents);
+      break;
+    }
+    EXPECT_THAT(P, HasMarkdown(T.ExpectedMarkdown));
+    EXPECT_THAT(P, HasPlainText(T.ExpectedPlainText));
+  }
 }
 
+TEST(Paragraph, ExtraSpaces) {
+  // Make sure spaces inside chunks are dropped.
+  Paragraph P;
+  P.appendText("foo\n   \t   baz");
+  P.appendCode(" bar\n");
+  EXPECT_THAT(P, HasMarkdown(R"md(foo baz `bar`)md"));
+  EXPECT_THAT(P, HasPlainText(R"pt(foo baz bar)pt"));
+}
+
+TEST(Paragraph, NewLines) {
+  // New lines before and after chunks are dropped.
+  Paragraph P;
+  P.appendText(" \n foo\nbar\n ");
+  P.appendCode(" \n foo\nbar \n ");
+  EXPECT_THAT(P, HasMarkdown(R"md(foo bar `foo bar`)md"));
+  EXPECT_THAT(P, HasPlainText(R"pt(foo bar foo bar)pt"));
+}
+
+TEST(Document, Separators) {
+  Document D;
+  D.addParagraph().appendText("foo");
+  D.addParagraph().appendText("bar");
+  EXPECT_EQ(D.asMarkdown(), "foo\nbar");
+  EXPECT_EQ(D.asPlainText(), "foo\nbar");
+}
+
+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");
+}
 
 } // namespace
+} // namespace markup
 } // namespace clangd
 } // namespace clang