search

preservim / vimux

easily interact with tmux from vim
MIT License
2.19k stars 158 forks source link

Cleanup public/private API‌ split & coding style #184

Closed alerque closed 3 years ago

alerque commented 3 years ago
alerque commented 3 years ago

These changes are going to conflict with #183, but I think they should be made separate PRs and‌ I don't mind untangling the conflicts. This would be a breaking change if I hadn't already broken it in #181 / e6fb662 where I changed the pseudo-private _VimuxOption() to a proper script local s:VimuxOption(). This should have been an easy refactor, but it turns out lots of people and plugins were using this private API as if it was public.

And I think it's reasonable to use publicly, which means it was always misnamed and shouldn't have been even pseudo-private. This PR makes it officially public, giving an upgrade path for things using the obsolete pseudo-private function to switch to rather than hiding it from them completely.

alerque commented 3 years ago

Honestly this is a disaster. I've been doing some code searches and there is a long tail of people doing things with methods that were intended to be private. Usually these could have been done better with public function anyway so I don't have a lot of sympathy. In some cases the private functions themselves are redefined and then used locally for ... reasons that are unclear to me. In many cases they are just hacking in bits from PRs that were not merged timely enough.

The proper fix is probably to move the whole plugin to autoload and refactor all the private bits to be fully documented public methods. Most of them are shallow wrappers for preferences anyway which will have a better public API soon anyway (see #183) so they won't need their own wrappers.