sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:164
}
- return R;
+ OS << '\n';
}
----------------
this is worth a comment - we translate Paragraphs into markdown lines, not
markdown paragraphs
================
Comment at: clang-tools-extra/clangd/FormattedString.cpp:199
+Paragraph &Document::addParagraph() {
+ Children.emplace_back(std::make_unique<Paragraph>());
+ return *static_cast<Paragraph *>(Children.back().get());
----------------
push_back (and below)
================
Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:21
-TEST(FormattedString, Basic) {
- FormattedString S;
- EXPECT_EQ(S.renderAsPlainText(), "");
- EXPECT_EQ(S.renderAsMarkdown(), "");
+enum RenderKind {
+ PlainText,
----------------
dead?
================
Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:79
+ {
+ Test::PlainText,
+ "after",
----------------
I'd consider writing this as
`[&] { Para.appendText("after"); }` to make it clearer what's being tested.
Are you sure the table test is clearer than straight-line code incrementally
building the paragrap/asserting here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71248/new/
https://reviews.llvm.org/D71248
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
