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

:CoverageShowDiff is not implemented?! #19

Open
blueyed opened this issue Sep 20, 2017 · 5 comments
Open

:CoverageShowDiff is not implemented?! #19

blueyed opened this issue Sep 20, 2017 · 5 comments

Comments

@blueyed
Copy link
Contributor

blueyed commented Sep 20, 2017

It looks like diff_path is not being added to the data anywhere!? (

if has_key(l:data, 'diff_path')
)

And (although mentioned in

" function(coverage_data) prototype. Coverage data format is as described in
" the general help for this plugin.
) the coverage data format is not described in the help, is it?

@blueyed
Copy link
Contributor Author

blueyed commented Jun 21, 2018

Ping @stgpetrovic (who committed this initially for "Goran Petrovic").

As it currently stands I would just remove it.

blueyed added a commit to blueyed/vim-coverage that referenced this issue Jun 21, 2018
It does not appear to be implemented fully.

Fixes google#19.
blueyed added a commit to blueyed/vim-coverage that referenced this issue Jun 26, 2018
It does not appear to be implemented fully.

Fixes google#19.
@dbarnett
Copy link
Contributor

dbarnett commented Jul 2, 2018

Aha, I found we actually have private shared coverage provider that patches this 'diff_path' key into the coverage data. I agree it shouldn't be there and we can be somewhat aggressive ripping it out on that end too, but could we get a sketch of how we'd want caching/reloading to work for #27 before proceeding here?

@blueyed
Copy link
Contributor Author

blueyed commented Jul 5, 2018

Sure. The general idea would be to not use caching at all at first.

Can you share how it is used internally (in an abstract way)?

@dbarnett
Copy link
Contributor

Sure, had to look up what it was doing… It says "diff_path" but it's actually not a diff but a snapshot of the file as it was at the time coverage was evaluated, used for diffing with the current version. The common case for this tool is to render server-side coverage data on submitted files, and it can actually be kind of awkward getting fresh coverage results from modified versions of the file. I filed #34 to discuss how to handle this kind of snapshotted coverage data functionality long term, since there's some complexity to getting it right.

@dbarnett
Copy link
Contributor

Overall it makes sense coverage providers should be able to capture the snapshotted file contents. We could just rename it to something like file_snapshot and add it to the docs for the coverage provider interface. It should probably also support storing file contents as a list of strings and not just a file path since there won't always be a file snapshot on disk.

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

No branches or pull requests

2 participants