[PATCH][RFC] llvm-cov HTML generation

[Harlan Haskins via llvm-commits]

> On Mar 10, 2016, at 1:30 PM, Ying Yi <maggieyi666 at gmail.com> wrote:
> 
> 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.
> 
I haven’t seen this happen. Can you provide some steps to reproduce? Thanks!
> 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”
> 
That warning is incorrect. It’s a switch statement over both cases of an enum — I want this because if we add another format, say, OFJSON or something, I want a compiler warning telling me the switch case isn’t exhaustive.

Here’s an updated patch with some small sanity tests added:



Best,
Harlan

> 
> On Wed, Mar 9, 2016 at 12:50 AM, Harlan Haskins <hhaskins at apple.com <mailto: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 <mailto:mail at justinbogner.com>> wrote:
>> 
>> Harlan Haskins <hhaskins at apple.com <mailto:hhaskins at apple.com>> writes:
>>> Thanks again! Responses inline.
>>>> On Mar 8, 2016, at 5:08 AM, Ying Yi <maggieyi666 at gmail.com <mailto: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/ef5437f1/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-cov-html.diff
Type: application/octet-stream
Size: 55519 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160310/ef5437f1/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160310/ef5437f1/attachment-0003.html>