[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 12 03:59:30 PST 2019
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
More information about the cfe-commits
mailing list