sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/FormattedString.h:25
/// plaintext upon requrest.
-class FormattedString {
+class RenderableString {
public:
----------------
Naming: not sure "string" is the right name - it doesn't represent the things
here that are stringiest.
Block? (maybe in a `markup` namespace?)
================
Comment at: clang-tools-extra/clangd/FormattedString.h:34
+
+/// Leaf type of the struct representation.
+class Paragraph : public RenderableString {
----------------
Comments throughout should probably refer to the semantics (what part of markup
is this) rather than its role in the tree.
================
Comment at: clang-tools-extra/clangd/FormattedString.h:67
+/// Container for a set of documents. Each document is prepended with a "- "
and
+/// separated by newlines.
----------------
"List" and "Container" commonly mean other things in C++, so I got confused.
`BulletList`?
The comment "Container for a list of documents" describes its structure in the
tree rather than its semantics, which most callers (markup composers) will care
about. Maybe "A bulleted list. Each item is a child Document"?
I think the comment "each document is prepended..." belongs on
renderAsPlainText() if at all
================
Comment at: clang-tools-extra/clangd/FormattedString.h:70
+class List : public RenderableString {
+public:
+ std::string renderAsMarkdown() const override;
----------------
obvious attribute here is bullet style (bullets vs numbers) - but you probably
only need one for now
================
Comment at: clang-tools-extra/clangd/FormattedString.h:82
+/// Top-level container for structured strings.
+class Document : public RenderableString {
+public:
----------------
Document doesn't need to inherit from RenderableString AFAICS, it doesn't need
to be embedded in Documents itself, just Lists.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71063/new/
https://reviews.llvm.org/D71063
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
