Skip to content

[llvm-cov] format cells in code coverage report with 0/0 branches/functions/lines differently #75780

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

Closed
wants to merge 1 commit into from

Conversation

hanickadot
Copy link
Contributor

Currently such cells in the report are formatted as red. Which screams "problem" but with more and more rangefied code, there can be whole files without any branch inside them.

Before:
Screenshot 2023-12-18 at 11 09 07

After:
Screenshot 2023-12-18 at 11 08 29

Things to notice:

  • highlighted line which is result of cursor hover (now whole line is highlighted)
  • before there were some borders doubled (fixed)
  • cells with - 0/0 coverage are no longer red

@hanickadot hanickadot force-pushed the main branch 2 times, most recently from efc86db to 7a75dc6 Compare December 19, 2023 08:28
@hanickadot
Copy link
Contributor Author

I also needed to fix one test which was matching multiple rows in the table instead of one.

Copy link
Contributor

@chapuni chapuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This is a good improvement for the UX!
I pre-approve with one nit.

I would like to wait for at least one more for the approval. @petrhosek , how is this?

Copy link

github-actions bot commented Dec 19, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@hanickadot
Copy link
Contributor Author

I'm sorry I didn't notice you already looked into it, and I added also sorting mechanism. Can you look at it too?

@hanickadot hanickadot requested a review from chapuni December 19, 2023 09:28
@hanickadot hanickadot changed the title [llvm-cov] format cells in code coverage report with 0/0 branches/functions/lines differently [llvm-cov] format cells in code coverage report with 0/0 branches/functions/lines differently + adding sorting of the report table Dec 19, 2023
DOWNGRADE1: <td class='column-entry-green'>
DOWNGRADE1: 100.00% (3/3)
DOWNGRADE1: <td class='column-entry-yellow'>
DOWNGRADE1: 75.00% (9/12)
DOWNGRADE1: <td class='column-entry-red'>
DOWNGRADE1: 66.67% (4/6)
DOWNGRADE1: <td class='column-entry-gray'>
DOWNGRADE1: - (0/0)
DOWNGRADE1: </tr>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previously the red was actually the - (0/0), so I changed the threshold and make the test more precise

@chapuni
Copy link
Contributor

chapuni commented Dec 19, 2023

@hanickadot I hope we could land the 1st idea (when I approved) in advance.
Could you rewind extra changes and create another PR?

I expect sorting will be a good idea. (I also wish filtering)
It will introduce JS and it should be another PR.

@hanickadot hanickadot changed the title [llvm-cov] format cells in code coverage report with 0/0 branches/functions/lines differently + adding sorting of the report table [llvm-cov] format cells in code coverage report with 0/0 branches/functions/lines differently Dec 19, 2023
@hanickadot
Copy link
Contributor Author

yes, already rewinded

Copy link
Contributor

@chapuni chapuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Don't forget squashing.

…ifferenly (gray instead red) and make the table a bit nicer
chapuni pushed a commit that referenced this pull request Dec 19, 2023
…ifferenly (gray instead red) and make the table a bit nicer (#75780)
@chapuni
Copy link
Contributor

chapuni commented Dec 19, 2023

Pushed manually, 18e1179

@chapuni chapuni closed this Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants