srcery-colors / srcery-vim

Srcery is a dark color scheme with clearly defined contrasting colors and a slightly earthy tone.
https://srcery.sh
Other
824 stars 60 forks source link

Add option to set bg to hard_black for gui. #104

Closed micangl closed 1 year ago

micangl commented 1 year ago

Srcery already has a vim-only option to set the terminal background to hard_black. This commit adds another option, g:srcery_hard_black_gui_bg, compatible with the termguicolors (both Vim and Neovim) set-option.

micangl commented 1 year ago

This was already partially possible by modifying the value of g:srcery_black in one's own config. Needless to say, this is all but an elegant solution.

roosta commented 1 year ago

This looks good, but I was a bit confused by the option name, gui makes me think of gvim, not the background. How about naming the option srcery_high_contrast_bg? Possibly we should rename option srcery_hard_black_terminal_bg to srcery_high_contrast_terminal_bg What do you think @MindTooth ?

roosta commented 1 year ago

We should probably mention in the readme that these options are exclusive, so bg passthrough wont work if you set this option, and vice versa.

MindTooth commented 1 year ago

Would it be better that the background could take a value instead? Instead of having two variables controlling the same thing. 🤷🏻‍♂️ Might be that I'm missing something, but are they the same?

roosta commented 1 year ago

Agree, that would be better. The way it is now is confusing, and cause issues if you have more than one background option set.

We could allow a custom background value, and default to regular black.

micangl commented 1 year ago

Having the option take a specific RGB value would definitely be an improvement.

micangl commented 1 year ago

The issue with g:srcery_hard_black_terminal_bg is that it is necessary for users without set termguicolors in their config. So we should probably preserve it, whilst adding a new g:srcery_bg option - mutually exclusive with the former - which would take a hexadecimal RGB value.

micangl commented 1 year ago

I've noticed that I was wrong in the last message. It's possible to remove the g:srcery_hard_black_terminal_bg option, since the highlight function takes as the guibg argument a list of of two elements: [<gui_background>, <cterm_background>]. In the last commit I've set the to 0, since I didn't know how you'd like the user to be able to customize the color. What I mean, by this, is: What option should we give the user to let him customize the gui and cterm bg colors? A possible option would be to make the g:srcery_bg variable a list, which could then be passed directly to the highlight function: let g:srcery_bg = [<guibg>, <ctermbg>]. A drawback is that the user would have to configure manually both the gui and cterm values, even if he were to be only interested in one. The only other option I can think of is to have two variables, each with a default value (so that the user could only set the one he's interested in), and then combine them in a list (maybe, by defining a global varible containing the list, or directly passing the variable in a list to the highlight function).

roosta commented 1 year ago

Sorry about the delay on this, I've been swamped with projects and not had time to look at this yet. I'll have to test this branch a bit, not sure about the best approach yet. Is it possible to set the background to none in your current configuration?

micangl commented 1 year ago

Is it possible to set the background to none in your current configuration? Isn't that what the g:srcery_bg_passthrough option is for?

roosta commented 1 year ago

yes, but the idea was to combine all the background options into a single var, instead of having mutually exclusive options.

micangl commented 1 year ago

We could try something like this inside autoload/srcery.vim:

if !exists('g:srcery_bg')
  "Sets default for both guisp and cterm backgrounds.
  let g:srcery_bg=[g:srcery_black, 0]
elseif (index(g:srcery_bg, 'DEFAULT') >= 0) || (index(g:srcery_bg, 'NONE') >= 0 && has('gui_running'))
  "Defaults should be set if the user specifies it, or if the background is set as 'NONE' whilst the gui is running.
  for i in [0, 1]
    if g:srcery_bg[i] == 'DEFAULT' || (g:srcery_bg[i] == 'NONE' && has('gui_running'))
      let g:srcery_bg[i] = g:srcery_black
    endif
  endfor
endif

If the user wants to set the background as none, he can simply specify 'NONE', as this is what Srcery internally uses.

roosta commented 1 year ago

yeah, something like that could work. Feel free to push more commits, if you got a working implementation. I'm thinking that the terminal background remains a separate option cause I can imagine someone wanting a different background for only that.

micangl commented 1 year ago

I'm thinking that the terminal background remains a separate option cause I can imagine someone wanting a different background for only `\that.

What do you mean exactly by this?

roosta commented 1 year ago

Just that I'd like that to remain a separate option, looks like it is right now, viewing a8842ea. Just forget I mentioned it :)

EDIT: For for clarity, I'm refering to this variable g:srcery_hard_black_terminal_bg

micangl commented 1 year ago

I haven't updated the documentation yet. To specify the background colors, add the following line to your config:

 let g:srcery_bg = [<guibg>, <ctermbg>]

To leave one of the two to the default value set it to 'DEFAULT'. To set it to s:none, use 'NONE'.

roosta commented 1 year ago

Thank you for your contribution! Appreciate your willingness to modify the PR, and I'm happy with the new option, gives more flexibility.

micangl commented 1 year ago

No problem. I still need to update the documentation, though. Will do it during next week.