supermaven-inc / supermaven-nvim

The official Neovim plugin for Supermaven
https://supermaven.com/
MIT License
279 stars 16 forks source link

fix(#52): Set the colorscheme when lazyloading the plugin #53

Open arnevm123 opened 3 weeks ago

arnevm123 commented 3 weeks ago

When the plugin gets loaded we call the function first, so VimEnter is not needed anymore. If you would like to fix this another way, please let me know. fixes #52

AlejandroSuero commented 3 weeks ago

@arnevm123, in my personal opinion the changes of setting the highlight without listening to an event should be done in init.lua since its not listening to the document itself, then in the document_listener.lua just changing it to listen to the event ColorScheme it's fine.

arnevm123 commented 3 weeks ago

Then we'd have to make a setupColors function in util or a separate file, as the code in your PR is a bit too convoluted to be maintained in 2 places.

AlejandroSuero commented 3 weeks ago

I thought the same thing, that's why I said it as an opinion. I mean the code still gets the job done as it is and it's not like it needs to be in the init.lua or it won't work.

sm-victorw commented 3 weeks ago

There are some users, (e.g. as mentioned in #49 ) who seem to have their own logic involving require("supermaven-nvim.completion_preview").suggestion_group and I'm wondering if running preview.suggestion_group = "SupermavenSuggestion" would break this setup

e: although I suppose that would have been the intended/original effect, and it is only working right now due to this bug

AlejandroSuero commented 3 weeks ago

@sm-victorw I tested it and it seems if I have this config:

require("supermaven-nvim").setup({
  color = {
    suggestion_color = vim.api.nvim_get_hl(0, { name = "NonText" }).fg,
    cterm = vim.api.nvim_get_hl(0, { name = "NonText" }).cterm,
  },
})
require("supermaven-nvim.completion_preview").suggestion_group = "SupermavenSuggestion"

If lazy != false and event != "" it will indeed break.

If doing what I suggested in https://github.com/supermaven-inc/supermaven-nvim/pull/53#issuecomment-2154582714 it won't break.

AlejandroSuero commented 3 weeks ago

@sm-victorw @arnevm123 here is a demo of the changes, first to appear is when setting it initially in init.lua, second is how the changes are in this PR.

https://github.com/supermaven-inc/supermaven-nvim/assets/71392160/266e9b03-ccf4-496b-b5fa-1616561fed1f

AlejandroSuero commented 3 weeks ago

One more thing, if doing the same with the PR #51 it will default to Comment when "breaking", if the changes of this PR or the aforementioned one, the first one that gets merged will have to add the "setting a default" at the point of initialising the plugin.