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

Method used for highlighting breaks with colors set by theme #178

Closed
litoj opened this issue Nov 22, 2022 · 11 comments
Closed

Method used for highlighting breaks with colors set by theme #178

litoj opened this issue Nov 22, 2022 · 11 comments

Comments

@litoj
Copy link
Contributor

litoj commented Nov 22, 2022

When I set the colors of control_hl_groups in my theme, it breaks upon switching to dark/light variant, where I update only the black/white colors. It seems that after setting these groups they all get cleared, while the groups like DapUIScope and all other outside control_hl_groups stay fine:

DapUIStop      xxx cleared
DapUIRestart   xxx cleared
DapUIStepOver  xxx cleared
DapUIStepInto  xxx cleared
DapUIStepOut   xxx cleared
DapUIStepBack  xxx cleared
DapUIPlayPause xxx cleared
DapUIUnavailable xxx cleared
DapUIThread    xxx links to Fg

This causes an error in config/highlights.lua:74 since you are trying to format nil.
Also there is no reason to use vim.cmd for highlighting when 0.8 features setting the groups aside of just retrieving them.
I will try to rewrite all highlight groups to lua and see if that solves the issue. If I can get it solved that way, I would like to issue a PR.

@litoj
Copy link
Contributor Author

litoj commented Nov 22, 2022

Also I don't think there is a need to set colors multiple times, because no colorscheme clears groups set by itself, so if the groups were set, they will stay so anyway. It would be better to set all groups on setup and then dont touch them again. Also, is there a particular reason why the value of control groups is copied instead of linking to the group? Do the groups have different use case from non-NC counterparts?

@litoj
Copy link
Contributor Author

litoj commented Nov 22, 2022

The colors are being cleared by patch_background().

@litoj
Copy link
Contributor Author

litoj commented Nov 22, 2022

The reason is when using highlight cmd separately for fg and bg, each call overrides the previous one. Also, default isn't used so colors set by colorscheme are overwritten and when no bg is set, the result command is to set the group to empty background - ending with the group being cleared.

@litoj
Copy link
Contributor Author

litoj commented Nov 22, 2022

I understand you're trying to synchronize the background color of WinBar with all icons, but if no background is used, it should automatically be the color of background. that is, if the WinBar group highlight covers the entire line and on top are highlighted the separate icons.

@litoj
Copy link
Contributor Author

litoj commented Nov 22, 2022

Also, why is this method launched only for nvim 0.8+. there is no reason to use cmd as a table, the same can be done as for all the previous commands. This also means that nvim pre-0.8 users aren't currently getting the winbar different color anyway. Is patch_background() necessary at all then?

Only way to change the backgroud and keep the foreground is by highlighting with WinBar[NC] the entire line where these groups are used or copy the WinBar color at the same time as copying non-NC to NC groups.

@nyngwang
Copy link
Contributor

nyngwang commented May 6, 2023

I sincerely don't think what you did in #180 is correct. Currently, any colorscheme that has DapUINormal set will not be picked(some set of values was being used instead, I got a different one from the set during debugging.) It's interesting that you did a lot of reasoning for this though.

@litoj
Copy link
Contributor Author

litoj commented May 7, 2023

I am not sure I understand what you mean. What exactly do you mean by not being picked? What sets of values are being used, if they are different from the defaults, where do they come from, if not the colorscheme?

@litoj
Copy link
Contributor Author

litoj commented May 7, 2023

The problem might be I still don't understand the philosophy behind ...NC groups. But mainly I was hoping to get at least some response from the maker of the original PR that introduced this issue, because, as you can see from the comments I posted, there are couple of things in the color handling, that don't make much sense to me. I only wanted to understand it better so I could make a proper fix - cleaning it up. This way I only made changes that didn't touch things appart of what I considered broken.

@nyngwang
Copy link
Contributor

nyngwang commented May 7, 2023

Hi, first of all, thanks for your kind explanation.

the philosophy behind ...NC groups.

From my current observation, ...NC(means "not current") groups are designed to do something similar to what this plugin does: https://github.com/sunjon/Shade.nvim. But I don't use it since "adding shadow/shade" is just an eye candy application to help you find the current window faster. (btw, I did encounter some tricky issues there, i.e. it caused some troubles that eliminate its benefits)

What exactly do you mean by not being picked? [...] What sets of values are being used, if they are different from the defaults, where do they come from, if not the colorscheme?

For the first question, I meant/tried:

  1. Even if I have added DapUINormal in my colorscheme, it is either cleared or set to some strange value from nowhere (I tried :verb hi DapUINormal to find the source but apparently it's not useful.)
  2. I have to add vim.cmd('hi DapUINormal') right before the command dapui.open({ reset=true }) to make it take effect. So there must be some plugin(or maybe it's nvim-dap-ui itself) defining the same highlight group again in the middle.

For the second question: I don't know. The point is that the problem only happened on nvim-dap-ui.

@litoj
Copy link
Contributor Author

litoj commented May 7, 2023

I tried to find how the colorscheme you use works. I found this line, which might be something, but this would execute only if a different color scheme was set. Otherwise it works same as mine, so I don't see the issue there.

Edit:
But even when calling hi clear, after changing the colorscheme the Colorscheme event will notify dapui to update, so this still shouldn't be an issue. Sorry, I am out of ideas on this one.

@nyngwang
Copy link
Contributor

@JosefLitos Good news: the linked PR #276 resolves this :)

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

No branches or pull requests

2 participants