Closed alexanderjeurissen closed 10 years ago
@justinj Ready to Review
Awesome, thanks a lot! Do you think there is a nice way to keep the two branches in sync, or at least backport the changes to the snippets to the main branch (is there a common approach for handling this?)? It would be nice to be easily able to get the improvements you've made accessible from vim-snipmate as well. I can also just do them by hand if there's no nice way to handle it automatically.
@justinj Some improvements i made aren't available in vim-snipmate that I'm aware off (specifying if a snippet can be extended in-sentence for example). But for the most parts I agree, we should aim to have both branches in-sync when it comes to what snippets are available and aspects that are supported by both plugins.
I just checked the Vundle and NeoBundle documentation and it's possible to just install a plugin from a subdirectory by setting a runtimePath. I'm not sure about other plugin managers though.
Vundle example (just cloning the Ultisnippets directory):
Plugin 'justinj/vim-react-snippets', {'rtp': 'UltiSnips/'}
NeoBundle example:
NeoBundle 'justinj/vim-react-snippets', { 'rtp' : 'UltiSnips' }
I just merged my feature branch in my fork, and will test if i can extract the UltiSnips snippet file that way. If that is the case then we don't need to maintain two branches and that would make all of this a lot easier to maintain ;)
@justinj leave this PR as is, I'll let you know if this works
@justinj i just checked and it seems Ultisnippet is smart enough to prioritize it's own snippets before vimSnipmate ones. This is GOOD NEWS ! this means we don't have to maintain 2 separate branches.
Few TODO's remain though:
@alexanderjeurissen So would you say this is good to be merged as-is for the time being?
@justinj yeah sure, i can implement the improvements that are vim-snipmate compatible in a separate PR.
MERGE at will!
Thanks again for this!
Pullrequest description
This Pullrequest implements Ultisnips support by porting the original snippets to the UltiSnips format. I also made a few minor tweaks:
<div>{state<tab>
Previously state wouldnt expand tothis.state
because of the{
React.CreateClass
snippet by changing thejsx dom defenition
,requirejs
andmodule.exports
parts to separate snippet tabstops that can be removed or overridden. This gives the user more flexibility in reusing the createClass snippet multiple times inside the document without introducing a new snippet.$0
to all component functions likerender
,componentWillMount
etc. This allows the user to skip to the end when they implemented the method and thus chain methods more easily.this
to some snippets because it's the most common usecase. For example:this.setState()
instead ofsetState()
IMPORTANT: merging / implementation Notes
As to the merging strategy, I think it's best to have a separate branch at all times for the UltiSnips This allows vim-snipmate users to use the
master
branch and allows UltiSnips users to take advantage of some of the features that UltiSnips provides.If you combine this branch with master, then UltiSnips users will have conflicting snippets because it supports snipmate aswell.
One thing to keep in mind is that we should also update the documentation to let UltiSnips users know they can just checkout the UltiSnips branch, this is possible with most Vim plugin managers.
Also all future UltiSnips functionality should be created by making a featureBranch based on the UltiSnips branch.
I suggest you create a branch on your repository called
UltiSnippets
and base it off master then close this PR, that will be a sign for me that i can rebase my featureBranch on the new UltiSnippets branch and re-create the pullrequest.