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

Cobertura coverage gutter #10758

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dustinlagoy
Copy link

This adds a neat feature to display a coverage gutter in the current document, similar to the look of the diff gutter. It works by reading cobertura coverage information from a file set by the HELIX_COVERAGE_FILE environment variable, checking if the current document was contained in the coverage file and checking that the coverage file was modified more recently than the document. The coverage is only displayed if all these checks pass.

A screenshot showing some valid coverage, generated with RUSTFLAGS="-C instrument-coverage", LLVM, and lcov_cobertura:

20240514_07h33m36s_grim

If there is any interest in merging this, here are a few things that could be improved:

  • Cache the loaded coverage information so it is not re-read every time the gutter is updated
  • Add support for methods and branches (need to think about how to display this)
  • Use a coverage-specific style instead of borrowing from the diff gutter style
  • Better path handling if the coverage file has relative paths that are not relative to the current directory when running helix
  • Support more coverage formats than just cobertura

@the-mikedavis
Copy link
Member

Putting coverage in the gutter is a cool idea but this seems to me like something that should live as a plugin rather than a core feature. I believe there are a bunch of different file formats for coverage information and I wouldn't want to support multiple in core.

@dustinlagoy
Copy link
Author

@the-mikedavis I agree this seems better as a plugin overall. If you would rather not have any part of it implemented in the core that's fine by me and I can write the plugin once that is an option. Do you think any of the proposed plugin options could handle custom gutters like this?

@kirawi
Copy link
Member

kirawi commented May 14, 2024

#8675 should be able to, yeah

@the-mikedavis the-mikedavis added the A-plugin Area: Plugin system label May 14, 2024
@CraigglesO
Copy link

I LOVE this. I hope you can find time to make it a plugin when that system becomes available. I use cobertura all the time, so this would help a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-plugin Area: Plugin system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants