https://github.com/tcottin created https://github.com/llvm/llvm-project/pull/140498
This is a preparation for fixing clangd/clangd#529. It changes the Markup rendering to markdown and plaintext. - Properly separate paragraphs using an empty line between - Dont escape markdown syntax for markdown output except for HTML - Dont do any formatting for markdown because the client is handling the actual markdown rendering >From 8fadd8d51fa3d96c7fb82b9d749ef3f35441ac64 Mon Sep 17 00:00:00 2001 From: Tim Cottin <timcot...@gmx.de> Date: Mon, 19 May 2025 06:26:36 +0000 Subject: [PATCH] [clangd] Improve Markup Rendering --- clang-tools-extra/clangd/Hover.cpp | 81 +----- clang-tools-extra/clangd/support/Markup.cpp | 252 ++++++++++-------- clang-tools-extra/clangd/support/Markup.h | 32 ++- .../clangd/test/signature-help.test | 4 +- .../clangd/unittests/CodeCompleteTests.cpp | 8 +- .../clangd/unittests/HoverTests.cpp | 75 ++++-- .../clangd/unittests/support/MarkupTests.cpp | 214 +++++++++++---- 7 files changed, 410 insertions(+), 256 deletions(-) diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index 3ab3d89030520..88755733aa67c 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -960,42 +960,6 @@ std::optional<HoverInfo> getHoverContents(const Attr *A, ParsedAST &AST) { return HI; } -bool isParagraphBreak(llvm::StringRef Rest) { - return Rest.ltrim(" \t").starts_with("\n"); -} - -bool punctuationIndicatesLineBreak(llvm::StringRef Line) { - constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt"; - - Line = Line.rtrim(); - return !Line.empty() && Punctuation.contains(Line.back()); -} - -bool isHardLineBreakIndicator(llvm::StringRef Rest) { - // '-'/'*' md list, '@'/'\' documentation command, '>' md blockquote, - // '#' headings, '`' code blocks - constexpr llvm::StringLiteral LinebreakIndicators = R"txt(-*@\>#`)txt"; - - Rest = Rest.ltrim(" \t"); - if (Rest.empty()) - return false; - - if (LinebreakIndicators.contains(Rest.front())) - return true; - - if (llvm::isDigit(Rest.front())) { - llvm::StringRef AfterDigit = Rest.drop_while(llvm::isDigit); - if (AfterDigit.starts_with(".") || AfterDigit.starts_with(")")) - return true; - } - return false; -} - -bool isHardLineBreakAfter(llvm::StringRef Line, llvm::StringRef Rest) { - // Should we also consider whether Line is short? - return punctuationIndicatesLineBreak(Line) || isHardLineBreakIndicator(Rest); -} - void addLayoutInfo(const NamedDecl &ND, HoverInfo &HI) { if (ND.isInvalidDecl()) return; @@ -1601,51 +1565,32 @@ std::optional<llvm::StringRef> getBacktickQuoteRange(llvm::StringRef Line, return Line.slice(Offset, Next + 1); } -void parseDocumentationLine(llvm::StringRef Line, markup::Paragraph &Out) { +void parseDocumentationParagraph(llvm::StringRef Text, markup::Paragraph &Out) { // Probably this is appendText(Line), but scan for something interesting. - for (unsigned I = 0; I < Line.size(); ++I) { - switch (Line[I]) { + for (unsigned I = 0; I < Text.size(); ++I) { + switch (Text[I]) { case '`': - if (auto Range = getBacktickQuoteRange(Line, I)) { - Out.appendText(Line.substr(0, I)); + if (auto Range = getBacktickQuoteRange(Text, I)) { + Out.appendText(Text.substr(0, I)); Out.appendCode(Range->trim("`"), /*Preserve=*/true); - return parseDocumentationLine(Line.substr(I + Range->size()), Out); + return parseDocumentationParagraph(Text.substr(I + Range->size()), Out); } break; } } - Out.appendText(Line).appendSpace(); + Out.appendText(Text); } void parseDocumentation(llvm::StringRef Input, markup::Document &Output) { - std::vector<llvm::StringRef> ParagraphLines; - auto FlushParagraph = [&] { - if (ParagraphLines.empty()) - return; - auto &P = Output.addParagraph(); - for (llvm::StringRef Line : ParagraphLines) - parseDocumentationLine(Line, P); - ParagraphLines.clear(); - }; + llvm::StringRef Paragraph, Rest; + for (std::tie(Paragraph, Rest) = Input.split("\n\n"); + !(Paragraph.empty() && Rest.empty()); + std::tie(Paragraph, Rest) = Rest.split("\n\n")) { - llvm::StringRef Line, Rest; - for (std::tie(Line, Rest) = Input.split('\n'); - !(Line.empty() && Rest.empty()); - std::tie(Line, Rest) = Rest.split('\n')) { - - // After a linebreak remove spaces to avoid 4 space markdown code blocks. - // FIXME: make FlushParagraph handle this. - Line = Line.ltrim(); - if (!Line.empty()) - ParagraphLines.push_back(Line); - - if (isParagraphBreak(Rest) || isHardLineBreakAfter(Line, Rest)) { - FlushParagraph(); - } + if (!Paragraph.empty()) + parseDocumentationParagraph(Paragraph, Output.addParagraph()); } - FlushParagraph(); } - llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const HoverInfo::PrintedType &T) { OS << T.Type; diff --git a/clang-tools-extra/clangd/support/Markup.cpp b/clang-tools-extra/clangd/support/Markup.cpp index 63aff96b02056..b1e6252e473f5 100644 --- a/clang-tools-extra/clangd/support/Markup.cpp +++ b/clang-tools-extra/clangd/support/Markup.cpp @@ -11,7 +11,6 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/Compiler.h" #include "llvm/Support/raw_ostream.h" #include <cstddef> #include <iterator> @@ -56,80 +55,28 @@ bool looksLikeTag(llvm::StringRef Contents) { return true; // Potentially incomplete tag. } -// Tests whether C should be backslash-escaped in markdown. -// The string being escaped is Before + C + After. This is part of a paragraph. -// StartsLine indicates whether `Before` is the start of the line. -// After may not be everything until the end of the line. -// -// It's always safe to escape punctuation, but want minimal escaping. -// The strategy is to escape the first character of anything that might start -// a markdown grammar construct. -bool needsLeadingEscape(char C, llvm::StringRef Before, llvm::StringRef After, - bool StartsLine) { - assert(Before.take_while(llvm::isSpace).empty()); - auto RulerLength = [&]() -> /*Length*/ unsigned { - if (!StartsLine || !Before.empty()) - return false; - llvm::StringRef A = After.rtrim(); - return llvm::all_of(A, [C](char D) { return C == D; }) ? 1 + A.size() : 0; - }; - auto IsBullet = [&]() { - return StartsLine && Before.empty() && - (After.empty() || After.starts_with(" ")); - }; - auto SpaceSurrounds = [&]() { - return (After.empty() || llvm::isSpace(After.front())) && - (Before.empty() || llvm::isSpace(Before.back())); - }; - auto WordSurrounds = [&]() { - return (!After.empty() && llvm::isAlnum(After.front())) && - (!Before.empty() && llvm::isAlnum(Before.back())); - }; - +/// \brief Tests whether \p C should be backslash-escaped in markdown. +/// +/// The MarkupContent LSP specification defines that `markdown` content needs to +/// follow GFM (GitHub Flavored Markdown) rules. And we can assume that markdown +/// is rendered on the client side. This means we do not need to escape any +/// markdown constructs. +/// The only exception is when the client does not support HTML rendering in +/// markdown. In that case, we need to escape HTML tags and HTML entities. +/// +/// **FIXME:** handle the case when the client does support HTML rendering in +/// markdown. For this, the LSP server needs to check the +/// [supportsHtml capability](https://github.com/microsoft/language-server-protocol/issues/1344) +/// of the client. +/// +/// \param C The character to check. +/// \param After The string that follows \p C . This is used to determine if \p C is +/// part of a tag or an entity reference. +/// \returns true if \p C should be escaped, false otherwise. +bool needsLeadingEscape(char C, llvm::StringRef After) { switch (C) { - case '\\': // Escaped character. - return true; - case '`': // Code block or inline code - // Any number of backticks can delimit an inline code block that can end - // anywhere (including on another line). We must escape them all. - return true; - case '~': // Code block - return StartsLine && Before.empty() && After.starts_with("~~"); - case '#': { // ATX heading. - if (!StartsLine || !Before.empty()) - return false; - llvm::StringRef Rest = After.ltrim(C); - return Rest.empty() || Rest.starts_with(" "); - } - case ']': // Link or link reference. - // We escape ] rather than [ here, because it's more constrained: - // ](...) is an in-line link - // ]: is a link reference - // The following are only links if the link reference exists: - // ] by itself is a shortcut link - // ][...] is an out-of-line link - // Because we never emit link references, we don't need to handle these. - return After.starts_with(":") || After.starts_with("("); - case '=': // Setex heading. - return RulerLength() > 0; - case '_': // Horizontal ruler or matched delimiter. - if (RulerLength() >= 3) - return true; - // Not a delimiter if surrounded by space, or inside a word. - // (The rules at word boundaries are subtle). - return !(SpaceSurrounds() || WordSurrounds()); - case '-': // Setex heading, horizontal ruler, or bullet. - if (RulerLength() > 0) - return true; - return IsBullet(); - case '+': // Bullet list. - return IsBullet(); - case '*': // Bullet list, horizontal ruler, or delimiter. - return IsBullet() || RulerLength() >= 3 || !SpaceSurrounds(); case '<': // HTML tag (or autolink, which we choose not to escape) return looksLikeTag(After); - case '>': // Quote marker. Needs escaping at start of line. - return StartsLine && Before.empty(); case '&': { // HTML entity reference auto End = After.find(';'); if (End == llvm::StringRef::npos) @@ -142,10 +89,6 @@ bool needsLeadingEscape(char C, llvm::StringRef Before, llvm::StringRef After, } return llvm::all_of(Content, llvm::isAlpha); } - case '.': // Numbered list indicator. Escape 12. -> 12\. at start of line. - case ')': - return StartsLine && !Before.empty() && - llvm::all_of(Before, llvm::isDigit) && After.starts_with(" "); default: return false; } @@ -156,8 +99,7 @@ bool needsLeadingEscape(char C, llvm::StringRef Before, llvm::StringRef After, std::string renderText(llvm::StringRef Input, bool StartsLine) { std::string R; for (unsigned I = 0; I < Input.size(); ++I) { - if (needsLeadingEscape(Input[I], Input.substr(0, I), Input.substr(I + 1), - StartsLine)) + if (needsLeadingEscape(Input[I], Input.substr(I + 1))) R.push_back('\\'); R.push_back(Input[I]); } @@ -303,11 +245,12 @@ class CodeBlock : public Block { std::string indentLines(llvm::StringRef Input) { assert(!Input.ends_with("\n") && "Input should've been trimmed."); std::string IndentedR; - // We'll add 2 spaces after each new line. + // We'll add 2 spaces after each new line which is not followed by another new line. IndentedR.reserve(Input.size() + Input.count('\n') * 2); - for (char C : Input) { + for (size_t I = 0; I < Input.size(); ++I) { + char C = Input[I]; IndentedR += C; - if (C == '\n') + if (C == '\n' && (((I + 1) < Input.size()) && (Input[I + 1] != '\n'))) IndentedR.append(" "); } return IndentedR; @@ -348,20 +291,24 @@ void Paragraph::renderMarkdown(llvm::raw_ostream &OS) const { if (C.SpaceBefore || NeedsSpace) OS << " "; switch (C.Kind) { - case Chunk::PlainText: + case ChunkKind::PlainText: OS << renderText(C.Contents, !HasChunks); break; - case Chunk::InlineCode: + case ChunkKind::InlineCode: OS << renderInlineBlock(C.Contents); break; + case ChunkKind::Bold: + OS << "**" << renderText(C.Contents, !HasChunks) << "**"; + break; + case ChunkKind::Emphasized: + OS << "*" << renderText(C.Contents, !HasChunks) << "*"; + break; } HasChunks = true; NeedsSpace = C.SpaceAfter; } - // Paragraphs are translated into markdown lines, not markdown paragraphs. - // Therefore it only has a single linebreak afterwards. - // VSCode requires two spaces at the end of line to start a new one. - OS << " \n"; + // A paragraph in markdown is separated by a blank line. + OS << "\n\n"; } std::unique_ptr<Block> Paragraph::clone() const { @@ -370,8 +317,8 @@ std::unique_ptr<Block> Paragraph::clone() const { /// Choose a marker to delimit `Text` from a prioritized list of options. /// This is more readable than escaping for plain-text. -llvm::StringRef chooseMarker(llvm::ArrayRef<llvm::StringRef> Options, - llvm::StringRef Text) { +llvm::StringRef Paragraph::chooseMarker(llvm::ArrayRef<llvm::StringRef> Options, + llvm::StringRef Text) const { // Prefer a delimiter whose characters don't appear in the text. for (llvm::StringRef S : Options) if (Text.find_first_of(S) == llvm::StringRef::npos) @@ -379,18 +326,94 @@ llvm::StringRef chooseMarker(llvm::ArrayRef<llvm::StringRef> Options, return Options.front(); } +bool Paragraph::punctuationIndicatesLineBreak(llvm::StringRef Line) const{ + constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt"; + + Line = Line.rtrim(); + return !Line.empty() && Punctuation.contains(Line.back()); +} + +bool Paragraph::isHardLineBreakIndicator(llvm::StringRef Rest) const { + // '-'/'*' md list, '@'/'\' documentation command, '>' md blockquote, + // '#' headings, '`' code blocks, two spaces (markdown force newline) + constexpr llvm::StringLiteral LinebreakIndicators = R"txt(-*@\>#`)txt"; + + Rest = Rest.ltrim(" \t"); + if (Rest.empty()) + return false; + + if (LinebreakIndicators.contains(Rest.front())) + return true; + + if (llvm::isDigit(Rest.front())) { + llvm::StringRef AfterDigit = Rest.drop_while(llvm::isDigit); + if (AfterDigit.starts_with(".") || AfterDigit.starts_with(")")) + return true; + } + return false; +} + +bool Paragraph::isHardLineBreakAfter(llvm::StringRef Line, + llvm::StringRef Rest) const { + // In Markdown, 2 spaces before a line break forces a line break. + // Add a line break for plaintext in this case too. + // Should we also consider whether Line is short? + return Line.ends_with(" ") || punctuationIndicatesLineBreak(Line) || + isHardLineBreakIndicator(Rest); +} + void Paragraph::renderPlainText(llvm::raw_ostream &OS) const { bool NeedsSpace = false; + std::string ConcatenatedText; + llvm::raw_string_ostream ConcatenatedOS(ConcatenatedText); + for (auto &C : Chunks) { + + if (C.Kind == ChunkKind::PlainText) { + if (C.SpaceBefore || NeedsSpace) + ConcatenatedOS << ' '; + + ConcatenatedOS << C.Contents; + NeedsSpace = llvm::isSpace(C.Contents.back()) || C.SpaceAfter; + continue; + } + if (C.SpaceBefore || NeedsSpace) - OS << " "; + ConcatenatedOS << ' '; llvm::StringRef Marker = ""; - if (C.Preserve && C.Kind == Chunk::InlineCode) + if (C.Preserve && C.Kind == ChunkKind::InlineCode) Marker = chooseMarker({"`", "'", "\""}, C.Contents); - OS << Marker << C.Contents << Marker; + else if (C.Kind == ChunkKind::Bold) + Marker = "**"; + else if (C.Kind == ChunkKind::Emphasized) + Marker = "*"; + ConcatenatedOS << Marker << C.Contents << Marker; NeedsSpace = C.SpaceAfter; } - OS << '\n'; + + // We go through the contents line by line to handle the newlines + // and required spacing correctly. + llvm::StringRef Line, Rest; + + for (std::tie(Line, Rest) = + llvm::StringRef(ConcatenatedText).trim().split('\n'); + !(Line.empty() && Rest.empty()); + std::tie(Line, Rest) = Rest.split('\n')) { + + Line = Line.ltrim(); + if (Line.empty()) + continue; + + OS << canonicalizeSpaces(Line); + + if (isHardLineBreakAfter(Line, Rest)) + OS << '\n'; + else if (!Rest.empty()) + OS << ' '; + } + + // Paragraphs are separated by a blank line. + OS << "\n\n"; } BulletList::BulletList() = default; @@ -398,12 +421,13 @@ BulletList::~BulletList() = default; void BulletList::renderMarkdown(llvm::raw_ostream &OS) const { for (auto &D : Items) { + std::string M = D.asMarkdown(); // 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'; + OS << "- " << indentLines(M) << '\n'; } // We need a new line after list to terminate it in markdown. - OS << '\n'; + OS << "\n\n"; } void BulletList::renderPlainText(llvm::raw_ostream &OS) const { @@ -412,6 +436,7 @@ void BulletList::renderPlainText(llvm::raw_ostream &OS) const { // rid of the copies, if it turns out to be a bottleneck. OS << "- " << indentLines(D.asPlainText()) << '\n'; } + OS << '\n'; } Paragraph &Paragraph::appendSpace() { @@ -420,29 +445,44 @@ Paragraph &Paragraph::appendSpace() { return *this; } -Paragraph &Paragraph::appendText(llvm::StringRef Text) { - std::string Norm = canonicalizeSpaces(Text); - if (Norm.empty()) +Paragraph &Paragraph::appendChunk(llvm::StringRef Contents, ChunkKind K) { + if (Contents.empty()) return *this; Chunks.emplace_back(); Chunk &C = Chunks.back(); - C.Contents = std::move(Norm); - C.Kind = Chunk::PlainText; - C.SpaceBefore = llvm::isSpace(Text.front()); - C.SpaceAfter = llvm::isSpace(Text.back()); + C.Contents = std::move(Contents); + C.Kind = K; return *this; } +Paragraph &Paragraph::appendText(llvm::StringRef Text) { + if (!Chunks.empty() && Chunks.back().Kind == ChunkKind::PlainText) { + Chunks.back().Contents += std::move(Text); + return *this; + } + + return appendChunk(Text, ChunkKind::PlainText); +} + +Paragraph &Paragraph::appendEmphasizedText(llvm::StringRef Text) { + return appendChunk(canonicalizeSpaces(std::move(Text)), + ChunkKind::Emphasized); +} + +Paragraph &Paragraph::appendBoldText(llvm::StringRef Text) { + return appendChunk(canonicalizeSpaces(std::move(Text)), ChunkKind::Bold); +} + Paragraph &Paragraph::appendCode(llvm::StringRef Code, bool Preserve) { bool AdjacentCode = - !Chunks.empty() && Chunks.back().Kind == Chunk::InlineCode; + !Chunks.empty() && Chunks.back().Kind == ChunkKind::InlineCode; std::string Norm = canonicalizeSpaces(std::move(Code)); if (Norm.empty()) return *this; Chunks.emplace_back(); Chunk &C = Chunks.back(); C.Contents = std::move(Norm); - C.Kind = Chunk::InlineCode; + C.Kind = ChunkKind::InlineCode; C.Preserve = Preserve; // Disallow adjacent code spans without spaces, markdown can't render them. C.SpaceBefore = AdjacentCode; @@ -475,7 +515,9 @@ Paragraph &Document::addParagraph() { return *static_cast<Paragraph *>(Children.back().get()); } -void Document::addRuler() { Children.push_back(std::make_unique<Ruler>()); } +void Document::addRuler() { + Children.push_back(std::make_unique<Ruler>()); +} void Document::addCodeBlock(std::string Code, std::string Language) { Children.emplace_back( diff --git a/clang-tools-extra/clangd/support/Markup.h b/clang-tools-extra/clangd/support/Markup.h index 3a869c49a2cbb..a74fade13d115 100644 --- a/clang-tools-extra/clangd/support/Markup.h +++ b/clang-tools-extra/clangd/support/Markup.h @@ -49,6 +49,12 @@ class Paragraph : public Block { /// Append plain text to the end of the string. Paragraph &appendText(llvm::StringRef Text); + /// Append emphasized text, this translates to the * block in markdown. + Paragraph &appendEmphasizedText(llvm::StringRef Text); + + /// Append bold text, this translates to the ** block in markdown. + Paragraph &appendBoldText(llvm::StringRef Text); + /// Append inline code, this translates to the ` block in markdown. /// \p Preserve indicates the code span must be apparent even in plaintext. Paragraph &appendCode(llvm::StringRef Code, bool Preserve = false); @@ -58,11 +64,9 @@ class Paragraph : public Block { Paragraph &appendSpace(); private: + typedef enum { PlainText, InlineCode, Bold, Emphasized } ChunkKind; struct Chunk { - enum { - PlainText, - InlineCode, - } Kind = PlainText; + ChunkKind Kind = PlainText; // Preserve chunk markers in plaintext. bool Preserve = false; std::string Contents; @@ -73,6 +77,19 @@ class Paragraph : public Block { bool SpaceAfter = false; }; std::vector<Chunk> Chunks; + + Paragraph &appendChunk(llvm::StringRef Contents, ChunkKind K); + + llvm::StringRef chooseMarker(llvm::ArrayRef<llvm::StringRef> Options, + llvm::StringRef Text) const; + bool punctuationIndicatesLineBreak(llvm::StringRef Line) const; + bool isHardLineBreakIndicator(llvm::StringRef Rest) const; + bool isHardLineBreakAfter(llvm::StringRef Line, llvm::StringRef Rest) const; +}; + +class ListItemParagraph : public Paragraph { +public: + void renderMarkdown(llvm::raw_ostream &OS) const override; }; /// Represents a sequence of one or more documents. Knows how to print them in a @@ -82,6 +99,9 @@ class BulletList : public Block { BulletList(); ~BulletList(); + // A BulletList rendered in markdown is a tight list if it is not a nested + // list and no item contains multiple paragraphs. Otherwise, it is a loose + // list. void renderMarkdown(llvm::raw_ostream &OS) const override; void renderPlainText(llvm::raw_ostream &OS) const override; std::unique_ptr<Block> clone() const override; @@ -118,8 +138,8 @@ class Document { BulletList &addBulletList(); /// Doesn't contain any trailing newlines. - /// We try to make the markdown human-readable, e.g. avoid extra escaping. - /// At least one client (coc.nvim) displays the markdown verbatim! + /// It is expected that the result of this function + /// is rendered as markdown. std::string asMarkdown() const; /// Doesn't contain any trailing newlines. std::string asPlainText() const; diff --git a/clang-tools-extra/clangd/test/signature-help.test b/clang-tools-extra/clangd/test/signature-help.test index a642574571cc3..cc6f3a09cee71 100644 --- a/clang-tools-extra/clangd/test/signature-help.test +++ b/clang-tools-extra/clangd/test/signature-help.test @@ -2,7 +2,7 @@ # Start a session. {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument": {"signatureHelp": {"signatureInformation": {"documentationFormat": ["markdown", "plaintext"]}}}},"trace":"off"}} --- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"// comment `markdown` _escape_\nvoid x(int);\nint main(){\nx("}}} +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"// comment `markdown` _noescape_\nvoid x(int);\nint main(){\nx("}}} --- {"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":3,"character":2}}} # CHECK: "id": 1, @@ -14,7 +14,7 @@ # CHECK-NEXT: { # CHECK-NEXT: "documentation": { # CHECK-NEXT: "kind": "markdown", -# CHECK-NEXT: "value": "comment `markdown` \\_escape\\_" +# CHECK-NEXT: "value": "comment `markdown` _noescape_" # CHECK-NEXT: }, # CHECK-NEXT: "label": "x(int) -> void", # CHECK-NEXT: "parameters": [ diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index b12f8275b8a26..db9626bee300e 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1098,7 +1098,7 @@ TEST(CompletionTest, Documentation) { EXPECT_THAT(Results.Completions, Contains(AllOf( named("foo"), - doc("Annotation: custom_annotation\nNon-doxygen comment.")))); + doc("Annotation: custom_annotation\n\nNon-doxygen comment.")))); EXPECT_THAT( Results.Completions, Contains(AllOf(named("bar"), doc("Doxygen comment.\n\\param int a")))); @@ -2297,7 +2297,7 @@ TEST(CompletionTest, Render) { EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText); EXPECT_EQ(R.filterText, "x"); EXPECT_EQ(R.detail, "int"); - EXPECT_EQ(R.documentation->value, "From \"foo.h\"\nThis is x()"); + EXPECT_EQ(R.documentation->value, "From \"foo.h\"\n\nThis is x()"); EXPECT_THAT(R.additionalTextEdits, IsEmpty()); EXPECT_EQ(R.sortText, sortText(1.0, "x")); EXPECT_FALSE(R.deprecated); @@ -2332,7 +2332,7 @@ TEST(CompletionTest, Render) { C.BundleSize = 2; R = C.render(Opts); EXPECT_EQ(R.detail, "[2 overloads]"); - EXPECT_EQ(R.documentation->value, "From \"foo.h\"\nThis is x()"); + EXPECT_EQ(R.documentation->value, "From \"foo.h\"\n\nThis is x()"); C.Deprecated = true; R = C.render(Opts); @@ -2340,7 +2340,7 @@ TEST(CompletionTest, Render) { Opts.DocumentationFormat = MarkupKind::Markdown; R = C.render(Opts); - EXPECT_EQ(R.documentation->value, "From `\"foo.h\"` \nThis is `x()`"); + EXPECT_EQ(R.documentation->value, "From `\"foo.h\"`\n\nThis is `x()`"); } TEST(CompletionTest, IgnoreRecoveryResults) { diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp index 69f6df46c87ce..0047eed03d8d9 100644 --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -3233,8 +3233,8 @@ TEST(Hover, ParseProviderInfo) { struct Case { HoverInfo HI; llvm::StringRef ExpectedMarkdown; - } Cases[] = {{HIFoo, "### `foo` \nprovided by `\"foo.h\"`"}, - {HIFooBar, "### `foo` \nprovided by `<bar.h>`"}}; + } Cases[] = {{HIFoo, "### `foo`\n\nprovided by `\"foo.h\"`"}, + {HIFooBar, "### `foo`\n\nprovided by `<bar.h>`"}}; for (const auto &Case : Cases) EXPECT_EQ(Case.HI.present().asMarkdown(), Case.ExpectedMarkdown); @@ -3441,6 +3441,7 @@ TEST(Hover, Present) { R"(class foo Size: 10 bytes + documentation template <typename T, typename C = bool> class Foo {})", @@ -3465,8 +3466,8 @@ template <typename T, typename C = bool> class Foo {})", }, "function foo\n" "\n" - "→ ret_type (aka can_ret_type)\n" - "Parameters:\n" + "→ ret_type (aka can_ret_type)\n\n" + "Parameters:\n\n" "- \n" "- type (aka can_type)\n" "- type foo (aka can_type)\n" @@ -3491,8 +3492,11 @@ template <typename T, typename C = bool> class Foo {})", R"(field foo Type: type (aka can_type) + Value = value + Offset: 12 bytes + Size: 4 bytes (+4 bytes padding), alignment 4 bytes // In test::Bar @@ -3514,8 +3518,11 @@ def)", R"(field foo Type: type (aka can_type) + Value = value + Offset: 4 bytes and 3 bits + Size: 25 bits (+4 bits padding), alignment 8 bytes // In test::Bar @@ -3573,6 +3580,7 @@ protected: size_t method())", R"(constructor cls Parameters: + - int a - int b = 5 @@ -3609,7 +3617,9 @@ private: union foo {})", R"(variable foo Type: int + Value = 3 + Passed as arg_a // In test::Bar @@ -3644,7 +3654,9 @@ Passed by value)", R"(variable foo Type: int + Value = 3 + Passed by reference as arg_a // In test::Bar @@ -3667,7 +3679,9 @@ int foo = 3)", R"(variable foo Type: int + Value = 3 + Passed as arg_a (converted to alias_int) // In test::Bar @@ -3705,7 +3719,9 @@ int foo = 3)", R"(variable foo Type: int + Value = 3 + Passed by const reference as arg_a (converted to int) // In test::Bar @@ -3752,57 +3768,67 @@ TEST(Hover, ParseDocumentation) { llvm::StringRef ExpectedRenderPlainText; } Cases[] = {{ " \n foo\nbar", - "foo bar", + "foo\nbar", "foo bar", }, { "foo\nbar \n ", - "foo bar", + "foo\nbar", "foo bar", }, { "foo \nbar", - "foo bar", - "foo bar", + "foo \nbar", + "foo\nbar", }, { "foo \nbar", - "foo bar", - "foo bar", + "foo \nbar", + "foo\nbar", }, { "foo\n\n\nbar", - "foo \nbar", - "foo\nbar", + "foo\n\nbar", + "foo\n\nbar", }, { "foo\n\n\n\tbar", - "foo \nbar", - "foo\nbar", + "foo\n\n\tbar", + "foo\n\nbar", + }, + { + "foo\n\n\n bar", + "foo\n\n bar", + "foo\n\nbar", + }, + { + "foo\n\n\n bar", + "foo\n\n bar", + "foo\n\nbar", }, { "foo\n\n\n bar", - "foo \nbar", - "foo\nbar", + "foo\n\n bar", + "foo\n\nbar", }, { "foo.\nbar", - "foo. \nbar", + "foo.\nbar", "foo.\nbar", }, { "foo. \nbar", - "foo. \nbar", + "foo. \nbar", "foo.\nbar", }, { "foo\n*bar", - "foo \n\\*bar", + "foo\n*bar", "foo\n*bar", }, { "foo\nbar", - "foo bar", + "foo\nbar", "foo bar", }, { @@ -3812,15 +3838,16 @@ TEST(Hover, ParseDocumentation) { }, { "'`' should not occur in `Code`", - "'\\`' should not occur in `Code`", + "'`' should not occur in `Code`", "'`' should not occur in `Code`", }, { "`not\nparsed`", - "\\`not parsed\\`", + "`not parsed`", "`not parsed`", }}; + //Case C = Cases[2]; for (const auto &C : Cases) { markup::Document Output; parseDocumentation(C.Documentation, Output); @@ -3850,10 +3877,10 @@ TEST(Hover, PresentRulers) { HI.Definition = "def"; llvm::StringRef ExpectedMarkdown = // - "### variable `foo` \n" + "### variable `foo`\n" "\n" "---\n" - "Value = `val` \n" + "Value = `val`\n" "\n" "---\n" "```cpp\n" diff --git a/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp b/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp index 2d86c91c7ec08..f1a4211997c9c 100644 --- a/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp +++ b/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp @@ -33,26 +33,25 @@ MATCHER(escapedNone, "") { TEST(Render, Escaping) { // Check all ASCII punctuation. std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt"; - std::string EscapedPunc = R"txt(!"#$%&'()\*+,-./:;<=>?@[\\]^\_\`{|}~)txt"; - EXPECT_EQ(escape(Punctuation), EscapedPunc); + EXPECT_EQ(escape(Punctuation), Punctuation); // Inline code - EXPECT_EQ(escape("`foo`"), R"(\`foo\`)"); - EXPECT_EQ(escape("`foo"), R"(\`foo)"); - EXPECT_EQ(escape("foo`"), R"(foo\`)"); - EXPECT_EQ(escape("``foo``"), R"(\`\`foo\`\`)"); + EXPECT_THAT(escape("`foo`"), escapedNone()); + EXPECT_THAT(escape("`foo"), escapedNone()); + EXPECT_THAT(escape("foo`"), escapedNone()); + EXPECT_THAT(escape("``foo``"), escapedNone()); // Code blocks - EXPECT_EQ(escape("```"), R"(\`\`\`)"); // This could also be inline code! - EXPECT_EQ(escape("~~~"), R"(\~~~)"); + EXPECT_THAT(escape("```"), escapedNone()); + EXPECT_THAT(escape("~~~"), escapedNone()); // Rulers and headings - EXPECT_THAT(escape("## Heading"), escaped('#')); + EXPECT_THAT(escape("## Heading"), escapedNone()); EXPECT_THAT(escape("Foo # bar"), escapedNone()); - EXPECT_EQ(escape("---"), R"(\---)"); - EXPECT_EQ(escape("-"), R"(\-)"); - EXPECT_EQ(escape("==="), R"(\===)"); - EXPECT_EQ(escape("="), R"(\=)"); - EXPECT_EQ(escape("***"), R"(\*\*\*)"); // \** could start emphasis! + EXPECT_THAT(escape("---"), escapedNone()); + EXPECT_THAT(escape("-"), escapedNone()); + EXPECT_THAT(escape("==="), escapedNone()); + EXPECT_THAT(escape("="), escapedNone()); + EXPECT_THAT(escape("***"), escapedNone()); // \** could start emphasis! // HTML tags. EXPECT_THAT(escape("<pre"), escaped('<')); @@ -68,24 +67,24 @@ TEST(Render, Escaping) { EXPECT_THAT(escape("Website <http://foo.bar>"), escapedNone()); // Bullet lists. - EXPECT_THAT(escape("- foo"), escaped('-')); - EXPECT_THAT(escape("* foo"), escaped('*')); - EXPECT_THAT(escape("+ foo"), escaped('+')); - EXPECT_THAT(escape("+"), escaped('+')); + EXPECT_THAT(escape("- foo"), escapedNone()); + EXPECT_THAT(escape("* foo"), escapedNone()); + EXPECT_THAT(escape("+ foo"), escapedNone()); + EXPECT_THAT(escape("+"), escapedNone()); EXPECT_THAT(escape("a + foo"), escapedNone()); EXPECT_THAT(escape("a+ foo"), escapedNone()); - EXPECT_THAT(escape("1. foo"), escaped('.')); + EXPECT_THAT(escape("1. foo"), escapedNone()); EXPECT_THAT(escape("a. foo"), escapedNone()); // Emphasis. - EXPECT_EQ(escape("*foo*"), R"(\*foo\*)"); - EXPECT_EQ(escape("**foo**"), R"(\*\*foo\*\*)"); - EXPECT_THAT(escape("*foo"), escaped('*')); + EXPECT_THAT(escape("*foo*"), escapedNone()); + EXPECT_THAT(escape("**foo**"), escapedNone()); + EXPECT_THAT(escape("*foo"), escapedNone()); EXPECT_THAT(escape("foo *"), escapedNone()); EXPECT_THAT(escape("foo * bar"), escapedNone()); EXPECT_THAT(escape("foo_bar"), escapedNone()); - EXPECT_THAT(escape("foo _bar"), escaped('_')); - EXPECT_THAT(escape("foo_ bar"), escaped('_')); + EXPECT_THAT(escape("foo _bar"), escapedNone()); + EXPECT_THAT(escape("foo_ bar"), escapedNone()); EXPECT_THAT(escape("foo _ bar"), escapedNone()); // HTML entities. @@ -97,8 +96,8 @@ TEST(Render, Escaping) { EXPECT_THAT(escape("foo &?; bar"), escapedNone()); // Links. - EXPECT_THAT(escape("[foo](bar)"), escaped(']')); - EXPECT_THAT(escape("[foo]: bar"), escaped(']')); + EXPECT_THAT(escape("[foo](bar)"), escapedNone()); + EXPECT_THAT(escape("[foo]: bar"), escapedNone()); // No need to escape these, as the target never exists. EXPECT_THAT(escape("[foo][]"), escapedNone()); EXPECT_THAT(escape("[foo][bar]"), escapedNone()); @@ -182,14 +181,87 @@ TEST(Paragraph, SeparationOfChunks) { P.appendCode("no").appendCode("space"); EXPECT_EQ(P.asMarkdown(), "after `foobar` bat`no` `space`"); EXPECT_EQ(P.asPlainText(), "after foobar batno space"); + + P.appendText(" text"); + EXPECT_EQ(P.asMarkdown(), "after `foobar` bat`no` `space` text"); + EXPECT_EQ(P.asPlainText(), "after foobar batno space text"); + + P.appendSpace().appendCode("code").appendText(".\n newline"); + EXPECT_EQ(P.asMarkdown(), "after `foobar` bat`no` `space` text `code`.\n newline"); + EXPECT_EQ(P.asPlainText(), "after foobar batno space text code.\nnewline"); +} + +TEST(Paragraph, SeparationOfChunks2) { + // 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 + // where the spacing is in the appended string rather set by appendSpace. + Paragraph P; + + P.appendText("after "); + EXPECT_EQ(P.asMarkdown(), "after"); + EXPECT_EQ(P.asPlainText(), "after"); + + P.appendText("foobar"); + EXPECT_EQ(P.asMarkdown(), "after foobar"); + EXPECT_EQ(P.asPlainText(), "after foobar"); + + P.appendText(" bat"); + EXPECT_EQ(P.asMarkdown(), "after foobar bat"); + EXPECT_EQ(P.asPlainText(), "after foobar bat"); + + P.appendText("baz"); + EXPECT_EQ(P.asMarkdown(), "after foobar batbaz"); + EXPECT_EQ(P.asPlainText(), "after foobar batbaz"); + + P.appendText(" faz "); + EXPECT_EQ(P.asMarkdown(), "after foobar batbaz faz"); + EXPECT_EQ(P.asPlainText(), "after foobar batbaz faz"); + + P.appendText(" bar "); + EXPECT_EQ(P.asMarkdown(), "after foobar batbaz faz bar"); + EXPECT_EQ(P.asPlainText(), "after foobar batbaz faz bar"); + + P.appendText("qux"); + EXPECT_EQ(P.asMarkdown(), "after foobar batbaz faz bar qux"); + EXPECT_EQ(P.asPlainText(), "after foobar batbaz faz bar qux"); +} + +TEST(Paragraph, SeparationOfChunks3) { + // 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 + // where the spacing is in the appended string rather set by appendSpace. + Paragraph P; + + P.appendText("after \n"); + EXPECT_EQ(P.asMarkdown(), "after"); + EXPECT_EQ(P.asPlainText(), "after"); + + P.appendText(" foobar\n"); + EXPECT_EQ(P.asMarkdown(), "after \n foobar"); + EXPECT_EQ(P.asPlainText(), "after\nfoobar"); + + P.appendText("- bat\n"); + EXPECT_EQ(P.asMarkdown(), "after \n foobar\n- bat"); + EXPECT_EQ(P.asPlainText(), "after\nfoobar\n- bat"); + + P.appendText("- baz"); + EXPECT_EQ(P.asMarkdown(), "after \n foobar\n- bat\n- baz"); + EXPECT_EQ(P.asPlainText(), "after\nfoobar\n- bat\n- baz"); + + P.appendText(" faz "); + EXPECT_EQ(P.asMarkdown(), "after \n foobar\n- bat\n- baz faz"); + EXPECT_EQ(P.asPlainText(), "after\nfoobar\n- bat\n- baz faz"); } TEST(Paragraph, ExtraSpaces) { - // Make sure spaces inside chunks are dropped. + // Make sure spaces inside chunks are preserved for markdown + // and dropped for plain text. Paragraph P; P.appendText("foo\n \t baz"); P.appendCode(" bar\n"); - EXPECT_EQ(P.asMarkdown(), "foo baz`bar`"); + EXPECT_EQ(P.asMarkdown(), "foo\n \t baz`bar`"); EXPECT_EQ(P.asPlainText(), "foo bazbar"); } @@ -197,7 +269,7 @@ TEST(Paragraph, SpacesCollapsed) { Paragraph P; P.appendText(" foo bar "); P.appendText(" baz "); - EXPECT_EQ(P.asMarkdown(), "foo bar baz"); + EXPECT_EQ(P.asMarkdown(), "foo bar baz"); EXPECT_EQ(P.asPlainText(), "foo bar baz"); } @@ -206,17 +278,48 @@ TEST(Paragraph, NewLines) { Paragraph P; P.appendText(" \n foo\nbar\n "); P.appendCode(" \n foo\nbar \n "); - EXPECT_EQ(P.asMarkdown(), "foo bar `foo bar`"); + EXPECT_EQ(P.asMarkdown(), "foo\nbar\n `foo bar`"); EXPECT_EQ(P.asPlainText(), "foo bar foo bar"); } +TEST(Paragraph, BoldText) { + Paragraph P; + P.appendBoldText(""); + EXPECT_EQ(P.asMarkdown(), ""); + EXPECT_EQ(P.asPlainText(), ""); + + P.appendBoldText(" \n foo\nbar\n "); + EXPECT_EQ(P.asMarkdown(), "**foo bar**"); + EXPECT_EQ(P.asPlainText(), "**foo bar**"); + + P.appendSpace().appendBoldText("foobar"); + EXPECT_EQ(P.asMarkdown(), "**foo bar** **foobar**"); + EXPECT_EQ(P.asPlainText(), "**foo bar** **foobar**"); +} + +TEST(Paragraph, EmphasizedText) { + Paragraph P; + P.appendEmphasizedText(""); + EXPECT_EQ(P.asMarkdown(), ""); + EXPECT_EQ(P.asPlainText(), ""); + + P.appendEmphasizedText(" \n foo\nbar\n "); + EXPECT_EQ(P.asMarkdown(), "*foo bar*"); + EXPECT_EQ(P.asPlainText(), "*foo bar*"); + + P.appendSpace().appendEmphasizedText("foobar"); + EXPECT_EQ(P.asMarkdown(), "*foo bar* *foobar*"); + EXPECT_EQ(P.asPlainText(), "*foo bar* *foobar*"); +} + TEST(Document, Separators) { Document D; D.addParagraph().appendText("foo"); D.addCodeBlock("test"); D.addParagraph().appendText("bar"); - const char ExpectedMarkdown[] = R"md(foo + const char ExpectedMarkdown[] = R"md(foo + ```cpp test ``` @@ -238,7 +341,7 @@ TEST(Document, Ruler) { // Ruler followed by paragraph. D.addParagraph().appendText("bar"); - EXPECT_EQ(D.asMarkdown(), "foo \n\n---\nbar"); + EXPECT_EQ(D.asMarkdown(), "foo\n\n---\nbar"); EXPECT_EQ(D.asPlainText(), "foo\n\nbar"); D = Document(); @@ -246,7 +349,7 @@ TEST(Document, Ruler) { D.addRuler(); D.addCodeBlock("bar"); // Ruler followed by a codeblock. - EXPECT_EQ(D.asMarkdown(), "foo \n\n---\n```cpp\nbar\n```"); + EXPECT_EQ(D.asMarkdown(), "foo\n\n---\n```cpp\nbar\n```"); EXPECT_EQ(D.asPlainText(), "foo\n\nbar"); // Ruler followed by another ruler @@ -260,7 +363,7 @@ TEST(Document, Ruler) { // Multiple rulers between blocks D.addRuler(); D.addParagraph().appendText("foo"); - EXPECT_EQ(D.asMarkdown(), "foo \n\n---\nfoo"); + EXPECT_EQ(D.asMarkdown(), "foo\n\n---\nfoo"); EXPECT_EQ(D.asPlainText(), "foo\n\nfoo"); } @@ -272,7 +375,7 @@ TEST(Document, Append) { E.addRuler(); E.addParagraph().appendText("bar"); D.append(std::move(E)); - EXPECT_EQ(D.asMarkdown(), "foo \n\n---\nbar"); + EXPECT_EQ(D.asMarkdown(), "foo\n\n---\nbar"); } TEST(Document, Heading) { @@ -280,8 +383,8 @@ TEST(Document, Heading) { D.addHeading(1).appendText("foo"); D.addHeading(2).appendText("bar"); D.addParagraph().appendText("baz"); - EXPECT_EQ(D.asMarkdown(), "# foo \n## bar \nbaz"); - EXPECT_EQ(D.asPlainText(), "foo\nbar\nbaz"); + EXPECT_EQ(D.asMarkdown(), "# foo\n\n## bar\n\nbaz"); + EXPECT_EQ(D.asPlainText(), "foo\n\nbar\n\nbaz"); } TEST(CodeBlock, Render) { @@ -336,7 +439,7 @@ TEST(BulletList, Render) { // Nested list, with a single item. Document &D = L.addItem(); - // First item with foo\nbaz + // First item with 2 paragraphs - foo\n\n baz D.addParagraph().appendText("foo"); D.addParagraph().appendText("baz"); @@ -352,18 +455,26 @@ TEST(BulletList, Render) { DeepDoc.addParagraph().appendText("baz"); StringRef ExpectedMarkdown = R"md(- foo - bar -- foo - baz - - foo - - baz +- foo + + baz + + - foo + + - baz + baz)md"; EXPECT_EQ(L.asMarkdown(), ExpectedMarkdown); StringRef ExpectedPlainText = R"pt(- foo - bar - foo + baz + - foo + - baz + baz)pt"; EXPECT_EQ(L.asPlainText(), ExpectedPlainText); @@ -371,21 +482,30 @@ TEST(BulletList, Render) { Inner.addParagraph().appendText("after"); ExpectedMarkdown = R"md(- foo - bar -- foo - baz - - foo - - baz +- foo + + baz + + - foo + + - baz + baz - + after)md"; EXPECT_EQ(L.asMarkdown(), ExpectedMarkdown); ExpectedPlainText = R"pt(- foo - bar - foo + baz + - foo + - baz + baz + after)pt"; EXPECT_EQ(L.asPlainText(), ExpectedPlainText); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits