[PATCH] D18278: llvm-cov HTML Generation

[Matthias Braun via llvm-commits]

MatzeB added a subscriber: MatzeB.
MatzeB added a comment.

Definitely a nice feature to have (I assume we can replace http://llvm.org/reports/coverage/ with this at some point).

I didn't have time to actually try it out, so no comments on the looks of the generated pages here.

While I like the fact that a stylesheet is hardcoded by default, I can imagine some users wanting to tweak it. Adding an option that generates a `<link rel="stylesheet"...` instead of outputting the hardcoded stylesheet should be easy.
You could address this in followup patches though.


================
Comment at: tools/llvm-cov/HTMLCoverage.inc:1
@@ +1,2 @@
+//===- HTMLCoverage.inc - Helper definitions for html output --------------===//
+//
----------------
Using a .inc file feels slightly unusual for llvm. Why not call this HTMLCoverage.h or alternatively simply copy the contents to the beginning of HTMLCoverageViewHTML.cpp...

================
Comment at: tools/llvm-cov/SourceCoverageViewHTML.cpp:57
@@ +56,3 @@
+// Create a tag around the provided text.
+std::string tag(std::string Name, std::string Str, std::string ClassName = "") {
+  std::string Tag = "<" + Name;
----------------
Passing the parameters by reference `const std::string&` or alternatively `StringRef` would avoid some unnecessary allocating/copying of strings.
The same applies to some other functions here.


http://reviews.llvm.org/D18278