sodapopcan / vim-twiggy

Git branch management for Vim
280 stars 15 forks source link

Move mappings, syntax, etc. into ftplugin and syntax folders #19

Closed stellarhoof closed 5 years ago

stellarhoof commented 5 years ago

This will (hopefully) prevent re-running code that is supposed to be executed only once (buffer local mappings, etc...) as well as clean up the code a bit. The code in the ftplugin directory is run on setfiletype .... Also moved syntax statements under the syntax/ directory.

sodapopcan commented 5 years ago

Hiya, thanks for the PR. Unfortunately, I'm not able to accept it for a number of reasons.

Right off the bat, I can't merge anything that hopefully achieves something—I would need actual proof. Even if it did improve performance, it would be such a micro-optimization that the amount of changes this PR introduces wouldn't be worth it to me anyway. Also, defining mapping and syntax rules are incredibly cheap operations.

To dig into it, the ftplugin stuff isn't doing what I think you you think it is. Because the twiggy buffer gets destroyed every time it's closed, all the mappings have to be redefined anyway, so even with your changes here, stuff in ftplugin is still being run every time twiggy is opened.

Lastly, I'm just not interesting in having my project organized this way. I understand wishes to split stuff up, but deciding to author vim plugins means you've signed up to deal with a lot of hairy-ness. I hate it when projects expose functions publicly (ie, plugin#foo() autoloaded functions) that are actually considered private. Aside from making me wonder if it's safe to use them or not, they're always barfing all over my tab-completion!

stellarhoof commented 5 years ago

Fair enough. The focus of this PR is not on performance though, but I hear you, everyone's got their styles and preferences. I still think there's room for improvement on this plugin (from my point of view), but there are so many things I've got in mind that I wonder if it'd be better to make my own. In any case, I'll present another version (even more invasive and hopefully :) simpler) to see if you like it.

sodapopcan commented 5 years ago

I figured since you said the point was to prevent re-running mappings and syntax that your focus was performance but yes, while there are of course many good refactoring candidates here, I can guarantee I won't accept anything that massively re-organizes/refactors in one fell swoop. When it comes to viml, I prefer to have as few files as possible and will outright refuse anything that introduces a bunch of twiggy#foo() functions. Not trying to be rude, just letting you know before you pour a bunch of time into something.

I'd definitely encourage you to write your own as it's always good to have alternative solutions! Otherwise, if it's just that you don't like twiggy and you don't feel like starting a project from scratch, merginal might be to your taste.