search

google / vim-coverage

Apache License 2.0
101 stars 22 forks source link

:CoverageShowDiff is not implemented?! #19

Open blueyed opened 6 years ago

blueyed commented 6 years ago

It looks like diff_path is not being added to the data anywhere!? (https://github.com/google/vim-coverage/blob/88d0204016934e83e374db534d2b6156aa4ac6bb/autoload/coverage.vim#L130)

And (although mentioned in https://github.com/google/vim-coverage/blob/88d0204016934e83e374db534d2b6156aa4ac6bb/autoload/coverage.vim#L184-L185) the coverage data format is not described in the help, is it?

blueyed commented 6 years ago

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

As it currently stands I would just remove it.

dbarnett commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.