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

Add light theme #35

Merged
merged 2 commits into from
May 31, 2021
Merged

Add light theme #35

merged 2 commits into from
May 31, 2021

Conversation

Nargonath
Copy link

@Nargonath Nargonath commented May 23, 2021

This PR is an attempt to fix #24.

To easily toggle between the light/dark background I believe we need to rewrite the colors/spaceduck.vim file and change its structure. For now I only included an example of how I intend to do it. I believe we'll need to remove hi Spaceduck statements in the &background if blocks since they won't be needed anymore once we start using the palette dictionary. I added a custom function to keep the highlight call DRY. I also just changed the hi Normal call to showcase the function use case.

@jinh0 jinh0 self-assigned this May 26, 2021
@jinh0
Copy link
Collaborator

jinh0 commented May 26, 2021

Hey @Nargonath thanks for making the local highlight function! This was something @pineapplegiant and I have discussed for some time now, though we never got to it. I'm glad you did!

Regarding rewriting the entire colors/spaceduck.vim file with your local highlight function, we need to take this step-by-step.

  1. Replace the color variables with the local highlight function.
  2. Replace all remaining highlight groups not linked to the color variables
  3. Improve the highlight function

One improvement for the highlight function necessary is taking arguments for gui= and cterm= such as

hi SpellBad guifg=#e33400 ctermfg=166 guibg=NONE ctermbg=NONE gui=underline cterm=underline

hi StatusLineTermNC guifg=#000000 ctermfg=0 guibg=#30365F ctermbg=237 gui=reverse cterm=reverse

gui= and cterm= is usually set to NONE but sometimes need different arguments. I think this will be a really quick fix.

I'll test out your code and merge it with the dev branch if there is no further problem.

Thanks!

@Nargonath
Copy link
Author

Alright thank you @jinh0 for the feedback. I'll add your suggestion to my MR.

@jinh0 jinh0 marked this pull request as ready for review May 31, 2021 01:11
@jinh0 jinh0 marked this pull request as draft May 31, 2021 01:11
@jinh0 jinh0 marked this pull request as ready for review May 31, 2021 01:12
@jinh0 jinh0 merged commit 52f53ba into pineapplegiant:dev May 31, 2021
@jinh0
Copy link
Collaborator

jinh0 commented May 31, 2021

Hey @Nargonath , sorry for being late. I merged it, adding the gui=NONE option for the highlight function. Everything works 😃 . For now, I changed the way to view the light theme in progress. You'll have to have let g:spaceduck_dev_light_theme = 1 in your vim config to view the light theme. I thought this to be the prudent choice so that nobody accidentally stumbles upon the horror that is the current light theme.

Thanks for the pull request! Looking forward to working more with you!

@Nargonath Nargonath deleted the light-theme branch May 31, 2021 06:45
@Nargonath
Copy link
Author

I didn't think you'd want to merge this for now. Good call for the custom variable trigger. I'll open a new PR once I make more progress on the light theme then.

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.

2 participants