Update for Vundle's new interface #241

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Mar 27, 2014

Contributor

There might be a chicken-and-egg problem if you pull in these changes to an existing installation and run rcup without first running BundleInstall! (note exclamation point) in Vim to update Vundle itself.

Contributor

rspeicher commented Mar 27, 2014

There might be a chicken-and-egg problem if you pull in these changes to an existing installation and run rcup without first running BundleInstall! (note exclamation point) in Vim to update Vundle itself.

@mike-burns

This comment has been minimized.

Show comment
Hide comment
@mike-burns

mike-burns Apr 3, 2014

Member

Is there a way we can write a hook that will run BundleInstall! if the old version is detected but normal PluginInstall otherwise?

Member

mike-burns commented Apr 3, 2014

Is there a way we can write a hook that will run BundleInstall! if the old version is detected but normal PluginInstall otherwise?

@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Apr 4, 2014

Contributor

For the time being it looks like BundleInstall! will still work, so you could just run that on post-up (as opposed to the non-bang version that gets run now). Downside is that post-up will take longer as it updates every single defined bundle.

It looks like PluginInstall was added in VundleVim/Vundle.vim@0521de9. Could you do some kind of check with git in the local Vundle checkout to see if that ref exists and take action based on that?

Contributor

rspeicher commented Apr 4, 2014

For the time being it looks like BundleInstall! will still work, so you could just run that on post-up (as opposed to the non-bang version that gets run now). Downside is that post-up will take longer as it updates every single defined bundle.

It looks like PluginInstall was added in VundleVim/Vundle.vim@0521de9. Could you do some kind of check with git in the local Vundle checkout to see if that ref exists and take action based on that?

@christoomey

This comment has been minimized.

Show comment
Hide comment
@christoomey

christoomey Apr 17, 2014

Member

@mike-burns From Vim you can check that a command is defined with if exists(':PluginInstall') == 2 (2 is an exact match). We could create a wrapper function at the top of vimrc.bundles that determines the correct command to use:

function! s:SmartVundleInstall()
  if exists(':PluginInstall') == 2
    PluginInstall
  else
    BundleInstall
  endif
endfunction!

command! SmartVundleInstall call <sid>SmartVundleInstall<cr>

then the post-up command becomes:

vim -u $HOME/.vimrc.bundles +SmartVundleInstall +qa

Feels a bit strange, but assuming they will remove the deprecated commands at some point then this could keep it safe.

Member

christoomey commented Apr 17, 2014

@mike-burns From Vim you can check that a command is defined with if exists(':PluginInstall') == 2 (2 is an exact match). We could create a wrapper function at the top of vimrc.bundles that determines the correct command to use:

function! s:SmartVundleInstall()
  if exists(':PluginInstall') == 2
    PluginInstall
  else
    BundleInstall
  endif
endfunction!

command! SmartVundleInstall call <sid>SmartVundleInstall<cr>

then the post-up command becomes:

vim -u $HOME/.vimrc.bundles +SmartVundleInstall +qa

Feels a bit strange, but assuming they will remove the deprecated commands at some point then this could keep it safe.

@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Apr 20, 2014

Contributor

I just ran into the problem I described when I updated my dotfiles on my personal server. My quick solution was to just go into the Vundle directory and do a git pull.

Maybe the easiest solution, then, would be to modify the post-up to something like this?

#!/bin/sh

if [ ! -e $HOME/.vim/bundle/vundle ]; then
  git clone https://github.com/gmarik/vundle.git $HOME/.vim/bundle/vundle
else
  cd $HOME/.vim/bundle/vundle && git pull
fi
vim -u $HOME/.vimrc.bundles +PluginInstall +qa
Contributor

rspeicher commented Apr 20, 2014

I just ran into the problem I described when I updated my dotfiles on my personal server. My quick solution was to just go into the Vundle directory and do a git pull.

Maybe the easiest solution, then, would be to modify the post-up to something like this?

#!/bin/sh

if [ ! -e $HOME/.vim/bundle/vundle ]; then
  git clone https://github.com/gmarik/vundle.git $HOME/.vim/bundle/vundle
else
  cd $HOME/.vim/bundle/vundle && git pull
fi
vim -u $HOME/.vimrc.bundles +PluginInstall +qa
@gylaz

View changes

vimrc.bundles
@@ -7,24 +7,24 @@ set rtp+=~/.vim/bundle/vundle/
call vundle#rc()

This comment has been minimized.

@gylaz

gylaz Jun 25, 2014

Contributor

Looks like this line should be call vundle#begin() now.

@gylaz

gylaz Jun 25, 2014

Contributor

Looks like this line should be call vundle#begin() now.

@gylaz

This comment has been minimized.

Show comment
Hide comment
@gylaz

gylaz Jun 25, 2014

Contributor

We'll also need call vundle#end() at the end of the script.

Contributor

gylaz commented Jun 25, 2014

We'll also need call vundle#end() at the end of the script.

@gylaz

This comment has been minimized.

Show comment
Hide comment
@gylaz

gylaz Jun 25, 2014

Contributor

@tsigo I like your solution of forcing the update of vundle. Otherwise, the new DSL (e.g Plugin) won't work, right? A potential issue is that an update of vundle may eventually introduce breaking changes that we haven't anticipated, but I think we can deal with them as they come up.

Contributor

gylaz commented Jun 25, 2014

@tsigo I like your solution of forcing the update of vundle. Otherwise, the new DSL (e.g Plugin) won't work, right? A potential issue is that an update of vundle may eventually introduce breaking changes that we haven't anticipated, but I think we can deal with them as they come up.

@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Jun 25, 2014

Contributor

You could do a check to see if the specific commit I mentioned above exists
in the reflog, doing the update if not, otherwise skipping it.

On Wednesday, June 25, 2014, Greg Lazarev notifications@github.com wrote:

@tsigo https://github.com/tsigo I like your solution of forcing the
update of vundle. Otherwise, the new DSL (e.g Plugin) won't work, right?
A potential issue is that an update of vundle may eventually introduce
breaking changes that we haven't anticipated, but I think we can deal with
them as they come up.


Reply to this email directly or view it on GitHub
#241 (comment).

Contributor

rspeicher commented Jun 25, 2014

You could do a check to see if the specific commit I mentioned above exists
in the reflog, doing the update if not, otherwise skipping it.

On Wednesday, June 25, 2014, Greg Lazarev notifications@github.com wrote:

@tsigo https://github.com/tsigo I like your solution of forcing the
update of vundle. Otherwise, the new DSL (e.g Plugin) won't work, right?
A potential issue is that an update of vundle may eventually introduce
breaking changes that we haven't anticipated, but I think we can deal with
them as they come up.


Reply to this email directly or view it on GitHub
#241 (comment).

@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Jun 26, 2014

Contributor

Ok, I've forced an update. It should address the chicken-and-egg problem (I'm interested in a better method -- mine seems a little hacky) without introducing possible breaking changes in the future, and added the #start and #end calls @gylaz pointed out.

Contributor

rspeicher commented Jun 26, 2014

Ok, I've forced an update. It should address the chicken-and-egg problem (I'm interested in a better method -- mine seems a little hacky) without introducing possible breaking changes in the future, and added the #start and #end calls @gylaz pointed out.

@rspeicher

View changes

hooks/post-up
@@ -2,5 +2,10 @@
if [ ! -e $HOME/.vim/bundle/vundle ]; then
git clone https://github.com/gmarik/vundle.git $HOME/.vim/bundle/vundle
+elif ! git -C $HOME/.vim/bundle/vundle rev-list HEAD|grep -q 0521de95 ; then

This comment has been minimized.

@rspeicher

rspeicher Jun 26, 2014

Contributor

I truncated the SHA to save some line space, but looking at it now I'm wondering about the possibility of those eight characters appearing as a substring in another hash. Should I just go ahead and use the full SHA?

@rspeicher

rspeicher Jun 26, 2014

Contributor

I truncated the SHA to save some line space, but looking at it now I'm wondering about the possibility of those eight characters appearing as a substring in another hash. Should I just go ahead and use the full SHA?

This comment has been minimized.

@gylaz

gylaz Jun 26, 2014

Contributor

I agree, full sha to be safe. Can we capture the full sha into a descriptively named var?

@gylaz

gylaz Jun 26, 2014

Contributor

I agree, full sha to be safe. Can we capture the full sha into a descriptively named var?

@gylaz

View changes

