Use the global bundles instead of reinitializing

[weynhamz]

This PR makes it possible to let additional information of a Bundle to be used like in commit weynhamz/vim-plugin-vundle@f88bdbf. It is part of a PR #200 I previously submitted and been rejected due to duplication.

This is a resubmition of PR #230 which I accidently closed.

[greduan]

👍 Bump!

[jdevera]

I'm all for having this merged, in fact, I had done something similar using a dictionary, but this is simpler, although performance-wise, it's still O(n^2) for the whole installation, I don't think it's a big deal, since it's installation after all. Have you tried this with a large number of bundles?

I think, however, and for the sake of sanity, that this would be a great opportunity to replace some of those one letter variable names with other more meaningful ones. That's my suggestion here.

[greduan]

I agree that having more meaningful names here would be better.

Also, I don't know about performance since I haven't tried this out, but I think a couple of seconds wouldn't really matter, IMO. But if it goes over 4+ seconds I think we should probably find another solution. That's just my two cents.

[MarcWeber]

Can you document which additional keys you need? vim-addon-manager is based on vim-pi which naturally has dictionaries for each known plugin. We have special keys such as 'rtp' (like vundle), so we're interested in additional use cases. git sparse checkouts (#252) could be one. Please also document in which way this is different from 'rtp' setting (see examples in README).

[jdevera]

Anything that you might every want to configure and will then have an impact on the git command to be executed. Some examples:

• rtp, for you know what
• specifying the name of a directory to held the clone
• specifying a tag, branch,, or sha to checkout
• specifying that you don't want any git operation on that plugin, just add it to the rtp.
• specifying the desired depth of this clone

The problem with current implementation is that it throws away anything that is not the Bundle name, because Bundles are reparsed from the installation screen, which is unfortunate. This PR takes the name of the bundle and looks for it in the list of already loaded bundles, with all the config from vimrc, and uses that one, instead of creating a new one.

Can't see why we wouldn't merge this one. @starcraftman @gmarik can you give it a spin?

[jdevera]

I'm going to be running this PR for a few days, then merge it.

[jdevera]

Unfortunately, this PR is not enough to achieve all the benefits I mentioned, we still need this:

--- a/autoload/vundle/config.vim
+++ b/autoload/vundle/config.vim
@@ -27,7 +27,7 @@ func! vundle#config#init_bundle(name, opts)
   if a:name != substitute(a:name, '^\s*\(.\{-}\)\s*$', '\1', '')
     echo "Spurious leading and/or trailing whitespace found in bundle spec '" .
   endif
-  let opts = extend(s:parse_options(a:opts), s:parse_name(substitute(a:name,"['".'"]\+','','g')))
+  let opts = extend(s:parse_options(a:opts), s:parse_name(substitute(a:name,"['".'"]\+','','g')), 'keep')
   let b = extend(opts, copy(s:bundle))
   let b.rtpath = s:rtpath(opts)
   return b

So the overridden config options are kept, otherwise they get overwritten by the calculated ones, which kind of defeats the purpose. I think this is a separate bug, so I'll log it.

Then as it is the impact of this PR is merely a performance improvement. I have tested and it works fine.

@starcraftman @gmarik If no objections, I'll merge it.

[starcraftman]

@jdevera Sorry I missed this earlier, this patch looks fine to me. I just ran a quick test and works as expected on my linux box.

Before merging though I'd like this documented somewhere, at least in the vimdoc. Don't really have a good format for explaining dict args yet so we'll just put it after the rtp explanation. If we get more, we'll have to rethink how we document them.

diff --git a/doc/vundle.txt b/doc/vundle.txt
index 4bc3a5f..a1b4f2a 100644
--- a/doc/vundle.txt
+++ b/doc/vundle.txt
@@ -138,6 +138,10 @@ plugin resides.  For example:
 This can be used with git repositories that put the vim plugin inside a
 subdirectory.

+To rename the destination folder of the plugin, use the following option
+in the dictionary.
+>
+  Bundle 'git_URI', {'name': 'newPluginName'}

[jdevera]

Okay I'll merge this one tomorrow and then create another pull request with the 'keep' change and your proposed doc change

[starcraftman]

@jdevera Also I forgot to mention but we should have a test in the test file per gmariks want to have all stuff tested.

[gmarik]

yeah, having at least one entry with this config in test/vimrc would be nice.
Personally i'm always using it for a smoke test: vim -u test/vimrc

[jdevera]

Ok I'll add a test too.

[jdevera]

See #406