Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arnevm123
Copy link

@arnevm123 arnevm123 commented Jun 7, 2024

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
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Collaborator

sm-victorw commented Jun 11, 2024

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
Copy link
Contributor

@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 #53 (comment) it won't break.

@AlejandroSuero
Copy link
Contributor

AlejandroSuero commented Jun 11, 2024

@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.

document_listener-vs-init_lua.mp4

@AlejandroSuero
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing color does not work when Lazy loading
3 participants