vimrc.bundles
if filereadable(expand("~/.vimrc.bundles.local"))
source ~/.vimrc.bundles.local
endif
-filetype on
+call vundle#end()
+filetype plugin indent on

This comment has been minimized.

@gylaz

gylaz Jun 26, 2014

Contributor

Please keep this as filetype on. We turn on plugin indent elsewhere.

@gylaz

gylaz Jun 26, 2014

Contributor

Please keep this as filetype on. We turn on plugin indent elsewhere.

@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Jun 26, 2014

Contributor

Rebased from latest master.

Contributor

rspeicher commented Jun 26, 2014

Rebased from latest master.

@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Jul 22, 2014

Contributor

Rebased on latest master and updated the Vundle repository path as mentioned in #286

Contributor

rspeicher commented Jul 22, 2014

Rebased on latest master and updated the Vundle repository path as mentioned in #286

@mike-burns

This comment has been minimized.

Show comment
Hide comment
@mike-burns

mike-burns Jul 22, 2014

Member

This all makes sense to me, but I'll wait to merge this until someone can confirm it works for them.

Member

mike-burns commented Jul 22, 2014

This all makes sense to me, but I'll wait to merge this until someone can confirm it works for them.

@gylaz

View changes

hooks/post-up
if [ ! -e $HOME/.vim/bundle/vundle ]; then
- git clone https://github.com/gmarik/vundle.git $HOME/.vim/bundle/vundle
+ git clone https://github.com/gmarik/Vundle.vim.git $HOME/.vim/bundle/vundle

This comment has been minimized.

@gylaz

gylaz Jul 22, 2014

Contributor

Does the vundle directory need to be renamed to Vundle.vim? Otherwise, will there be problems with line 10 in vimrc.bunldes?

@gylaz

gylaz Jul 22, 2014

Contributor

Does the vundle directory need to be renamed to Vundle.vim? Otherwise, will there be problems with line 10 in vimrc.bunldes?

This comment has been minimized.

@rspeicher

rspeicher Jul 22, 2014

Contributor

Ugh, you're right. I avoided changing the name of the local directory for backwards compatibility, but forgot that Vundle will just name the folder the same thing as whatever's defined in the bundles file.

Should I just change the folder from $HOME/.vim/bundle/vundle to $HOME/.vim/bundle/Vundle.vim? This has the added benefit of negating the specific revision check since everyone who runs rcup after this is merged will be pulling the latest version of Vundle.

@rspeicher

rspeicher Jul 22, 2014

Contributor

Ugh, you're right. I avoided changing the name of the local directory for backwards compatibility, but forgot that Vundle will just name the folder the same thing as whatever's defined in the bundles file.

Should I just change the folder from $HOME/.vim/bundle/vundle to $HOME/.vim/bundle/Vundle.vim? This has the added benefit of negating the specific revision check since everyone who runs rcup after this is merged will be pulling the latest version of Vundle.

This comment has been minimized.

@gylaz

gylaz Jul 22, 2014

Contributor

I like that approach.

@gylaz

gylaz Jul 22, 2014

Contributor

I like that approach.

@rspeicher

This comment has been minimized.

Show comment
Hide comment
@rspeicher

rspeicher Jul 23, 2014

Contributor

Updated again.

Contributor

rspeicher commented Jul 23, 2014

Updated again.

@gylaz

View changes

vimrc.bundles
@@ -4,33 +4,34 @@ end
filetype off
set rtp+=~/.vim/bundle/vundle/

This comment has been minimized.

@gylaz

gylaz Jul 26, 2014

Contributor

This should be Vundle.vim as well.

@gylaz

gylaz Jul 26, 2014

Contributor

This should be Vundle.vim as well.

This comment has been minimized.

@rspeicher

rspeicher Jul 26, 2014

Contributor

Good catch.

@rspeicher

rspeicher Jul 26, 2014

Contributor

Good catch.

@gylaz

This comment has been minimized.

Show comment
Hide comment
@gylaz

gylaz Jul 26, 2014

Contributor

Thanks @tsigo! Merged this as 0191762.

Contributor

gylaz commented Jul 26, 2014

Thanks @tsigo! Merged this as 0191762.

@gylaz gylaz closed this Jul 26, 2014

@rspeicher rspeicher deleted the rspeicher:rs-new-vundle-interface branch Jul 26, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment