Skip to content

feature: Improve background colors of signs highlight groups #58

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

Closed
kmoschcau opened this issue Jul 10, 2024 · 9 comments
Closed

feature: Improve background colors of signs highlight groups #58

kmoschcau opened this issue Jul 10, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@kmoschcau
Copy link

kmoschcau commented Jul 10, 2024

Is your feature request related to a problem? Please describe.

Currently it looks like the signs added by this plugin use the background color of the Normal highlight group. This leads to effects like this:
image

Describe the solution you'd like

Here are three possible solutions:

  • Do not set a background color for the signs.
  • Use the background color of the SignColumn highlight group instead.
  • Provide a way to configure the sign highlight groups separately.
@kmoschcau kmoschcau added the enhancement New feature or request label Jul 10, 2024
@kmoschcau kmoschcau changed the title feature: Use the background color of the SignColumn highlight group for signs or provide a way to configure sign highlights feature: Improve background colors of signs highlight groups Jul 10, 2024
@MeanderingProgrammer
Copy link
Owner

MeanderingProgrammer commented Jul 10, 2024

Thanks for bringing this up, that is definitely unexpected.

The signs added by this plugin don't add a background highlight group.

In the case of code we use only the highlight group returned from the icon provider. So in the case of mini.icons & python we get MiniIconsYellow and only set that: https://github.com/MeanderingProgrammer/markdown.nvim/blob/main/lua/render-markdown/handler/markdown.lua#L92-L101

Then in the case of headings we use only the value of the foreground highlight group: https://github.com/MeanderingProgrammer/markdown.nvim/blob/main/lua/render-markdown/handler/markdown.lua#L37-L59

In both of these cases we do not add Normal as a background.

I've played around with a few other color schemes and am unable to replicate the background being different, though I can clearly see it in your screenshot. What colorscheme do you use?

@kmoschcau
Copy link
Author

kmoschcau commented Jul 11, 2024

I use my own theme, which I generate with this lua tool. For the icons I'm using mini.icons, which has highlight groups with only a foreground color defined.

Also there's no such thing really as background and foreground highlight groups. Highlight groups can have both a foreground and background color. If you'd want to make sure to only use the foreground color of an existing highlight, you'd need to extract that from the highlight group.

@MeanderingProgrammer
Copy link
Owner

Yeah, I call them foreground since the highlights seem to only define guifg (at least for me).

Which icon provider are you using?

If it's mini.icons then the highlight group for python resolves to MiniIconsYellow which for me only has a guifg defined.

If it's nvim-web-devicons then the highlight group for python resolves to DevIconPy which only has ctermfg and guifg defined.

I'm just rather lost on how a background is getting set on these icons for you. Is there some fallback logic happening somewhere? As far as I'm aware the solution Do not set a background color for the signs. is already the case.

@kmoschcau
Copy link
Author

I use mini.icons. And I just checked myself: MiniIconsYellow only has guifg and ctermfg.
It's a bit difficult to inspect the final result. I could just use :Inspect, if it was in a buffer, but for signs I'd need some other way.

I just looked through my tool again and tested a bit: Signs highlight groups do not behave different from other highlight groups. Anything that's not specified for a highlight group falls back to what's specified for Normal. Meaning, for signs you have to specifically set the background to match that of the SignColumn highlight group. That's what I do for all sign highlights I have defined.

@MeanderingProgrammer
Copy link
Owner

Ah, I was wondering why I wasn't able to get something to display this behavior. Turns out that most color schemes use the same background color for Normal as SignColumn. Found one that doesn't and now see the same thing, will work on a fix, probably need to create some custom merged highlights.

MeanderingProgrammer added a commit that referenced this issue Jul 12, 2024
…d by buftype

# Details

Request: #58

Not all color schemes use the Normal background color for the sign
column. This can lead to some not so great looking icons. Fix this
by combining the color we were going to set with the SignColumn
background. Add a new configuration section 'sign' where this is
defined and moifiable.

While we are adding a new section for signs add 2 other abilities.

- The ability to globally disable them from components that create them
- The ability to disable them within a set of buftypes, use a default
  value of 'nofile' so that signs do not appear in LSP documentation
@MeanderingProgrammer
Copy link
Owner

Added the logic to use the SignColumn background here: d398f3e

Please update and lmk if it looks better!

Also added the ability to disable signs on specific buftypes, defaults to nofile. Will follow up in the other thread with that.

@kmoschcau
Copy link
Author

Yep, that works like a charm!
image
image

@MeanderingProgrammer
Copy link
Owner

Sweet! Btw for the optional fields there's an equivalent type called 'render.md.UserConfig'. I have a script to generate one from the other so they stay in sync.

@kmoschcau
Copy link
Author

Ah! Good to know. I admit it was a subtle attempt to get it fixed or info on how to improve it on my side. 😄
Thanks again for all the work you put in. This is really a great plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants