sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
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)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79139
Files:
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
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ 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`",
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ 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) {
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ 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) {
Index: clang-tools-extra/clangd/FormattedString.h
===================================================================
--- clang-tools-extra/clangd/FormattedString.h
+++ 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<Chunk> Chunks;
};
Index: clang-tools-extra/clangd/FormattedString.cpp
===================================================================
--- clang-tools-extra/clangd/FormattedString.cpp
+++ 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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
