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 @@ -54,6 +54,10 @@ /// \p Preserve indicates the code span must be apparent even in plaintext. Paragraph &appendCode(llvm::StringRef Code, bool Preserve = false); + /// 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 { @@ -63,6 +67,11 @@ // Preserve chunk markers in plaintext. bool Preserve = false; std::string Contents; + // Whether this chunk should be surrounded by whitespace. + // Consecutive SpaceAfter and SpaceBefore will be collapsed into one space. + // Code spans don't usually set this: their spaces belong "inside" the span. + bool SpaceBefore = false; + bool 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 @@ -346,18 +346,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. @@ -381,13 +384,15 @@ } void Paragraph::renderPlainText(llvm::raw_ostream &OS) const { - llvm::StringRef Sep = ""; + bool NeedsSpace = false; for (auto &C : Chunks) { + if (C.SpaceBefore || NeedsSpace) + OS << " "; llvm::StringRef Marker = ""; if (C.Preserve && C.Kind == Chunk::InlineCode) Marker = chooseMarker({"`", "'", "\""}, C.Contents); - OS << Sep << Marker << C.Contents << Marker; - Sep = " "; + OS << Marker << C.Contents << Marker; + NeedsSpace = C.SpaceAfter; } OS << '\n'; } @@ -410,6 +415,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()) @@ -418,10 +429,14 @@ 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; } Paragraph &Paragraph::appendCode(llvm::StringRef Code, bool Preserve) { + bool AdjacentCode = + !Chunks.empty() && Chunks.back().Kind == Chunk::InlineCode; std::string Norm = canonicalizeSpaces(std::move(Code)); if (Norm.empty()) return *this; @@ -430,6 +445,8 @@ C.Contents = std::move(Norm); C.Kind = Chunk::InlineCode; C.Preserve = Preserve; + // Disallow adjacent code spans without spaces, markdown can't render them. + C.SpaceBefore = AdjacentCode; 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 @@ -773,7 +773,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); @@ -787,9 +787,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; @@ -804,7 +804,7 @@ if (Value) { markup::Paragraph &P = Output.addParagraph(); - P.appendText("Value ="); + P.appendText("Value = "); P.appendCode(*Value); } @@ -883,7 +883,7 @@ break; } } - Out.appendText(Line); + Out.appendText(Line).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 @@ Paragraph P = Paragraph(); P.appendText("One "); P.appendCode("fish"); - P.appendText(", two"); + P.appendText(", two "); P.appendCode("fish", /*Preserve=*/true); - EXPECT_EQ(P.asMarkdown(), "One `fish` , two `fish`"); - EXPECT_EQ(P.asPlainText(), "One fish , two `fish`"); + EXPECT_EQ(P.asMarkdown(), "One `fish`, two `fish`"); + EXPECT_EQ(P.asPlainText(), "One fish, two `fish`"); } TEST(Paragraph, SeparationOfChunks) { @@ -168,17 +168,21 @@ // 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"); P.appendText("bat"); EXPECT_EQ(P.asMarkdown(), "after `foobar` bat"); EXPECT_EQ(P.asPlainText(), "after foobar bat"); + + P.appendCode("no").appendCode("space"); + EXPECT_EQ(P.asMarkdown(), "after `foobar` bat`no` `space`"); + EXPECT_EQ(P.asPlainText(), "after foobar batno space"); } TEST(Paragraph, ExtraSpaces) { @@ -186,8 +190,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 @@ -2019,10 +2019,9 @@ "foo bar", }, { - // 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`",