[PATCH] D23486: [llvm-cov] Add coverage summary information to the HTML index page.

Sun Aug 14 11:52:00 PDT 2016

vsk added a comment.

This is the right way to update a review :).


================
Comment at: SourceCoverageViewHTML.cpp:213
@@ -211,1 +212,3 @@
+                 const std::string &PathToStyle = "",
+                 bool trAlternate = false) {
   OS << "<!doctype html>"
----------------
rdp wrote:
> I think I understand, but I'm not an HTML/CSS expert. I think I want the CSSForCoverage used for every page, and the AlternateRowColor added for just the index page. Are you suggesting that getCSSForCoverage return the string for the entire CSS?
We're on the same page: with this change, CSSForCoverage will still be used in each file. I'm suggesting that getCSSForCoverage return either a link to a stylesheet or a full CSS blob. In either case, it should be able to optionally add the CSS you wrote to alternate row colors.

================
Comment at: SourceCoverageViewHTML.cpp:277
@@ +276,3 @@
+template <typename T>
+void emitFormattedCount(raw_ostream &OSRef, format_object<T> value,
+                        const char *align, const char *color = NULL) {
----------------
I'm curious: does it work to drop the template stuff and pass in a `const format_object_base &` instead of `format_object<T>`?

================
Comment at: SourceCoverageViewHTML.cpp:288
@@ +287,3 @@
+
+static void showSummary(raw_ostream &OSRef, std::string Name,
+                        FileCoverageSummary &File) {
----------------
Can you make Name a `const std::string &` or a StringRef?


https://reviews.llvm.org/D23486



