Add dependency support. #558
Conversation
ghost
commented
Feb 17, 2015
ghost
commented
- Closes Support plugin dependencies #7
- Closes #7
|
Without going into the change itself, which is appreciated, there is the question of this I had read of a json file that VAM uses with some metadata. And since it's been there for ages, there is a change a few plugins are using it. I always imagined we would use whatever plugins were using already. Otherwise http://xkcd.com/927/ 😄 |
|
Dang, forgot about xkcd/927! Yes, there are a few plugins that use |
- Same format as VAM
|
I read issue #384 and added one last commit to fix it. (Should be able to push in 5 hours.) |
|
If I'm reading the code right, the dependency names have to be Vundle plugin identifiers, which are incompatible with addon-info.json keys (but in some cases can be translated). I'm very +1 for using the addon-info.json format, but unless/until vim-pi support is added to Vundle, you'd need to translate entries and ignore some of them to avoid forking a "Vundle addon-info.json format". A bare name in Vundle points to vim-scripts.org, but in addon-info.json it's a vim-pi plugin name. So "L9" fetches the same plugin in both, but many names will exist in one and not the other, or point to one plugin on vim-scripts.org and a different plugin in vim-pi. Plugins can be specified by github URL in both Vundle and addon-info.json, but the syntax is different. Where in Vundle you would install "dependencies": {
"fugitive": {
"type": "git",
"url": "git://github.com/tpope/vim-fugitive"
}
}So I would recommend in v1 you detect the special case of github paths like this and translate them into Vundle plugin handles, and ignore everything else for now. That would only work for explicit repo details (for names that exist in vim-pi like "fugitive", it's common to omit the repo details to have them looked up in vim-pi), but it's a good start. |
- Closes #384 - Users can set `g:vundle_no_deps` to disable dependency support if they want to.
|
@dbarnett: Now I know why this hasn't been done in 4 years. I shall make it so! (probably) |
|
There we go!
|
|
If you want, I'll squash the commits, but I am not sure how GitHub will react to a |
| if !empty(glob(a:bundle['rtpath'] . '/addon-info.json')) | ||
| let true = 1 | ||
| let false = 0 | ||
| let null = '' |
There was a problem hiding this comment.
These three variables seem unused
There was a problem hiding this comment.
@jdevera: AFAIK they are constants in JSON, so they need to be defined when vim parses it.
|
I added some comments to the commits, not just the pull request. |
| let null = '' | ||
| let dependencyentries = get(eval(join(readfile(a:bundle['rtpath'] . '/addon-info.json'), '')), 'dependencies', {}) | ||
| let bundles_needed = [] | ||
| for information in keys(dependencyentries) |
There was a problem hiding this comment.
information is really one plugin, maybe it could have a more descriptive name
| let null = '' | ||
| let dependencyentries = get(eval(join(readfile(a:bundle['rtpath'] . '/addon-info.json'), '')), 'dependencies', {}) | ||
| let bundles_needed = [] | ||
| for dep_name in keys(dependencyentries) |
There was a problem hiding this comment.
You could shorten the dependencyentries[dep_name]s everywhere and make the code more readable if you assigned the value to a variable, too:
for [depname, depinfo] in items(dependencyentries)There was a problem hiding this comment.
@dbarnett: Good idea. I didn't know vim could do that!
|
Yeah... This isn't going to work without rewriting vim-addon-manager inside of Vundle. |
ghost
closed this
ghost
deleted the
|
Well, I don't think installing dependencies by name will work without a bunch of changes, but installing by git URL looked fine. Why not start with that to get things moving on dependency support? |
|
So, what didn't work? |
|
@jdevera: Adding full dependency support for the @dbarnett: I set up a better pull request at #560, it includes the non-standard |
