[PATCH] D63180: [clang-doc] Adds HTML generator

Julie Hockett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 21 10:05:41 PDT 2019


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





More information about the cfe-commits mailing list