dracula / vim

🧛🏻‍♂️ Dark theme for Vim
https://draculatheme.com/vim
MIT License
1.34k stars 454 forks source link

Don't call `require` on gitsigns to allow lazy-loading it #301

Closed staticssleever668 closed 1 year ago

staticssleever668 commented 1 year ago

Instead, check if it has ever been loaded before.

Now it's possible to lazy-load gitsigns, and then reload Dracula to apply the colorscheme: runtime after/plugin/dracula.vim.

benknoble commented 1 year ago

Will this continue to work for people that don't lazy-load gitsigns? For such people, doesn't this code working now depend on the order in which the two plugins are loaded? In general that seems brittle and likely to result in confusion.

staticssleever668 commented 1 year ago

You are right, sorry.

Replicating:

" Assuming vim-plug is installed.
" Run `:PlugUpdate` once with this config to install the plugins.
call plug#begin("/tmp/test_plugins")
    Plug 'dracula/vim', { 'as': 'dracula' }

    Plug 'lewis6991/gitsigns.nvim'
call plug#end()

" At this point `package.loaded.gitsigns ~= nil` is `false`.

" Load Dracula without setting colors for gitsigns.
colorscheme dracula

" Load gitsigns.
lua require("gitsigns").setup()

" Now `package.loaded.gitsigns ~= nil` is `true`, but we already missed loading Dracula.

I tried to find a good way to work around it, but I couldn't. It's possible to do it with package.searchers, but it feels very unreliable and needs some Lua code.

I've thought of adding g:loaded_gitsigns variable to gitsigns, but it doesn't really make sense as it's not actually doing anything until require is called. (https://github.com/lewis6991/gitsigns.nvim/issues/94#issuecomment-800147759)

Looking at how some other themes change the color scheme (e.g. gruvbox.nvim or tokyonight.nvim), they seem to do it unconditionally. Maybe Dracula should do the same?

benknoble commented 1 year ago

Maybe Dracula should do the same?

Perhaps. I'm sure @dsifford and I would be willing to have this discussion with any interested.

bluz71 commented 1 year ago

With my own themes moonfly and nightfly I came to the conclusion that doing something like the following is sub-optimal inside a color theme:

if has('nvim-0.5') && luaeval("pcall(require, 'gitsigns')")

In my case, on my machine, that added 5ms to the runtime of my theme. Hence, it is much better, performance wise, to be unconditional for Gitsigns instead of calling pcall(require.

My 2cents.