goolord / alpha-nvim

a lua powered greeter like vim-startify / dashboard-nvim
MIT License
1.84k stars 109 forks source link

refactor: use lua api for buf and win opts #66

Closed kylo252 closed 2 years ago

kylo252 commented 2 years ago

The window opts are backed up first because they are not local and will be inherited otherwise on alpha.close()

kylo252 commented 2 years ago

This also opens the possibility of making these configurable.

P.s. I had actually done this since yesterday, before the noautocmd, but I didn't want to include it in that other PR.

goolord commented 2 years ago

i want to avoid using the api for win options until the window option api is complete, otherwise this is inviting bugs https://github.com/neovim/neovim/issues/14670

the win_opts_backup trick doesn't prevent the window options from being messed with when you open a file in a split for instance. it's also difficult for me to predict the horrible ways this might interact with users' (frankly sometimes spaghetti) vim configs, which is why i avoided doing this

believe me every call to vim.cmd in this plugin makes me immensely sad, but i'll just be patient while things are worked on upstream ` ~ `

goolord commented 2 years ago

i'll happily accept the buffer part as is tho, even though it would be a little awkward for buffer options to be configurable idiomatically but not window options

since the buffer is

        buftype = "nofile",
        bufhidden = "wipe",
        buflisted = false,

it shouldn't rly matter that the api is mildly inconsistent for buffer local options too

kylo252 commented 2 years ago

the win_opts_backup trick doesn't prevent the window options from being messed with when you open a file in a split for instance. it's also difficult for me to predict the horrible ways this might interact with users' (frankly sometimes spaghetti) vim configs, which is why i avoided doing this

hmm, I think I see what you mean but still, why would someone open a split in their dashboard? 😢


Anyways, the new commit should still be an improvement overall!

goolord commented 2 years ago

why would someone open a split in their dashboard

i actually do this sometimes when i want to open multiple files in the startify theme's MRU section

goolord commented 2 years ago

filetype=alpha should also be in the body of vim.cmd just to keep the benefits of noautocmd

i guess this is another gap in the api

kylo252 commented 2 years ago

filetype=alpha should also be in the body of vim.cmd just to keep the benefits of noautocmd

I tried benchmarking that noautocmd and I couldn't get any consistent results tbh. I can still move that call there if you want.

goolord commented 2 years ago

yeah it's within margin of error on my machine with my config too, but the person who requested it said that it "visibly reduce[s] startup time " https://github.com/goolord/alpha-nvim/issues/49

i don't understand what config is possibly causing this, maybe using https://github.com/sheerun/vim-polyglot or treesitter or something? but searching for FileType autocmds should always be O(1) in my mind and there isn't even anything to execute by default so i don't understand why i or anyone would even see any improvement

goolord commented 2 years ago

well, maybe nvim has to scan your rtp or something, which isn't free

kylo252 commented 2 years ago

i don't understand what config is possibly causing this, maybe using sheerun/vim-polyglot or treesitter or something? but searching for FileType autocmds should always be O(1) in my mind and there isn't even anything to execute by default so i don't understand why i or anyone would even see any improvement

One more thing, that benchmark is flawed in the sense that it's still actually using the default runtimepath. Instead, you should be using a custom one with a limited set of pre-defined plugins. I tried with both Packer and Impatient turned off and enabled, and the results are basically a wash for me. And that RNG call at the end also makes things even more nondeterministic, imo.