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 @@ -51,6 +51,10 @@ /// Append inline code, this translates to the ` block in markdown. Paragraph &appendCode(llvm::StringRef Code); + /// Ensure there is space between the surrounding chunks. + /// Has no effect at the beginning or end of a paragraph. + Paragraph &appendSpace(); + private: struct Chunk { enum { @@ -60,6 +64,10 @@ std::string Contents; /// Language for code block chunks. Ignored for other chunks. std::string Language; + // Whether this chunk should be surrounded by whitespace. + // Consecutive SpaceAfter and SpaceBefore will be collapsed into one space. + // Code spans don't set this: their spaces belong "inside" the span. + bool SpaceBefore = false, SpaceAfter = false; }; std::vector Chunks; }; 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 @@ -339,18 +339,21 @@ } void Paragraph::renderMarkdown(llvm::raw_ostream &OS) const { - llvm::StringRef Sep = ""; + bool NeedsSpace = false; + bool HasChunks = false; for (auto &C : Chunks) { - OS << Sep; + if (C.SpaceBefore || NeedsSpace) + OS << " "; switch (C.Kind) { case Chunk::PlainText: - OS << renderText(C.Contents, Sep.empty()); + OS << renderText(C.Contents, !HasChunks); break; case Chunk::InlineCode: OS << renderInlineBlock(C.Contents); break; } - Sep = " "; + HasChunks = true; + NeedsSpace = C.SpaceAfter; } // Paragraphs are translated into markdown lines, not markdown paragraphs. // Therefore it only has a single linebreak afterwards. @@ -359,10 +362,12 @@ } void Paragraph::renderPlainText(llvm::raw_ostream &OS) const { - llvm::StringRef Sep = ""; + bool NeedsSpace = false; for (auto &C : Chunks) { - OS << Sep << C.Contents; - Sep = " "; + if (C.SpaceBefore || NeedsSpace) + OS << " "; + OS << C.Contents; + NeedsSpace = C.SpaceAfter; } OS << '\n'; } @@ -385,6 +390,12 @@ } } +Paragraph &Paragraph::appendSpace() { + if (!Chunks.empty()) + Chunks.back().SpaceAfter = true; + return *this; +} + Paragraph &Paragraph::appendText(llvm::StringRef Text) { std::string Norm = canonicalizeSpaces(Text); if (Norm.empty()) @@ -393,6 +404,8 @@ Chunk &C = Chunks.back(); C.Contents = std::move(Norm); C.Kind = Chunk::PlainText; + C.SpaceBefore = isWhitespace(Text.front()); + C.SpaceAfter = isWhitespace(Text.back()); return *this; } 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 @@ -772,7 +772,7 @@ // https://github.com/microsoft/vscode/issues/88417 for details. markup::Paragraph &Header = Output.addHeading(3); if (Kind != index::SymbolKind::Unknown) - Header.appendText(index::getSymbolKindString(Kind)); + Header.appendText(index::getSymbolKindString(Kind)).appendSpace(); assert(!Name.empty() && "hover triggered on a nameless symbol"); Header.appendCode(Name); @@ -786,9 +786,9 @@ // Parameters: // - `bool param1` // - `int param2 = 5` - Output.addParagraph().appendText("→").appendCode(*ReturnType); + Output.addParagraph().appendText("→ ").appendCode(*ReturnType); if (Parameters && !Parameters->empty()) { - Output.addParagraph().appendText("Parameters:"); + Output.addParagraph().appendText("Parameters: "); markup::BulletList &L = Output.addBulletList(); for (const auto &Param : *Parameters) { std::string Buffer; @@ -803,7 +803,7 @@ if (Value) { markup::Paragraph &P = Output.addParagraph(); - P.appendText("Value ="); + P.appendText("Value = "); P.appendCode(*Value); } @@ -883,6 +883,7 @@ } } Out.appendText(Line); + Out.appendSpace(); } void parseDocumentation(llvm::StringRef Input, markup::Document &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 @@ -155,11 +155,11 @@ // Purpose is to check for separation between different chunks. Paragraph P; - P.appendText("after"); + P.appendText("after "); EXPECT_EQ(P.asMarkdown(), "after"); EXPECT_EQ(P.asPlainText(), "after"); - P.appendCode("foobar"); + P.appendCode("foobar").appendSpace(); EXPECT_EQ(P.asMarkdown(), "after `foobar`"); EXPECT_EQ(P.asPlainText(), "after foobar"); @@ -173,8 +173,16 @@ Paragraph P; P.appendText("foo\n \t baz"); P.appendCode(" bar\n"); - EXPECT_EQ(P.asMarkdown(), "foo baz `bar`"); - EXPECT_EQ(P.asPlainText(), "foo baz bar"); + EXPECT_EQ(P.asMarkdown(), "foo baz`bar`"); + EXPECT_EQ(P.asPlainText(), "foo bazbar"); +} + +TEST(Paragraph, SpacesCollapsed) { + Paragraph P; + P.appendText(" foo bar "); + P.appendText(" baz "); + EXPECT_EQ(P.asMarkdown(), "foo bar baz"); + EXPECT_EQ(P.asPlainText(), "foo bar baz"); } TEST(Paragraph, NewLines) { diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -2021,8 +2021,8 @@ { // FIXME: we insert spaces between code and text chunk. "Tests primality of `p`.", - "Tests primality of `p` .", - "Tests primality of p .", + "Tests primality of `p`.", + "Tests primality of p.", }, { "'`' should not occur in `Code`",