juliehockett added inline comments.
================ Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:87 + std::string CloseTag = I.SelfClosing ? "/>" : ">"; + writeLine("<" + I.Name + Attrs.str() + CloseTag, OS); + } else if (I.Kind == "HTMLEndTagComment") { ---------------- You shouldn't be using writeLine here, since it wraps the tag in an extra <p> element (which is okay for all the other pieces, but is weird on a specified HTML element. ================ Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:89 + } else if (I.Kind == "HTMLEndTagComment") { + writeLine("</" + I.Name + ">", OS); + } else if (I.Kind == "TextComment") { ---------------- Same as above ================ Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:110 + for (const auto &N : I.Members) + Members << "| " << N << " |\n"; + writeLine(Members.str(), OS); ---------------- The pipes don't mean anything in HTML, so you can remove them. ================ Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:214-218 + HTML->Children.back()->Kind = "HTMLStartTagComment"; + HTML->Children.back()->Name = "li"; + HTML->Children.emplace_back(llvm::make_unique<CommentInfo>()); + HTML->Children.back()->Kind = "TextComment"; + HTML->Children.back()->Text = " Testing."; ---------------- For the sake of vaguely valid HTML in the test output, it'd be nice if we had an end tag for the <li> element as well. ================ Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:287 +<p><em>Defined at line 10 of test.cpp</em></p> +<br> + Brief description.<br> ---------------- Should the whole comment be wrapped in a <p> element? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63180/new/ https://reviews.llvm.org/D63180 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits