vim-maktaba icon indicating copy to clipboard operation
vim-maktaba copied to clipboard

Should have a maktaba#plugin#Register that doesn't install

Open dbarnett opened this issue 12 years ago • 10 comments

We have maktaba#plugin#Install to install a plugin on the runtimepath and register it with maktaba. But that's a fairly heavy operation that should usually be left to plugin managers, and in many cases we just want to make sure a plugin has been initialized with maktaba without affecting the runtimepath. For instance, both maktaba#plugin#Detect and maktaba#plugin#Enter potentially install the plugin on runtimepath, but in both cases we already know it's on runtimepath and #Install only adds the overhead of rtp manipulation and the potential for stomping on things (#49).

We should implement a maktaba#plugin#Register({dir}) that's a more pure maktaba registration/init utility. Instead of installing plugins on runtimepath, it could either throw an error if it detects a plugin isn't installed or just not even check runtimepath.

dbarnett avatar Feb 01 '14 05:02 dbarnett

In terms of whether to check rtp and throw an error, I can see pros and cons either way.

If we don't check rtp, it'll be a little weird sourcing instant/ files for a plugin that hasn't been installed. In fact, some instant/ files expect to have autoload functions available, and will give cryptic "Unknown function" errors otherwise. So it would be nice to get a descriptive error message instead, and get it consistently.

OTOH, it's nontrivial overhead doing all the path manipulations, and probably even more overhead to do it properly (for instance, as of right now we still get duplicate entries if the paths aren't spelled identically, as in maktaba#rtp#Add('foo/bar') + maktaba#plugin#Install('foo/bar/')).

dbarnett avatar Feb 01 '14 06:02 dbarnett

I also feel that maktaba#plugin#RegisteredPlugins has begun to do more than what the name implies.

It is documented, but we've still lost (after #42) the ability to just query maktaba about what's installed and ready to do maktaba stuff with. In glaive autocompletion especially, which relies on maktaba#plugin#RegisteredPlugins, the candidate list is now cluttered with non-maktaba plugins. And these can't be used with glaive since they don't define flags.

glts avatar Feb 07 '14 13:02 glts

The Glaive autocomplete should check each plugin for flags and filter out the plugins that don't expose any. Our plugin manager story means that we can't count on plugins being explicitly "registered" with maktaba, so the list of registered plugins will be a haphazard thing.

But what do you mean by "ready to do maktaba stuff with"? #Get will give you the plugin object for any plugin listed in #RegisteredPlugins, and instant/ files have already been sourced.

dbarnett avatar Feb 07 '14 15:02 dbarnett

Ok, I see.

It is still a bit hard for me to understand the intended division of labour between maktaba and an as-yet-unseen maktaba-aware plugin manager.

That makes it hard for me to know if I can really afford depending on 'maktaba, the library' in my plugins (which is great!) and not impose 'maktaba, the plugin management' on users.

But this is off-topic. I will lay low for a while and see how things develop. Cheers.

glts avatar Feb 07 '14 16:02 glts

Yep, that's partially because we didn't nail down the division of labor very well from the start. My goal is to fix it to not impose plugin management on anyone. I'm planning to sideline plugin installation as much as possible and get it off of the "critical path" of functionality.

BTW, VAM is already a good plugin manager for maktaba plugins (I added a usage example to our README). But again, my goal isn't to force plugin manager decisions on users. @Soares is working on a proposal for addon-info.json support in Vundle first, hopefully the others soon after.

dbarnett avatar Feb 07 '14 16:02 dbarnett

@Soares What do you think about my initial question, whether it makes sense for Register to verify whether a plugin has been installed? Is it too weird being able to register a plugin without installing it, and is it worth any resultant overhead to enforce that?

dbarnett avatar Feb 09 '14 21:02 dbarnett

You mean we should have one plugin which adds things to the runtimepath and another which does the source-the-instant-files step? I'd prefer to find ways to directly reduce the overhead of the rtp manipulation, but I'm not sure that's feasible.

(Also, this seems like a good time to note how much I despise stringly typed filepath manipulation in every programming language under the sun.)

Soares avatar Feb 10 '14 21:02 Soares

Nope, I don't think that's at all what I mean.

I went ahead and sent a pull req (#60) since it's probably easier to explain when there's code available for you to look at. What I mean is that maktaba#plugin#Install and maktaba#plugin#GetOrInstall add plugin paths to runtimepath, but maktaba#plugin#Register won't, so it can either check and explicitly warn that a path isn't on runtimepath, or just assume the plugin path is already installed on runtimepath and leave it up to you to use it correctly.

I was concerned about the fact that if you don't use it correctly, you might e.g., trigger instant/ files and see them blow up with weird errors because their corresponding autoload functions aren't available. But as I look at it more, I'm not really concerned about it and I think it's fine for us to just skip the overhead of checking rtp (and with it, the chance of erroneously complaining in corner cases).

dbarnett avatar Feb 11 '14 09:02 dbarnett

Discussing the naming with @Soares in #60. "Register" is too vague and we haven't found another name we both like. I'm thinking something like "Init", "Activate", "Setup", or… "Maktabaize"?

Another issue is that the maktaba#plugin API is already a little awkward and gets worse with the new addition. We already have #Install({dir}, [settings]) and #GetOrInstall({dir}, [settings]), and it's unclear what "Install" means. We discussed maybe deprecating these and adding something more explicit.

One interesting suggestion is to handle rtp separately, so

let myplugin = maktaba#plugin#Install('/path/to/myplugin/', ['some', '!flags'])

becomes

call maktaba#rtp#Add('/path/to/myplugin/')
let myplugin = maktaba#plugin#XXXXXX('myplugin', ['some', '!flags'])

This has the advantage that it removes the different "flavors" of installation, makes the rtp manipulation explicit, and makes it impossible to get a plugin handle for a plugin that's not already on rtp since we don't accept arbitrary paths (so I can stop worrying about what to do with plugins that aren't on rtp yet).

Still a few details to sort out, but how does that sound? And do any of these names sound okay, given there will only be one non-deprecated flavor?

dbarnett avatar Feb 19 '14 21:02 dbarnett

This sounds like a good idea to me (modulo concerns about duplicate plugin names on the rtp and so on, which are already problems anyway). I haven't had any insights towards what the names should be, but +1 to the 'splitting up the rtp manipulation and plugin object creation' idea. Those probably never should have been conflated in the first place, given that in the wild, most plugins will be added to the rtp by plugin managers.

Soares avatar Feb 20 '14 00:02 Soares