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

refactoring: split theme.lua into separate groups. #51

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

killitar
Copy link
Contributor

Hi, thanks for the plugin. I refactored it, but I don't know how to do the switching between dark and light themes. I'll have a look at how to do that. Sorry for open and close previous PR, I split one commit into more smaller commits.

Fixes #48.

@killitar killitar marked this pull request as draft December 17, 2024 23:56
@redoxahmii
Copy link
Collaborator

Thank you for the work I was meaning to do this myself once i had some time.
If you mean how the day colors are done it inverts the colors that are already defined by converting them hsluv color space which you can find more info on how in the hsluv.lua file.
If you mean how to set it in your editor to see :colorscheme solarized-osaka-day should work fine.
I just took a quick glance in your commits and you seem to have removed the part that inverts the colors and highlights in theme.lua on line 1026 so that might be your clue to why.

@killitar
Copy link
Contributor Author

killitar commented Dec 18, 2024

If possible I would like to split the different themes into files, like it is tokyonight. And don't use the colour inversion function inside the theme.lua file.

@redoxahmii
Copy link
Collaborator

We'll have to wait for @craftzdog to approve cause this is a huge change and see if that's something he thinks should be considered.
In my opinion this just creates an extra part to maintain while the previous one is functional and defeats the purpose of using hsl color space.
This still isn't a strong opinion as I don't use day mode enough to say it has faults for how it works currently.

@redoxahmii redoxahmii requested a review from craftzdog December 18, 2024 08:56
@killitar
Copy link
Contributor Author

killitar commented Dec 18, 2024

You're right. I have implemented theme switching using the invert function. If @craftzdog decides to split themes according tokyonight, another PR can be opened in the future.

@killitar killitar marked this pull request as ready for review December 23, 2024 15:37
@craftzdog
Copy link
Owner

Hi, thanks for the work!
Regarding the day mode, the original Solarized themes have light and dark modes, and this theme is based on the dark one.
If you are interested in adding the day mode, it should rather refer to the light one than just inverting the palette.

Here is the reference: https://ethanschoonover.com/solarized/

I'm happy to review this PR. Could you resolve the conflicts?

@killitar
Copy link
Contributor Author

Thanks for the opportunity to work on this PR. It was one of my first big PRs. If I did anything wrong please post it. I tried to do my best, but there may be some mistakes, as I'm just learning how to work with Git and open source projects. I would really like to work on another PR after the holidays. I would like to separate the Kinds that are used in lsp symbol, сmp and blink.cmp as it is done in tokyonight.. What are your thoughts on this?. Merry Christmas @craftzdog. Thanks for the great 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.

Refactoring: Split theme.lua into groups
3 participants