sammccall updated this revision to Diff 261399.
sammccall marked an inline comment as done.
sammccall added a comment.
tweak coment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79139/new/
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
@@ -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`",
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 @@
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) {
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ 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) {
Index: clang-tools-extra/clangd/FormattedString.h
===================================================================
--- clang-tools-extra/clangd/FormattedString.h
+++ 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<Chunk> Chunks;
};
Index: clang-tools-extra/clangd/FormattedString.cpp
===================================================================
--- clang-tools-extra/clangd/FormattedString.cpp
+++ 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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
