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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits