diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1091,10 +1091,10 @@ 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"); diff --git a/clang-tools-extra/clangd/FormattedString.h b/clang-tools-extra/clangd/FormattedString.h --- a/clang-tools-extra/clangd/FormattedString.h +++ b/clang-tools-extra/clangd/FormattedString.h @@ -13,38 +13,45 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FORMATTEDSTRING_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FORMATTEDSTRING_H +#include "llvm/Support/raw_ostream.h" +#include #include #include 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; @@ -52,6 +59,23 @@ std::vector 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> Children; +}; + +} // namespace markup } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/FormattedString.cpp b/clang-tools-extra/clangd/FormattedString.cpp --- a/clang-tools-extra/clangd/FormattedString.cpp +++ b/clang-tools-extra/clangd/FormattedString.cpp @@ -7,14 +7,22 @@ //===----------------------------------------------------------------------===// #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 +#include #include +#include namespace clang { namespace clangd { +namespace markup { namespace { /// Escape a markdown text block. Ensures the punctuation will not introduce @@ -63,137 +71,117 @@ 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) { - // 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; + +// 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 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> 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); } - 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 OS.str(); } +// 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 {} +}; + } // namespace -void FormattedString::appendText(std::string Text) { - Chunk C; - C.Kind = ChunkKind::PlainText; - C.Contents = Text; - Chunks.push_back(C); +void Paragraph::asMarkdown(llvm::raw_ostream &OS) const { + llvm::StringRef Sep = ""; + for (auto &C : Chunks) { + OS << Sep; + switch (C.Kind) { + case Chunk::PlainText: + OS << renderText(C.Contents); + break; + case Chunk::InlineCode: + OS << renderInlineBlock(C.Contents); + break; + } + Sep = " "; + } } -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)); +void Paragraph::asPlainText(llvm::raw_ostream &OS) const { + llvm::StringRef Sep = ""; + for (auto &C : Chunks) { + OS << Sep << C.Contents; + Sep = " "; + } } -void FormattedString::appendInlineCode(std::string Code) { - Chunk C; - C.Kind = ChunkKind::InlineCodeBlock; +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); - Chunks.push_back(std::move(C)); + C.Kind = Chunk::InlineCode; + return *this; } -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) { - 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; - } - llvm_unreachable("unhanlded ChunkKind"); - } - return R; +Paragraph &Document::addParagraph() { + Children.emplace_back(std::make_unique()); + return *static_cast(Children.back().get()); } -std::string FormattedString::renderAsPlainText() const { - std::string R; - auto EnsureWhitespace = [&]() { - if (R.empty() || isWhitespace(R.back())) - return; - R += " "; - }; - Optional LastWasBlock; - for (const auto &C : Chunks) { - bool IsBlock = C.Kind == ChunkKind::CodeBlock; - if (LastWasBlock.hasValue() && (IsBlock || *LastWasBlock)) - R += "\n\n"; - LastWasBlock = IsBlock; +void Document::addSpacer() { + Children.emplace_back(std::make_unique()); +} - 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; +std::string Document::asMarkdown() const { + return renderBlocks(Children, RenderType::Markdown); } -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; +std::string Document::asPlainText() const { + return renderBlocks(Children, RenderType::PlainText); } +} // namespace markup } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h --- a/clang-tools-extra/clangd/Hover.h +++ b/clang-tools-extra/clangd/Hover.h @@ -72,7 +72,7 @@ llvm::Optional 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, diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -11,6 +11,7 @@ #include "AST.h" #include "CodeCompletionStrings.h" #include "FindTarget.h" +#include "FormattedString.h" #include "Logger.h" #include "Selection.h" #include "SourceCode.h" @@ -440,28 +441,30 @@ 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; } diff --git a/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp b/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp --- a/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp +++ b/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp @@ -8,192 +8,139 @@ #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"); +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, 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"); -} +MATCHER_P(HasMarkdown, MD, "") { return renderBlock(arg, Markdown) == MD; } +MATCHER_P(HasPlainText, PT, "") { return renderBlock(arg, PlainText) == PT; } -TEST(FormattedString, Escaping) { +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"); + 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(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"); +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 "); +} } // namespace +} // namespace markup } // namespace clangd } // namespace clang