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

Issues Related to "reverse" color schemes #807

Closed
chuck-flowers opened this issue Sep 1, 2023 · 18 comments
Closed

Issues Related to "reverse" color schemes #807

chuck-flowers opened this issue Sep 1, 2023 · 18 comments
Labels
bug Something isn't working

Comments

@chuck-flowers
Copy link

Description

This is related to another issue that I opened and was partially fixed (#793).

Instead of a hard error now, my highlighted changes are not legible because I believe the foreground and background highlighting are the same color.

Neovim version

NVIM v0.9.1
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3

Operating system and version

WSL2 Ubuntu 22.04 / Arch Linux Latest

Steps to reproduce

  1. Open neovim in a project with git changes
  2. Open the neogit status page
  3. Highlight

Expected behavior

I expect the highlighted code blocks to be readable instead of solid colors (like in the below image)

Actual behavior

image

Minimal config

-- gruvbox config (lazy)
return {
	'ellisonleao/gruvbox.nvim',
	config = function()
		local gruvbox = require 'gruvbox'

		gruvbox.setup({
			transparent_mode = false
		})

		vim.opt.termguicolors = true
		vim.o.background = 'dark'
		vim.cmd('colorscheme gruvbox')
	end
}

-- neogit config (lazy)
return {
	'NeogitOrg/neogit',
	dependencies = {
		'nvim-lua/plenary.nvim',
		'nvim-telescope/telescope.nvim',
		'sindrets/diffview.nvim'
	},
	opts = {},
	keys = {
		{
			'<leader>gs',
			function()
				require('neogit').open()
			end
		},
		{
			'<leader>gc',
			function()
				require("neogit").open({ "commit" })
			end
		},
		{
			'<leader>gl',
			function()
				require("neogit").open({ "log" })
			end
		}
	}
}
@chuck-flowers chuck-flowers added the bug Something isn't working label Sep 1, 2023
@zdcthomas
Copy link

zdcthomas commented Sep 11, 2023

I'm seeing the exact same behavior

@chuck-flowers
Copy link
Author

@zdcthomas I might take a crack at writing a fix this weekend if I have time.

@kekscode
Copy link

Just for the record: I see the same issue using gbprod/nord.nvim colorscheme.

@CKolkey
Copy link
Member

CKolkey commented Sep 12, 2023

IMO the real fix is to open a PR for your colorschemes and get them to support all the highlight groups.. All the highlight logic I added is (mostly) a stop-gap. But, by all means, I'd happily accept improvements :)

@chuck-flowers
Copy link
Author

chuck-flowers commented Sep 12, 2023

IMO the real fix is to open a PR for your colorschemes and get them to support all the highlight groups.. All the highlight logic I added is (mostly) a stop-gap. But, by all means, I'd happily accept improvements :)

Do you have any recommended resource for understanding the semantics of "reverse"? If I understand its purpose better I'd be able to make an appropriate PR in the colorscheme or maybe add better fallback logic to neotest.

EDIT: It was actually a lot simpler than I thought. I've fixed the issue by defining the following as links:

  • NeogitChangeAdded
  • NeogitChangeDeleted
  • NeogitDiffAdd
  • NeogitDiffDelete
  • NeogitDiffAddHighlight
  • NeogitDiffDeleteHighlight

I'll test these changes to the color scheme out for a few days and then submit them as a PR to gruvbox

@jamesbvaughan
Copy link

jamesbvaughan commented Sep 12, 2023

I'm seeing this with gruvbox.nvim. I can try making a PR there this week. I might wait until you're finished @chuck-flowers so that I can use yours as an example.

edit: Whoops, I got the comments about gruvbox and nord mixed up in my head and missed that you were already talking about fixing this wrt gruvbox.

@chuck-flowers
Copy link
Author

@jamesbvaughan My fork of gruvbox is on my profile. You can checkout the neogit branch to see what changes I made.

@zdcthomas
Copy link

What are the highlights being used that gruvbox doesn't support rn?

@chuck-flowers
Copy link
Author

@zdcthomas I'm assuming he's talking about the Neogit* highlights. When I defined those in my fork, everything works properly.

@zdcthomas
Copy link

Then it feels like those shouldn't be expected to exist in all available color schemes, and instead by default they should be bound to another highlight that's more standard no?

@CKolkey
Copy link
Member

CKolkey commented Sep 12, 2023

Thats...exactly what's going on: https://github.com/NeogitOrg/neogit/blob/master/lua/neogit/lib/hl.lua#L56-L90 🤷🏼

@zdcthomas
Copy link

zdcthomas commented Sep 12, 2023

Oh sorry! I'm super behind then

@CKolkey
Copy link
Member

CKolkey commented Sep 12, 2023

All good :) No joke, this has been the hardest thing to get "right" since I did the major overhaul to this project. (and arguably it's still not right)

@sealor
Copy link
Contributor

sealor commented Jan 12, 2024

I'm using onedark with a light background and also have issues with "invisible" text.

What do you think about the following approach?
Let's link all main highlight groups (especially with text) to the default vim highlight groups. Doing so will eliminate all problems with unreadable text. Furthermore the basic colors of the current scheme will be applied automatically.
For more advanced colors the color scheme should implement specific colors for every provided highlight group.

@CKolkey
Copy link
Member

CKolkey commented Jan 14, 2024

I'll go ahead and close this since @sealor did some work to improve the light colorscheme situation. Feel free to reopen if you're still experiencing issues :)

@CKolkey CKolkey closed this as completed Jan 14, 2024
@elC0mpa
Copy link

elC0mpa commented Feb 15, 2024

IMO the real fix is to open a PR for your colorschemes and get them to support all the highlight groups.. All the highlight logic I added is (mostly) a stop-gap. But, by all means, I'd happily accept improvements :)

Do you have any recommended resource for understanding the semantics of "reverse"? If I understand its purpose better I'd be able to make an appropriate PR in the colorscheme or maybe add better fallback logic to neotest.

EDIT: It was actually a lot simpler than I thought. I've fixed the issue by defining the following as links:

  • NeogitChangeAdded
  • NeogitChangeDeleted
  • NeogitDiffAdd
  • NeogitDiffDelete
  • NeogitDiffAddHighlight
  • NeogitDiffDeleteHighlight

I'll test these changes to the color scheme out for a few days and then submit them as a PR to gruvbox

Could you please explain me how to do this?
I am trying to get it right for gruvbox, assume that defining the highliht colors will fix it, but don´t know how to do it

@awalvie
Copy link

awalvie commented Feb 22, 2024

Running into the exact same problem with nord as well. Would appreciate if we could get some generalized docs about how to go about setting these highlight groups for our respective color schemes.

@CKolkey
Copy link
Member

CKolkey commented Feb 23, 2024

What would you like to see beyond https://github.com/NeogitOrg/neogit/blob/master/doc/neogit.txt#L199-L330 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants