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

Support minisnip #1589

Merged
merged 3 commits into from
Dec 7, 2017
Merged

Support minisnip #1589

merged 3 commits into from
Dec 7, 2017

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented Nov 28, 2017

Needs joereynolds/vim-minisnip#5

I've also changed the default for g:go_snippet_engine to automatic,
which is a new value that will use the first snippet engine that's
loaded. I think that's more user-friendly than expecting the user to set
it.

I also changed the detection method to use the variables, instead of
scanning runtimepath. This is more efficient.

Need to actually add the snippets for minisnip though.

@arp242 arp242 added the wip label Nov 28, 2017
@fatih
Copy link
Owner

fatih commented Nov 30, 2017

What makes minisnip superior to ultisnips or neosnippets? Back then I've added them to add support, but I'm not sure we should support other plugins, especially new plugins those with a low userbase. I'm not against it, just trying to understand the value it'll bring.

@arp242
Copy link
Contributor Author

arp242 commented Nov 30, 2017

What makes minisnip superior to ultisnips or neosnippets

It's a very simple plugin which Just Works™ without too much Magic. I just personally prefer it over a large plugin written in Python etc.

I'm not sure we should support other plugins, especially new plugins those with a low userbase

That okay; I don't really care. I can reduce the PR to just some of the small improvements I made.

It just seemed to me that adding this doesn't really affect much of anything. If you don't use it then you won't know it's there, and it adds pretty much zero maintenance overhead.

@arp242 arp242 removed the wip label Nov 30, 2017
@fatih
Copy link
Owner

fatih commented Nov 30, 2017

It just seemed to me that adding this doesn't really affect much of anything. If you don't use it then you won't know it's there, and it adds pretty much zero maintenance overhead.

True that, I was just worried that this can easily can spread to other plugins. I'm curious if we can somehow do the detection manual. I'm ok merging this btw, I'll chime more if I have something to show.

Needs joereynolds/vim-minisnip#5

I've also changed the default for `g:go_snippet_engine` to `automatic`,
which is a new value that will use the first snippet engine that's
loaded. I think that's more user-friendly than expecting the user to set
it.

I also changed the detection method to use the variables, instead of
scanning `runtimepath`. This is more efficient.

Need to actually add the snippets for minisnip though.
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@6366c6e). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #1589   +/-   ##
========================================
  Coverage          ?   14.8%           
========================================
  Files             ?      53           
  Lines             ?    4146           
  Branches          ?       0           
========================================
  Hits              ?     614           
  Misses            ?    3532           
  Partials          ?       0
Flag Coverage Δ
#nvim 5.06% <0%> (?)
#vim74 14.73% <0%> (?)
#vim80 14.49% <0%> (?)
Impacted Files Coverage Δ
ftplugin/go/snippets.vim 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6366c6e...e6500c7. Read the comment docs.

@arp242 arp242 merged commit 2fe16b9 into fatih:master Dec 7, 2017
@arp242 arp242 deleted the minisnip branch December 7, 2017 18:34
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