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

Coverage overlay integration (vim-go-coverlay) #786

Merged
merged 2 commits into from
Apr 4, 2016
Merged

Conversation

fatih
Copy link
Owner

@fatih fatih commented Apr 3, 2016

This PR includes #785, mainly @t-yuki's excellent https://github.com/t-yuki/vim-go-coverlay plugin with additional improvements and integrations into the vim-go code base. Changes are:

  • :GoCoverage does highlights the current source code instead of opening a browser. When called again it clears the highlighting.
  • :CoCoverageBrowser opens the html annotated coverage result (previous :GoCoverage

Thanks for @t-yuki excellent initial work!

Here is demo:

gocoverage-2

@fatih fatih mentioned this pull request Apr 3, 2016
@gonzaloserrano
Copy link

Nice work!

I was thinking in a couple of UX suggestions:

Thanks anyway ;-)

@fatih
Copy link
Owner Author

fatih commented Apr 4, 2016

Hi @gonzaloserrano

have a "toggle" command

Yeap, I'm going to remove :GoCoverageClear and :GoCoverage will act as a toggle. I'm always trying to use a feature for a couple of days/weeks to get a sense how it should work and today I realized is that it should be indeed a toogle

instead of changing the lines colors, change its "gutter" similar to how does it

This is hard to do as I have to hack the sidebar. This is exactly the same as the HTML representation and I think it fits it better. So there will be only one opinionated way.

Thanks for the feedback!

@gonzaloserrano
Copy link

Sounds cool anyway, thanks a lot! 👍

@@ -0,0 +1 @@
flavor 'fatih/vim-go', '~> 1.5'
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should be removed since it means This test depends on external plugin vim-go

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good call. Btw I've changed the source code a lot and renamed the internal functions too. They will not work and probably will be broken. But I'm going to work on tests soon.

@fatih
Copy link
Owner Author

fatih commented Apr 4, 2016

I'm merging this. Tested it extensively and should be working very well. I've disabled supporting multiple files as it's not scalable. I prefer we go simple, let first be good at supporting the current buffer, later we can use the cached result for other buffers in the future. Please open an issue if you see bugs! Thanks.

@fatih fatih merged commit 407908a into master Apr 4, 2016
@fatih fatih deleted the t-yuki-vim-go-coverlay branch April 4, 2016 20:39
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

Successfully merging this pull request may close these issues.

3 participants