[PATCH][RFC] llvm-cov HTML generation

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 8 10:55:51 PST 2016


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.


More information about the llvm-commits mailing list