[PATCH] D63857: [clang-doc] Structured HTML generator

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:21
 
+class HTMLTag {
+public:
----------------
Put the class definitions in an anonymous namespace (https://llvm.org/docs/CodingStandards.html#anonymous-namespaces)


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:24
+  // Any other tag can be added if required
+  enum Tag {
+    meta,
----------------
Use `TagType` or some such to avoid confusion with the containing class


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:25-33
+    meta,
+    title,
+    div,
+    h1,
+    h2,
+    h3,
+    p,
----------------
Enum values should be capitalized, and probably prefixed with HTML_ or TAG_ or some such useful prefix (e.g. TAG_META)


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:42
+
+  bool IsSelfClosing() const {
+    switch (Value) {
----------------
For this and other functions, separate the implementation from the definition.


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:121
+struct TagNode : public HTMLNode {
+  TagNode(HTMLTag Tag) : Tag(Tag) {
+    InlineChildren = Tag.HasInlineChildren();
----------------
```TagNode(HTMLTag Tag) : Tag(Tag), InlineChildren(Tag.HasInlineChildren), SelfClosing(Tag.SelfClosing) {}```


================
Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:412
+
+  F.Children.push_back(llvm::make_unique<TagNode>(HTMLTag::title, InfoTitle));
+  F.Children.push_back(std::move(MainContentNode));
----------------
use `emplace_back` here for instantiation


================
Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:31
 
-static void writeLine(const Twine &Text, raw_ostream &OS) {
+void writeLine(const Twine &Text, raw_ostream &OS) {
   OS << Text << "\n\n";
----------------
Can you re-static these?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63857/new/

https://reviews.llvm.org/D63857