Add maktaba#plugin#Register that never touches runtimepath

[dbarnett]

Maktaba as a plugin framework should allow plugins to register/init themselves via maktaba without side effects like modifying runtimepath that should generally be left up to plugin managers. This change adds a maktaba#plugin#Register function to do just that. Also changes all maktaba#plugin#GetOrInstall calls in maktaba to maktaba#plugin#Register because:

• in many cases like maktaba#plugin#Detect and maktaba#plugin#Enter, we can assume that the plugin is already on runtimepath and skip the overhead of checking, and
• maktaba as a library shouldn't be modifying runtimepath unless instructed to by a plugin manager or explicit calls to maktaba#rtp utilities.

Resolves #50.

[Soares]

So now we have three types of plugin "installish"ness?

1. "Registered" with Maktaba (may or may not be on the rtp, may or may not be initialized)
2. Placed on the rtp (may or may not be registered, may or may not be initialized) (this is called "installed", for some reason).
3. Initialized by Maktaba (eg, instant files have been run) (you'd better hope it's both registered and 'installed')

First off, I'm not sure I see the point. What does the registered state buy us?

Secondly, the names are quite confusing. I would strongly expect "installation" to include both adding to the runtimepath and sourcing the instant files.

[dbarnett]

There is no "registered but not initialized" state, and "install" does what you're suggesting it should (rtp + instant files). If you're referring to the way I split up the script-local functions, that was just a quirk of the code flow, and I struggled to come up with names for the different blobs of logic. Now I've merged some of the functions together to help that part of code explain itself better (see the force_rtp arg).

I agree there are a lot of variants of "installish"ness. Part of the reason I wanted to create something with more tightly-defined behavior and call it "Register" is that the term "Install" is so murky. Just for the sake of argument, turn the question around: If we only had maktaba#plugin#Register, what does Install and GetOrInstall buy us? I believe those are only for plugin managers (or users playing plugin manager by calling them directly), and maybe they belong in maktaba but I'm not sure they should be the only option.

In fact, every major plugin manager already knows how to handle rtp without maktaba's help. If any of the GetOrInstalls I changed were the thing adding a plugin to rtp, it would imply something's messed up with the plugin manager. So maybe maktaba should try to automatically detect rtp problems and warn the user, or maybe it should just leave it alone and let any resulting errors speak for themselves.

[Soares]

In fact, every major plugin manager already knows how to handle rtp without maktaba's help.

This is where things get murky between maktaba's provision of utility functions and provision of a plugin format. Ideally, any plugin manager that actually adopts maktaba should also forgo its own rtp manipulation in favor of maktaba#rtp (instead of duplicating the functionality).

That said, there probably is a purpose for a "I've already handled the rtp stuff, handle the other stuff" function. Remind me again why we can't just have #Install not manipulate the rtp if it finds that the plugin is already on the rtp? (Or is the problem that checking the rtp is slow?)

[dbarnett]

Remind me again why we can't just have #Install not manipulate the rtp if it finds that the plugin is already on the rtp?

We do that, but the detection isn't quite perfect and probably never will be. When it fails, you get two different versions of the same path added to your runtimepath. For instance, my ~/.vim/ is a symlink, and I end up with both ~/.vim/bundle/maktaba and ~/.vim.mine/bundle/maktaba/. We can probably improve the detection further, but I'm very sympathetic to the user who says "I installed plugin X and suddenly plugins Y and Z are listed twice on my rtp. I spent 2 hours tracking it down to this maktaba code. WTF are you doing?!" so I think we should tread lightly if there's not a strong reason to do otherwise.

[Soares]

When are the settings applied if we aren't l:already_installed?

[dbarnett]

In s:InitPlugin (or, as you would have it, s:LoadInstantFilesAndApplySettings). They're applied in a precise sequence, after the flags are defined but before the instant/ files are sourced.

[Soares]

All right, the feature is fine by me, but I still find the names very confusing. Can we be more explicit about which function does what? I'd suggest LoadInstantFilesAndApplySettings or something very explicit, though this may be too long.

[dbarnett]

I could just inline the code into s:RegisterPlugin if you'd rather. It's only used in that one place, and that way at least you can clearly see the parallel s:ApplySettings in both l:already_installed branches.

Depending on which reads better for you, I could even pivot the whole thing into

    if !l:already_installed
      " Maybe force rtp.
      " Source flags.
    endif
    call s:ApplySettings(a:plugin, a:settings)
    if !already_installed
      " Source the rest of instant/ and define g:installed_PLUGIN.
    endif

[Soares]

The script-local code is fine, I'm worrying about the name of the user-visible maktaba#plugin#Register.

[dbarnett]

Oh, hmm… The name "Install" was unfortunately vague, but I wouldn't want to overcompensate on the "Register" naming. What it does is everything maktaba absolutely must do to fulfill its contract with plugins that depend on maktaba. Something like "Init", "Setup", or "RegisterAndInit" might suggest a bit more of its side-effecty nature, but LoadInstantFilesAndApplySettings implies a false level of precision: this will only load instant files if they haven't already been loaded, and may do other mundane things in the future (minimal things like warn for plugin problems). I'll also point out that it already has a [settings] arg in the function signature (if it's not applying the settings, what the hell else is it doing with them?). And I don't have to tell you what a pain long vimscript function names are. =)

[Soares]

Indeed. If I had known what a clusterfuck this was going to be, I'd have named Install differently. Personally, I'd ignore sunk costs and name Register whatever makes it most clear. The big difference from Install is that it doesn't touch the rtp, yes? Perhaps InstallExistingPlugin or something?