Initialize aerial as a plugin
With this, Aerial works just by being present in runtime path. No requires are called on initialization, which should keep its impact on startup time to the minimum.
Configuration is read from vim.g.aerial_nvim_config on the first
load of aerial.config, highlights are similarly created on the
first execution of aerial.highlight. This allows one to set up
configuration adjustments in, for example,
.config/nvim/plugin/aerial.lua and have the plugin just work, if it's
present.
I've intentionally dropped an eager command repeat on potentially loading state. If information is not present, it's simply not present. With LSP, it's natural to wait for LSP to initialize, with TreeSitter current symbols fetcher is synchronous, so waiting for parsing is baked in.
I don't use recession, and with no tests for recession integration, this change might've broken something.
While this partially reverts an older move away from global configuration, I believe that this PR achieves the best of both worlds by using global configuration once with an ability to use .setup call to reapply changes in configuration or apply a separate configuration altogether.
I'm not sure what makes "update docs" task fail on CI. When isolating it locally, it passes without any problems.
Primary goal - the plugin works just by adding it. This is an editor plugin, not a runtime library.
Secondary goal - when plugin is not yet present (e.g. initial setup, performance triage, testing out other plugin) the default editor just works. Plugins should gradually enhance user experience with as little demand for intervention as possible. Plugin configuration presence shouldn't throw errors if plugin is not present.
As for slow global access - it's not access that's slow, it's the lookup. It introduces about 1000x overhead on access, and in old aerial code it was used directly, if I rememeber correctly. In this PR it's performed exactly once if one uses defaults, twice if one sticks to .setup routine.
Here's an admittedly basic benchmarks I performed with https://github.com/luapower/time/blob/master/time.lua:
local func = function(box, mess, times)
local _start, _end, _breaker
_start = M.clock()
for i = 1, times do
_breaker = box.something
end
_end = M.clock()
vim.print(string.format(mess, (_end - _start) * 1000))
end
M.test_global = function()
vim.cmd('let g:test_glob = {"something": "value"}')
local test_loc = {something = "value"}
local times = 10000000
local _start, _end, _breaker
_start = M.clock()
for i = 1, times do
_breaker = vim.g.test_glob.something
end
_end = M.clock()
vim.print(string.format("Global access took %0.2fms", (_end - _start) * 1000))
_start = M.clock()
for i = 1, times do
_breaker = test_loc.something
end
_end = M.clock()
vim.print(string.format("Local access took %0.2fms", (_end - _start) * 1000))
local glob_ref = vim.g.test_glob
_start = M.clock()
for i = 1, times do
_breaker = glob_ref.something
end
_end = M.clock()
vim.print(string.format("Local ref to global access took %0.2fms", (_end - _start) * 1000))
func(test_loc, "Passed local ref access took %0.2fms", times)
func(vim.g.test_glob, "Passed global ref access took %0.2fms", times)
end
Results:
Global access took 2282.84ms
Local access took 2.24ms
Local ref to global access took 2.33ms
Passed local ref access took 2.19ms
Passed global ref access took 2.18ms
The noise about startup time comes exactly from the proliferation of .setup approach - we have a poisoned ecosystem of plugins that demand users to do require lookups early on. You won't find it mirrored in vim land, which is an evidence that core lazy loading routines are doing fine.
Ah, apologies I should have been more clear: performance was the impetus for switching from global-variable configuration to the current system, but I do not believe that your PR would introduce any performance problems. I put that in just to flesh out the history, but should have realized it would come across sounding like I was still worried about performance.
Primary goal - the plugin works just by adding it. This is an editor plugin, not a runtime library.
This is a high-level guiding principle, but what is the actual problem? I can think of two possibilities:
- Adding a
require("aerial").setup()line to configuration is undesirable (presumably for the reasons below) - Doesn't align with how most vanilla vim plugins work
Is there anything else that I'm missing?
Secondary goal - when plugin is not yet present (e.g. initial setup, performance triage, testing out other plugin) the default editor just works. Plugins should gradually enhance user experience with as little demand for intervention as possible. Plugin configuration presence shouldn't throw errors if plugin is not present.
If I'm interpreting this correctly, you're saying that you want the code that configures the plugin to not throw errors if the plugin is not installed. This makes it easier to try different plugins, have different configs, or just :help bisect. Is that a fair statement?
I'm currently still in the camp of using .setup(), and I'll explain my reasoning below, but I do want to make sure that I'm not overlooking anything. I think it's entirely possible for us to look at the same set of tradeoffs and disagree on which one is better, as that's a value judgement, but I want to ensure we're starting from a shared understanding of what those tradeoffs are. So if it seems like there's something I haven't considered, do let me know.
There are three main reasons I prefer the .setup() pattern:
- It's a convention. Regardless of if the convention is good or bad, it still comes with benefits. Users are more likely to be familiar with it, so there's less to grok when getting set up. Other tools are built that leverage this interface (e.g. lazy.nvim offers super sick configuration merging of
opts, which is passed into.setup()) - Calling
.setup()is explicit and imperative. It puts the user in complete control of when the plugin is initialized. If you leverage aplugin/file to do this, it's completely possible to mess up the initialization order because it's no longer visible exactly when these scripts will run. I have been bitten by this before. - It's the way the plugin currently works. Adding a new mechanism adds more complexity. It's another thing to maintain, it's another thing to document, it's another thing for users to grok as they're getting set up.
None of these are overwhelming reasons. Honestly the differences between the two mechanisms are not that significant, but these do seem like positives, and they make me mildly inclined to use .setup(). Additionally, I don't find that the global variable pattern is necessary to achieve the goal of avoiding errors during configuration. It's very easy to do:
local has_aerial, aerial = pcall(require, "aerial")
if has_aerial then
aerial.setup()
end
Maybe a bit more verbose than vim.g.aerial = { ... }, but it has advantages. You can explicitly choose what to do in the case that aerial is available, or is not available.
local has_aerial, aerial = pcall(require, "aerial")
if has_aerial then
aerial.setup()
else
vim.notify("Plugin missing: aerial", vim.log.levels.WARN)
end
Also, as soon as the configuration gets more involved by adding keymaps and autocmds, you're back to square one. Sure the vim.g.aerial = won't cause problems, but you'll have broken keymaps and autocmds. The way to fix that is by checking if aerial is installed or not with pcall(require, "aerial"), but at that point it seems like nothing is gained over .setup().
The problem ultimately is that when I add a plugin, I expect it to just work.
I don't initialize ftplugins, I don't initialize InspectTree, man, shada, or matchit plugins. In the same vein, I strive to have other plugins to simply work.
That aside, I do want to offer a deeper look into the arguments. I did spend last five of six years considering all the same things, funnily enough.
.setup being a convention is a self-defeating argument. Simply because plugin and ftplugin are older,wey better established, and actually codified conventions. For a time, neovim lacked support for .lua plugin files and many plugin writers either didn't want to deal with lua <<LUA or, which I see evidence for a lot, didn't know how neovim loaded plugins. Nowadays, setup is a bad pattern, because it forces an extra filesystem access when it least makes sense.
I do agree with explicitness of .setup call. There is a beauty to it that I desire as a software developer when I require a library, which is why I made sure to keep it working. Still, aerial.nvim is not a library, it's an editor plugin.
The new mechanism is an old mechanism with complexity removed. It replaces the old mechanism by merging .setup and vim.g... routes. I explicitly pursued this design to keep existing users oblivious, new users happy, and maintenance cost the same at worst.
As for lazy.nvim, it's not a standard and doesn't seem likely to become one. It's nice to acomodate it, but the standard that every plugin should absolutely work with is vim packages. I, for example, eschew lazy because it ironically does way too much.