Improve visibility into Plugin Status

[pdfowler]

I love this extension but have been running into issues lately with performance. Figured I'd take a look at what's going on under the hood and started tinkering. First up, visibility into whether the extension is still processing or not.

Summary

This PR partially addresses #124 by adding a Loading Spinner to Indicate when the plugin is processing a coverage file

Questions

1. I'm not certain the additional passing-around of the statusBar object is the best way. LMK if you can think of a better way to handle the data-flow for loading status.

Next Steps

I'm playing around with the notification window as specified in #124, but am still finding my way around the extension docs.

[ryanluker]

@pdfowler Thanks for the initial PR! Loving the initiative to help out with the codebase 😁 . RE This PR: I will take a look and tinker a bit locally to get the tests working for yah and give some code design feedback for the loading status bits.

General Extension Info: If you didn't know already you should be able to see the extension logs via the output logs area (see screenshot below) but I also agree that an indicator for activate is general a great UI pattern! These logs will help with the performance tuning we will do as apart of this ticket and the general notification you eluded too (I would put that work in another PR though and we can just consider the loading spinner in isolation).

For the notification window you can look at those api docs here https://code.visualstudio.com/api/references/vscode-api#ProgressLocation and the example project microsft did here https://github.com/microsoft/vscode-extension-samples/tree/master/progress-sample with the relevant section for progress here maybe https://github.com/microsoft/vscode-extension-samples/blob/master/progress-sample/src/extension.ts#L10-L14

For the software design of the notification window, I will maybe deposit some thoughts on the ticket you linked #124 as we probably want the coverage service to be in charge of managing this interaction.

[ryanluker]

@pdfowler Thanks again for the PR, I have cleaned up the failing tests https://github.com/ryanluker/vscode-coverage-gutters/pull/267#discussion_r453227185 and after you take a look at https://github.com/ryanluker/vscode-coverage-gutters/pull/267#discussion_r453227279 and decide on which way to go, I think this is ready to be merged!

I have setup a new release 2.6.0 but nothing else is currently planned for this release so it can be release soonish (~ a week).

Final Note: I had to update a test that I think should have been setting the loading indicator to false? it was a bit difficult to test so I had to use .only which is a great helper with the debug test vscode give yah 😁 .

[pdfowler]

@ryanluker I've addressed the spacing and the private variable issues as mentioned and pushed the changes. And nice catch on the copy-pasta testing error.

As I mentioned, I did some work yesterday to migrate to vscode-test (#263) and will package that up into a PR next.

[ryanluker]

@pdfowler looking excellent, I did find one small bug though (see screenshot below).

Seems to occur when you have the watch on and navigate to a file that has no coverage... we may want the spinner to only show up when the coverage is being generated and not also when it is being rendered (once the coverage is cached, rendering specific sections to the editor via decorations is very cheap and quick private async loadCache() {).

[pdfowler]

I haven't forgotten about this ... I'll take a look as soon as I have a minute

[ryanluker]

@pdfowler no worries! just let me know if you think you will be > 1 week and I will go ahead with release 2.6.0 without these changes. https://github.com/ryanluker/vscode-coverage-gutters/milestone/31

[pdfowler]

@ryanluker I have a few hours alotted today to address the bug you found, and hopefully get to filing the other work I've done locally. Update in a few hours

[pdfowler]

I've addressed the bug you found for no-coverage files, and added a test assertion to verify it.

I handled this in a finally block for renderCoverage, which requires a catch. I don't love the syntax, but there's no way to have a finally without a catch afaik. I've thrown the error to let the exception bubble up the chain and be handled in gutters#watchCoverageAndVisibleEditors

The setLoading calls within the loadCache method have also been removed.

[ryanluker]

@pdfowler hmm let me try to get the build working for yah. RE the try catch finally, I think the finally is a great idea and from what I saw you can do an optional catch now https://stackoverflow.com/questions/5764107/try-without-catch-possible-in-javascript/56404126#56404126 going to try that out 🤔 .

[ryanluker]

@pdfowler pretty sure I got it working, (I did notice sometimes the spinner doesn't show up but I believe that is only when the project is very small and microsoft doesn't update the GUI immediately or after the cache has been created).

Going to test a bit more but then merge this in and make an alpha github release for you to try out on your projects using the vsix file.

[ryanluker]

@pdfowler you can find the vsix download in the alpha release tag page. https://github.com/ryanluker/vscode-coverage-gutters/releases/tag/v2.7.0-alpha

[pdfowler]

So far so good ... the spinner is helping a lot with perceived responsiveness

[pdfowler]

I've been using the alpha release for the past week and haven't seen any issues. The loader REALLY helps when loading coverage in some of the embarrassingly large files in our main repo.

[ryanluker]

@pdfowler excellent thanks for the feedback! Hoping to work on the testing rework you did this upcoming weekend, then I should be able to release the new feature to the marketplace! https://github.com/ryanluker/vscode-coverage-gutters/pull/274