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

Open
arnevm123 wants to merge 1 commit from arnevm123/52-apply-colorscheme-when-lazyloading into main
arnevm123 commented 2024-06-07 04:36:03 -04:00 (Migrated from github.com)

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

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 2024-06-07 06:47:10 -04:00 (Migrated from github.com)

@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, 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 2024-06-07 09:16:18 -04:00 (Migrated from github.com)

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.

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 2024-06-07 11:56:54 -04:00 (Migrated from github.com)

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.

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 2024-06-11 15:41:54 -04:00 (Migrated from github.com)

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

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 2024-06-11 16:08:38 -04:00 (Migrated from github.com)

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

@sm-victorw I tested it and it seems if I have this config: ```lua 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 2024-06-11 16:12:59 -04:00 (Migrated from github.com)

@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

@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 2024-06-11 16:19:26 -04:00 (Migrated from github.com)

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.

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.
thesimonho commented 2025-02-23 06:48:54 -05:00 (Migrated from github.com)

whats currently holding up this PR from being merged? currently it seems setting suggestion color is broken with or without lazy loading...

whats currently holding up this PR from being merged? currently it seems setting suggestion color is broken with or without lazy loading...
AlejandroSuero commented 2025-03-15 05:50:26 -04:00 (Migrated from github.com)

@thesimonho ngl, I think this project's kinda abandoned since they starting exploring doing a code editor and then fused with an existing one.

I'll try to create a repo from this base one and try to maintain it properly.

@thesimonho ngl, I think this project's kinda abandoned since they starting exploring doing a code editor and then fused with an existing one. I'll try to create a repo from this base one and try to maintain it properly.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin arnevm123/52-apply-colorscheme-when-lazyloading:arnevm123/52-apply-colorscheme-when-lazyloading
git switch arnevm123/52-apply-colorscheme-when-lazyloading

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff arnevm123/52-apply-colorscheme-when-lazyloading
git switch arnevm123/52-apply-colorscheme-when-lazyloading
git rebase main
git switch main
git merge --ff-only arnevm123/52-apply-colorscheme-when-lazyloading
git switch arnevm123/52-apply-colorscheme-when-lazyloading
git rebase main
git switch main
git merge --no-ff arnevm123/52-apply-colorscheme-when-lazyloading
git switch main
git merge --squash arnevm123/52-apply-colorscheme-when-lazyloading
git switch main
git merge --ff-only arnevm123/52-apply-colorscheme-when-lazyloading
git switch main
git merge arnevm123/52-apply-colorscheme-when-lazyloading
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
supermaven-inc/supermaven-nvim!53
No description provided.