[clang-tools-extra] ec170b7 - [clangd] Fix whitespace between chunks in markdown paragraphs.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Sat May 2 05:54:16 PDT 2020
Author: Sam McCall
Date: 2020-05-02T14:39:54+02:00
New Revision: ec170b7ccd5bf7aa05dea80e6f246964fd081e98
URL: https://github.com/llvm/llvm-project/commit/ec170b7ccd5bf7aa05dea80e6f246964fd081e98
DIFF: https://github.com/llvm/llvm-project/commit/ec170b7ccd5bf7aa05dea80e6f246964fd081e98.diff
LOG: [clangd] Fix whitespace between chunks in markdown paragraphs.
Summary:
Old model: chunks are always separated by one space.
This makes it impossible to render "Foo `bar`." correctly.
New model: chunks are separated by space if the left had trailing space, or
the right had leading space, or space was explicitly requested.
(Only leading/trailing space in plaintext chunks count, not code)
Reviewers: kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D79139
Added:
Modified:
clang-tools-extra/clangd/FormattedString.cpp
clang-tools-extra/clangd/FormattedString.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/FormattedString.cpp b/clang-tools-extra/clangd/FormattedString.cpp
index 309cb69a2bfc..ae6eb1e31354 100644
--- a/clang-tools-extra/clangd/FormattedString.cpp
+++ b/clang-tools-extra/clangd/FormattedString.cpp
@@ -346,18 +346,21 @@ std::string Block::asPlainText() const {
}
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 @@ llvm::StringRef chooseMarker(llvm::ArrayRef<llvm::StringRef> Options,
}
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 @@ void BulletList::renderPlainText(llvm::raw_ostream &OS) const {
}
}
+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 @@ Paragraph &Paragraph::appendText(llvm::StringRef Text) {
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 @@ Paragraph &Paragraph::appendCode(llvm::StringRef Code, bool Preserve) {
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/FormattedString.h b/clang-tools-extra/clangd/FormattedString.h
index 295b22db8dab..06a8fd9052e6 100644
--- a/clang-tools-extra/clangd/FormattedString.h
+++ b/clang-tools-extra/clangd/FormattedString.h
@@ -54,6 +54,10 @@ class Paragraph : public Block {
/// \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 @@ class Paragraph : public Block {
// 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<Chunk> Chunks;
};
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index b563273733be..5e92ee189035 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -773,7 +773,7 @@ markup::Document HoverInfo::present() const {
// 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 @@ markup::Document HoverInfo::present() const {
// 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 @@ markup::Document HoverInfo::present() const {
if (Value) {
markup::Paragraph &P = Output.addParagraph();
- P.appendText("Value =");
+ P.appendText("Value = ");
P.appendCode(*Value);
}
@@ -883,7 +883,7 @@ void parseDocumentationLine(llvm::StringRef Line, markup::Paragraph &Out) {
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
index 63cbf3dac9d7..87df6396becc 100644
--- a/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -155,11 +155,11 @@ TEST(Paragraph, Chunks) {
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 @@ TEST(Paragraph, SeparationOfChunks) {
// Purpose is to check for separation between
diff erent 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 @@ TEST(Paragraph, ExtraSpaces) {
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
index c8689b7b9183..04842ba8d141 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2019,10 +2019,9 @@ TEST(Hover, ParseDocumentation) {
"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`",
More information about the cfe-commits
mailing list