https://github.com/tcottin updated https://github.com/llvm/llvm-project/pull/185197
>From d9714b7edf3b56ace9ed90aba3afb66891f7673f Mon Sep 17 00:00:00 2001 From: Tim Cottin <[email protected]> Date: Sun, 8 Mar 2026 12:37:10 +0000 Subject: [PATCH] use plaintext newline handling for escaped markdown hover style --- clang-tools-extra/clangd/support/Markup.cpp | 31 +++++++++++++++---- clang-tools-extra/clangd/support/Markup.h | 15 ++++++--- .../clangd/unittests/HoverTests.cpp | 28 ++++++++--------- .../unittests/SymbolDocumentationTests.cpp | 11 +++---- .../clangd/unittests/support/MarkupTests.cpp | 25 +++++++-------- 5 files changed, 65 insertions(+), 45 deletions(-) diff --git a/clang-tools-extra/clangd/support/Markup.cpp b/clang-tools-extra/clangd/support/Markup.cpp index 9ba993a04709c..e65263e6d1f7c 100644 --- a/clang-tools-extra/clangd/support/Markup.cpp +++ b/clang-tools-extra/clangd/support/Markup.cpp @@ -491,7 +491,12 @@ void Paragraph::renderNewlinesMarkdown(llvm::raw_ostream &OS, OS << Line; - if (!Rest.empty() && isHardLineBreakAfter(Line, Rest, /*IsMarkdown=*/true)) + if (Rest.empty()) + // In this case the paragraph ends and we don't need to add another + // newline + continue; + + if (isHardLineBreakAfter(Line, Rest, /*IsMarkdown=*/true)) // In markdown, 2 spaces before a line break forces a line break. OS << " "; OS << '\n'; @@ -528,7 +533,11 @@ void Paragraph::renderEscapedMarkdown(llvm::raw_ostream &OS) const { NeedsSpace = C.SpaceAfter; } - renderNewlinesMarkdown(OS, ParagraphText); + // We use the same newline handling as for plaintext to "escape" markdown + // whitespace rendering. + // But we need to use Markdown linebreaks since the client requested Markdown + // content. + renderNewlinesPlaintext(OS, ParagraphText, /*UseMarkdownLinebreaks=*/true); // A paragraph in markdown is separated by a blank line. OS << "\n\n"; @@ -635,7 +644,8 @@ bool Paragraph::isHardLineBreakAfter(llvm::StringRef Line, llvm::StringRef Rest, } void Paragraph::renderNewlinesPlaintext(llvm::raw_ostream &OS, - llvm::StringRef ParagraphText) const { + llvm::StringRef ParagraphText, + bool UseMarkdownLinebreaks) const { llvm::StringRef Line, Rest; for (std::tie(Line, Rest) = ParagraphText.trim().split('\n'); @@ -659,13 +669,22 @@ void Paragraph::renderNewlinesPlaintext(llvm::raw_ostream &OS, OS << canonicalizeSpaces(Line); - if (isHardLineBreakAfter(Line, Rest, /*IsMarkdown=*/false)) + if (Rest.empty()) + // In this case the paragraph ends and we don't need to add another + // newline + continue; + + if (isHardLineBreakAfter(Line, Rest, /*IsMarkdown=*/false)) { + if (UseMarkdownLinebreaks) + // In markdown, 2 spaces before a line break forces a line break. + OS << " "; OS << '\n'; - else if (!Rest.empty()) + } else { // Since we removed any trailing whitespace from the input using trim(), // we know that the next line contains non-whitespace characters. // Therefore, we can add a space without worrying about trailing spaces. OS << ' '; + } } } @@ -700,7 +719,7 @@ void Paragraph::renderPlainText(llvm::raw_ostream &OS) const { NeedsSpace = C.SpaceAfter; } - renderNewlinesPlaintext(OS, ParagraphText); + renderNewlinesPlaintext(OS, ParagraphText, /*UseMarkdownLinebreaks=*/false); // Paragraphs are separated by a blank line. OS << "\n\n"; diff --git a/clang-tools-extra/clangd/support/Markup.h b/clang-tools-extra/clangd/support/Markup.h index 219a7dad1e175..bf22abd82622c 100644 --- a/clang-tools-extra/clangd/support/Markup.h +++ b/clang-tools-extra/clangd/support/Markup.h @@ -161,15 +161,20 @@ class Paragraph : public Block { /// /// Lines containing only whitespace are ignored. /// - /// This newline handling is only used when the client requests plain - /// text for hover/signature help content. - /// Therefore with this approach we mimic the behavior of markdown rendering - /// for these clients. + /// This newline handling is used when the server is configured to provide + /// Plaintext hover content. + /// + /// In case the client requests Markdown content, the newline "character" + /// needs to be a Markdown line break (two spaces followed by `\n`). + /// Otherwise the linebreak will not be rendered correctly. + /// This can be controlled by the \p UseMarkdownLinebreaks parameter. /// /// \param OS The stream to render to. /// \param ParagraphText The text of the paragraph to render. + /// \param UseMarkdownLinebreaks Indicates whether to use markdown linebreaks void renderNewlinesPlaintext(llvm::raw_ostream &OS, - llvm::StringRef ParagraphText) const; + llvm::StringRef ParagraphText, + bool UseMarkdownLinebreaks) const; }; /// Represents a sequence of one or more documents. Knows how to print them in a diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp index 7bff20e6f5635..aeff0206a01e0 100644 --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -4362,13 +4362,13 @@ TEST(Hover, ParseDocumentation) { llvm::StringRef ExpectedRenderPlainText; } Cases[] = {{ " \n foo\nbar", - "foo\nbar", + "foo bar", "foo\nbar", "foo bar", }, { "foo\nbar \n ", - "foo\nbar", + "foo bar", "foo\nbar", "foo bar", }, @@ -4380,7 +4380,7 @@ TEST(Hover, ParseDocumentation) { }, { "foo \nbar", - "foo \nbar", + "foo \nbar", "foo \nbar", "foo\nbar", }, @@ -4392,25 +4392,25 @@ TEST(Hover, ParseDocumentation) { }, { "foo\n\n\n\tbar", - "foo\n\n\tbar", + "foo\n\nbar", "foo\n\n\tbar", "foo\n\nbar", }, { "foo\n\n\n bar", - "foo\n\n bar", + "foo\n\nbar", "foo\n\n bar", "foo\n\nbar", }, { "foo\n\n\n bar", - "foo\n\n bar", + "foo\n\nbar", "foo\n\n bar", "foo\n\nbar", }, { "foo\n\n\n bar", - "foo\n\n bar", + "foo\n\nbar", "foo\n\n bar", "foo\n\nbar", }, @@ -4422,25 +4422,25 @@ TEST(Hover, ParseDocumentation) { }, { "foo\n\n\n\n\tbar", - "foo\n\n\tbar", + "foo\n\nbar", "foo\n\n\tbar", "foo\n\nbar", }, { "foo\n\n\n\n bar", - "foo\n\n bar", + "foo\n\nbar", "foo\n\n bar", "foo\n\nbar", }, { "foo\n\n\n\n bar", - "foo\n\n bar", + "foo\n\nbar", "foo\n\n bar", "foo\n\nbar", }, { "foo\n\n\n\n bar", - "foo\n\n bar", + "foo\n\nbar", "foo\n\n bar", "foo\n\nbar", }, @@ -4452,7 +4452,7 @@ TEST(Hover, ParseDocumentation) { }, { "foo. \nbar", - "foo. \nbar", + "foo. \nbar", "foo. \nbar", "foo.\nbar", }, @@ -4464,7 +4464,7 @@ TEST(Hover, ParseDocumentation) { }, { "foo\nbar", - "foo\nbar", + "foo bar", "foo\nbar", "foo bar", }, @@ -4482,7 +4482,7 @@ TEST(Hover, ParseDocumentation) { }, { "`not\nparsed`", - "\\`not\nparsed\\`", + "\\`not parsed\\`", "`not\nparsed`", "`not parsed`", }, diff --git a/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp b/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp index 676f7dfc74483..236ac1d3ccc5c 100644 --- a/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolDocumentationTests.cpp @@ -33,7 +33,7 @@ TEST(SymbolDocumentation, DetailedDocToMarkup) { }, { "brief\n\nfoo\nbar\n", - "foo\nbar", + "foo bar", "foo\nbar", "foo bar", }, @@ -174,8 +174,7 @@ documentation)", these are details -More description -documentation)", +More description documentation)", R"(**\brief** another brief? these are details @@ -194,8 +193,7 @@ More description documentation)", <b>this is a bold text</b> normal text<i>this is an italic text</i> <code>this is a code block</code>)", - R"(\<b>this is a bold text\</b> -normal text\<i>this is an italic text\</i> + R"(\<b>this is a bold text\</b> normal text\<i>this is an italic text\</i> \<code>this is a code block\</code>)", R"(\<b>this is a bold text\</b> normal text\<i>this is an italic text\</i> @@ -711,8 +709,7 @@ TEST(SymbolDocumentation, MarkdownCodeSpans) { {R"(`multi line \c span`)", - R"(\`multi -line + R"(\`multi line \\c span\`)", R"(`multi line diff --git a/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp b/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp index af4782c07ae52..59746c6f3dcab 100644 --- a/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp +++ b/clang-tools-extra/clangd/unittests/support/MarkupTests.cpp @@ -304,7 +304,7 @@ TEST(Paragraph, SeparationOfChunks) { P.appendSpace().appendCode("code").appendText(".\n newline"); EXPECT_EQ(P.asEscapedMarkdown(), - "after `foobar` bat`no` `space` text `code`. \n newline"); + "after `foobar` bat`no` `space` text `code`. \nnewline"); EXPECT_EQ(P.asMarkdown(), "after `foobar` bat`no` `space` text `code`. \n newline"); EXPECT_EQ(P.asPlainText(), "after foobar batno space text code.\nnewline"); @@ -343,12 +343,12 @@ TEST(Paragraph, SeparationOfChunks2) { EXPECT_EQ(P.asPlainText(), "after foobar batbaz faz"); P.appendText(" bar "); - EXPECT_EQ(P.asEscapedMarkdown(), "after foobar batbaz faz bar"); + EXPECT_EQ(P.asEscapedMarkdown(), "after foobar batbaz faz 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.asEscapedMarkdown(), "after foobar batbaz faz bar qux"); + EXPECT_EQ(P.asEscapedMarkdown(), "after foobar batbaz faz bar qux"); EXPECT_EQ(P.asMarkdown(), "after foobar batbaz faz bar qux"); EXPECT_EQ(P.asPlainText(), "after foobar batbaz faz bar qux"); } @@ -366,23 +366,22 @@ TEST(Paragraph, SeparationOfChunks3) { EXPECT_EQ(P.asPlainText(), "after"); P.appendText(" foobar\n"); - EXPECT_EQ(P.asEscapedMarkdown(), "after \n foobar"); + EXPECT_EQ(P.asEscapedMarkdown(), "after \nfoobar"); EXPECT_EQ(P.asMarkdown(), "after \n foobar"); EXPECT_EQ(P.asPlainText(), "after\nfoobar"); P.appendText("- bat\n"); - EXPECT_EQ(P.asEscapedMarkdown(), "after \n foobar \n\\- bat"); + EXPECT_EQ(P.asEscapedMarkdown(), "after \nfoobar \n\\- bat"); EXPECT_EQ(P.asMarkdown(), "after \n foobar\n- bat"); EXPECT_EQ(P.asPlainText(), "after\nfoobar\n- bat"); P.appendText("- baz"); - EXPECT_EQ(P.asEscapedMarkdown(), "after \n foobar \n\\- bat \n\\- baz"); + EXPECT_EQ(P.asEscapedMarkdown(), "after \nfoobar \n\\- bat \n\\- 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.asEscapedMarkdown(), - "after \n foobar \n\\- bat \n\\- baz faz"); + EXPECT_EQ(P.asEscapedMarkdown(), "after \nfoobar \n\\- bat \n\\- baz faz"); EXPECT_EQ(P.asMarkdown(), "after \n foobar\n- bat\n- baz faz"); EXPECT_EQ(P.asPlainText(), "after\nfoobar\n- bat\n- baz faz"); } @@ -461,8 +460,8 @@ TEST(Paragraph, LineBreakIndicators) { "Visual linebreak for\n>blockquoute line 1\n> blockquoute line 2"}, {"Visual linebreak for\n# Heading 1\ntext under heading\n## Heading " "2\ntext under heading 2", - "Visual linebreak for \n\\# Heading 1\ntext under heading \n\\## " - "Heading 2\ntext under heading 2", + "Visual linebreak for \n\\# Heading 1 text under heading \n\\## " + "Heading 2 text under heading 2", "Visual linebreak for\n# Heading 1\ntext under heading\n## Heading " "2\ntext under heading 2", "Visual linebreak for\n# Heading 1 text under heading\n## Heading 2 " @@ -488,7 +487,7 @@ TEST(Paragraph, ExtraSpaces) { Paragraph P; P.appendText("foo\n \t baz"); P.appendCode(" bar\n"); - EXPECT_EQ(P.asEscapedMarkdown(), "foo\n \t baz`bar`"); + EXPECT_EQ(P.asEscapedMarkdown(), "foo baz`bar`"); EXPECT_EQ(P.asMarkdown(), "foo\n \t baz`bar`"); EXPECT_EQ(P.asPlainText(), "foo bazbar"); } @@ -497,7 +496,7 @@ TEST(Paragraph, SpacesCollapsed) { Paragraph P; P.appendText(" foo bar "); P.appendText(" baz "); - EXPECT_EQ(P.asEscapedMarkdown(), "foo bar baz"); + EXPECT_EQ(P.asEscapedMarkdown(), "foo bar baz"); EXPECT_EQ(P.asMarkdown(), "foo bar baz"); EXPECT_EQ(P.asPlainText(), "foo bar baz"); } @@ -507,7 +506,7 @@ TEST(Paragraph, NewLines) { Paragraph P; P.appendText(" \n foo\nbar\n "); P.appendCode(" \n foo\nbar \n "); - EXPECT_EQ(P.asEscapedMarkdown(), "foo\nbar\n `foo bar`"); + EXPECT_EQ(P.asEscapedMarkdown(), "foo bar \n`foo bar`"); EXPECT_EQ(P.asMarkdown(), "foo\nbar\n `foo bar`"); EXPECT_EQ(P.asPlainText(), "foo bar foo bar"); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
