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(colors): defer loading colors if colors is a function #190

Closed
wants to merge 1 commit into from

Conversation

mrcjkb
Copy link

@mrcjkb mrcjkb commented Apr 10, 2024

Hey 👋

This PR originates from this rocks.nvim thread.

The problem in the discussion is that the colorscheme plugin, mini.base16, requires a setup function to configure the colour palette. If the plugin is configured after this plugin's colours are loaded, the palette is not applied.

heirline.load_colors accepts a function as an input, and one would expect the function to be evaluated lazily, when needed.
But, it is evaluated immediately, essentially making the fact that you can pass it a function useless.

This PR proposes a change to lazily load the colours when they are needed, if the argument passed into heirline.load_colors is a function.
As a result, users don't have to worry about the order in which they configure their plugins.

Apart from that, the behaviour is unchanged.

@bew
Copy link

bew commented Apr 11, 2024

Would it then make sense (maybe in another PR) to call this function again after the colorscheme was changed?

@mrcjkb
Copy link
Author

mrcjkb commented Apr 11, 2024

Would it then make sense (maybe in another PR) to call this function again after the colorscheme was changed?

Hmm, that sounds like an interesting idea. Is there a way to detect colorscheme changes?

@bew
Copy link

bew commented Apr 11, 2024

Is there a way to detect colorscheme changes?

There is the ColorScheme autocmd

@rebelot
Copy link
Owner

rebelot commented Apr 11, 2024

colors() is evaluated immediately because its main purpose was to be used during ColorScheme events as described here, providing users a way to execute some code to redefine the colors on colororscheme changes.

This PR should address colorscheme changes (see utils.on_colorscheme)

However...

If the plugin is configured after this plugin's colours are loaded, the palette is not applied.

could doautocmd ColorScheme after loading such plugin solve this?

@mrcjkb
Copy link
Author

mrcjkb commented Apr 11, 2024

If the plugin is configured after this plugin's colours are loaded, the palette is not applied.

could doautocmd ColorScheme after loading such plugin solve this?

The affected user probably didn't have such an autocommand set up. I suppose if they had, they wouldn't have encountered this issue.

@rebelot
Copy link
Owner

rebelot commented Apr 11, 2024

The affected user probably didn't have such an autocommand set up. I suppose if they had, they wouldn't have encountered this issue.

Then I guess this pretty much invalidates this PR... I really appreciated your effort but I think that relying on the ColorScheme autocmd to reset highlights and color definitions makes more sense and solves the issue in the simplest way

@mrcjkb mrcjkb closed this Apr 11, 2024
@mrcjkb mrcjkb deleted the colors branch April 11, 2024 10:48
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.

3 participants