[PATCH][RFC] llvm-cov HTML generation

Ying Yi via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 10 13:30:04 PST 2016


Thanks for your reply and the updated patch.


1) When I use the release build of llvm-cov to generate HTML files, all the
HTML files are named “html”. This issue also exists on UNIX.


2) The following warning is given when building llvm-cov using Visual
Studio 2013.

“sourcecoverageview.cpp (108): warning C4715:
'llvm::SourceCoverageView::create' : not all control paths return a value”


Thanks,


Maggie

On Wed, Mar 9, 2016 at 12:50 AM, Harlan Haskins <hhaskins at apple.com> wrote:

> Updated patch with unistd removed, utf-8, and `console` replaced with
> `text`.
>
> Thanks!
> Harlan
>
>
> On Mar 8, 2016, at 10:55 AM, Justin Bogner <mail at justinbogner.com> wrote:
>
> Harlan Haskins <hhaskins at apple.com> writes:
>
> Thanks again! Responses inline.
>
> On Mar 8, 2016, at 5:08 AM, Ying Yi <maggieyi666 at gmail.com> wrote:
>
> Thanks for adding the index page. I have some comments:
>
> 1) The ‘unistd.h’ header won’t build on windows.
>
> Thanks for catching that — I was using it to debug, but don’t need
> it. I also don’t have a Windows machine to debug, so thank you for
> investigating portability!
>
> 2) The help text for ‘-format’ is different from llvm-profdata.exe
> tool, which uses the following style:
>    -format=<html|console>                 - Format to output comparison ….
>    Is there a standard way to do this?
>
> LLVM’s CommandLine library renders the help text. Also, I noticed I
> used ‘comparison’ instead of ‘coverage’. Thanks!
>
> 3) Should ‘console’ be changed to ‘text’, because it doesn’t show in
> the console when you give an output directory?
>
> Yeah, I think it could. I’ve cc’d Justin for his input — he originally
> suggested Console.
>
>
> My only problem with "text" is that this includes ANSI escape sequences
> when it uses colour, so text seems a bit misleading. That said, it
> doesn't matter much - I'm fine with either.
>
> 4) Could you please use an html charset that supports UTF-8?
>
> Yep. Thanks for catching that!
>
> 5) The HTML report can look quite different to the source file if it
> includes tabs. Could you specify a smaller tab-size in the CSS file?
>
> That’s a good question, and I don’t quite know the correct answer to
> it. GitHub has a complicated set of rules for tab sizing that makes a
> ‘best guess’ based on the source file type. I wonder if it’s a good
> candidate for a command line switch?
>
> In the future (not for this patch):
>
> 6) Could we support a user CSS file?
> Currently, the CSS rules are embedded into the html page. Could
> llvm-cov provide an option to use an external CSS file? This would
> give the user more flexibility.
>
> I agree that it’d give the user more flexibility, but it’d be
> difficult to do given the nature of the directory structure. Maybe, if
> you know you’re gonna be running it on a server where the ‘root’ is
> well-defined, we could just dump the css file to disk and put a
> relative <link> tag in. But if it’s gonna be ten directories down,
> we’ll need to have some logic in that replaces all the parent paths in
> the link with /../..’s to make the file resolve. I think it can be
> done, just with more input from the user about their intention with
> hosting.
>
> 7) Could we put the functions/lines summary information into the
> HTML files? As a rough example:
>
> Lines: 9/10 (90%)
> Functions: 1/1 (100%)
>
> I think this totally can be done — it’s out of scope for this patch
> (it’s already a pretty big patch as it is.)
> Some kind of merging of ‘show’ and ‘report’ would be ideal. Maybe the
> index page has at-a-glance information about each file?
>
>
> Yep, this sounds like a great idea, but it's probably more suited to a
> follow up patch.
>
>
>
>


-- 
Ying Yi
SN Systems Ltd - Sony Computer Entertainment Group.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160310/031ec50d/attachment.html>


More information about the llvm-commits mailing list