Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single file HTML output. #613

Open
smavridis opened this issue Apr 18, 2022 · 11 comments · May be fixed by #916
Open

Single file HTML output. #613

smavridis opened this issue Apr 18, 2022 · 11 comments · May be fixed by #916

Comments

@smavridis
Copy link

smavridis commented Apr 18, 2022

Is your feature request related to a problem? Please describe.
I would like the detailed HTML coverage report to be a single file.
Initially i thought html-self-contained would do this, but it does not support the detailed output.

Describe the solution you'd like
A single HTML file containing the detailed report.

Describe alternatives you've considered
In theory i could transform/'merge' the current output after gcovr.
This would require parsing the HTML output and then 'merging' it, but seems the wrong way to do it.
Ideally i would like for gcovr to provide this functionality as it already has the data and just the output needs tweaking.

Additional context
I have made a proof-of-concept in https://github.com/smavridis/gcovr/tree/single_file_html.
As per the guidelines i opened this issue before creating a pull request for more feedback.

Let me know what you think, and thank you for gcovr.

@latk
Copy link
Member

latk commented Apr 19, 2022

Hello @smavridis, thanks for this suggestion! It definitely looks like an interesting idea, and like a useful feature for some people.

The --html-self-contained feature doesn't do this because it was introduced to stop inlining CSS into the HTML, in order to satisfy the Content Security Policy of some web servers.

I have some concerns with your proposed implementation but that's something best discussed iteratively in a code review. For example, I don't like that partial HTML fragments are rendered up front and then concatenated, when instead the necessary data could be collected and then rendered as part of the template. I also think that the prototype breaks links to particular line numbers.

But to clarify, the fact that I'm already nitpicking such details means that I'm pretty much on board with this feature.

What it needs is a good name. Since it relates to --html-details, it should be named accordingly. I am mildly in favour of something like --html-details-single-page or --html-details-spa, especially as this is clearly pointing into the direction of relying on JavaScript which we've previously avoided.

@smavridis
Copy link
Author

smavridis commented Apr 19, 2022

For example, I don't like that partial HTML fragments are rendered up front and then concatenated, when instead the necessary data could be collected and then rendered as part of the template.

I wanted to minimize the modifications/additions to the existing code that's why i did the prototype this way, i agree creating new templates is a better approach.

I also think that the prototype breaks links to particular line numbers.

Yes you are correct.
Line ID are currently of the form l#number, this leads to duplicates since all files will have common line ids.
Again i avoided messing with that before discussing it with you.
For completeness I also did not handle the function listing, and the function list links.
I guess i can resolve all these by creating the custom templates as mentioned above.

What it needs is a good name.

I will change it to html-details-single-page for now.

clearly pointing into the direction of relying on JavaScript which we've previously avoided.

I have thus far avoided the use of js by using the css :target selector for the showing/hiding of pages.
I am using a hack meta tag to make the root/overview page visible.
I think there is a CSS way to do it, but i am not a web developer so am still looking for the solution.
Failing that, i might just add a link (if no js is available?) to manualy show the root page?

Thanks for the feedback, i will try to address them in the next days, and get back to you.

@latk
Copy link
Member

latk commented Apr 19, 2022

I don't think it's possible to do without JS for a single-page report unless all other anchors are entirely removed – for example, navigating to a function or line would mean that the surrounding part of the report isn't the :target. While there are ancient CSS tricks with invisible checkboxes and the HTML5 <details> element, adding small JS snippets to add/remove CSS classes seems to be the most sensible approach. As a no-JS fallback, the single-page report could just concatenate the reports for all source files without hiding anything. In essence, a suitable strategy might be:

  • All IDs are namespaced by a file ID. E.g. a #l22 anchor might become #page/example.cpp/l22
  • Upon page load, before the DOM is ready or any content is rendered, a synchronous JavaScript snippet attaches a class to the html or body element to indicate JS support. When this class is present, inactive parts of the report are hidden via CSS. For example, .js-enabled .report-page {display: none}
  • Once the page is ready, the hash/fragment of the document location is parsed to find the part of the report that should currently be visible. A CSS class is added to that part and the class is removed from all other parts. For example, .js-enabled .report-page.report-page-active {display: block}
  • A hashchange event listener repeats this process whenever navigation occurs.
  • If no fragment is currently active, the fragment is set to the default state, e.g. location.hash = "#page/summary".

This should be doable without requiring any libraries.

During your work you'll discover that the code for the HTML writer is not very pretty. If you see easy wins, feel free to restructure it a bit to make your changes easier – but please use separate commits for refactoring and for enhancements. If you have small questions along the way, feel free to ask in the chatroom (Gitter or Matrix).

@maestroIgor
Copy link

Hello, are there any updates on that nice to have feature?

@Spacetown
Copy link
Member

@smavridis Have you done some work on this?

@Spacetown Spacetown linked a pull request Apr 15, 2024 that will close this issue
@Spacetown
Copy link
Member

@smavridis Please can you take a look at the linked PR?

@Spacetown
Copy link
Member

@latk, @smavridis Will one of you have time to check the linked PR?

@Spacetown
Copy link
Member

@smavridis Will you be able to check the PR?

@smavridis
Copy link
Author

Hello, i had forgotten about this ... 2 years is a long time.
Unfortunately i am not a maintainer so i cant review/accept anything.
I did take a quick look though and i can note a few things:
If you fixed an existing bug, the fix should probably be isolated and submitted separately.
Other than that you refactored a lot of code which might be necessary, but does make review harder.

I will try an give it a try in the weekend to see the output, thank you for your effort.

@Spacetown
Copy link
Member

I will raise a separate Pr to fix the paths. I'm glad if you only comment on the PR. This is ev er n better then forcing the merge.

@Spacetown
Copy link
Member

@smavridis Have you checked the report?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants