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

Update terminal Visual #13940

Closed
wants to merge 2 commits into from
Closed

Update terminal Visual #13940

wants to merge 2 commits into from

Conversation

habamax
Copy link
Contributor

@habamax habamax commented Jan 30, 2024

  1. Use ctermbg=Grey ctermfg=Black for both dark and light

This uniforms Visual highlighting between default dark and light colors And should work for vim usually detecting light background for terminals with black/dark background colors.

Previously used ctermfg=White leaks cterm=bold if available colors are less than 16.

  1. Use term=reverse cterm=reverse ctermbg=NONE ctermfg=NONE for terminals reporting less than 8 colors available

If the terminal has less than 8 colors, grey just doesn't work right

1. Use `ctermbg=Grey ctermfg=Black` for both dark and light

This uniforms Visual highlighting between default dark and light colors
And should work for vim usually detecting light background for terminals
with black/dark background colors.

Previously used `ctermfg=White` leaks `cterm=bold` if available colors
are less than 16.

2. Use `term=reverse cterm=reverse ctermbg=NONE ctermfg=NONE`
   for terminals reporting less than 8 colors available

If the terminal has less than 8 colors, grey just doesn't work right
@habamax
Copy link
Contributor Author

habamax commented Jan 30, 2024

Screenshots with vim -Nu NONE:

autodetected background=light and t_Co=256

image

switch it to dark with set bg=dark:

image

autodetected background=light and manually set t_Co=16

image

switch it to dark with set bg=dark:

image

autodetected background=light and manually set t_Co=8

image

switch it to dark with set bg=dark:

image

autodetected background=light and manually set t_Co=2

image

switch it to dark with set bg=dark:

image

@habamax
Copy link
Contributor Author

habamax commented Jan 30, 2024

Here I have used one of @romainl suggestion mentioned in #13663 (comment)

This visual obviously would not work if terminal background is for real light and has it as 7.

Both @romainl and @neutaaaaan also believe (I am in the same camp) something other than grayish should be used for visual (yellow? cyan?), but I will leave it for the next iterations and/or discussions.

@chrisbra
Copy link
Member

Thanks.
Comments from anybody else?

@neutaaaaan
Copy link

I think it would be a better idea to work on the entire colorscheme at once, rather than attempting to fix discrete elements, knowing we're likely to modify them again later.

For this reason I don't have much of an opinion on this change.

@chrisbra
Copy link
Member

chrisbra commented Feb 1, 2024

I started that change, because it was explicitly only mentioned that Visual has a bad UX. And I thought it was only Visual. Now I am confused why you want to change other highlighting groups as well.

Having said that, I believe it's a good thing to only gradually change single highlighting groups and not making a big change of several groups at once. For once, it's easier to bisect and/or revert and I like having small atomic individual changes, so we can get feedback if a single group highlighting is problematic for some use cases, although improving the whole default color scheme will then take longer.

@romainl
Copy link

romainl commented Feb 1, 2024

@chrisbra Visual is the tip of the iceberg. The linked thread mentions a bunch of other issues with default: Search, diff*, Statusline, etc.

Fixing those in default is just as worthy as fixing Visual and, like @neutaaaaan I believe that fixing the whole thing in one go is not only possible but also easier than doing it group-by-group. Moreover, it would mean keeping the "whole picture" in mind and prevent things like the recent Diff* debacle. Also, given the way Vim is distributed, it would increase the chances for regular users to get a nice usable default earlier instead of having to deal with a continuously broken one for years.

@chrisbra
Copy link
Member

chrisbra commented Feb 1, 2024

If you want to go ahead and improve other default highlighting groups, please go ahead and create a separate PR. We can discuss the UX in this more specific PR then. But I won't merge until I got a fair amount of feedback that the new defaults are good.

nice usable default earlier instead of having to deal with a continuously broken one for years.

I don't buy that argument. We have had the default now for 20 years? And it's not exactly broken.

@romainl
Copy link

romainl commented Feb 1, 2024

If you want to go ahead and improve other default highlighting groups, please go ahead and create a separate PR. We can discuss the UX in this more specific PR then. But I won't merge until I got a fair amount of feedback that the new defaults are good.

That's, like… the whole purpose of the thread you linked to, over at vim/colorscheme. No one asked you to rush things out in #13663 at all and now another commit is needed to fix that hasty commit of yours. Fixing Visual is just one of the many things that should be made and no one in their right mind is thinking that it should be done in hasty commits, and reverts and fixes and whatnot. Remember the Diff* debacle? I do whenever I do a commit and now thousands of devs have to suffer through that crap because of mindless commits. Those things must be done carefully.

nice usable default earlier instead of having to deal with a continuously broken one for years.

I don't buy that argument. We have had the default now for 20 years? And it's not exactly broken.

That is not an argument, more like an observation. I wasn't there 20 years ago but I can assure you that default has been consistently sucking since at least 7.3, which was the version I started with. The horrendous diff, the visual selection, the crazy dark blue on black, etc. The sorry state of default (and of the other OG built-in colorschemes) is why we have more colorschemes in the scripts section of vim.org than any other types of scripts.

That… and of course Vim's legendary inability to correctly set &background, of course, or deal with italics.

That's a lot of things to fix. For now, let's focus on what we can do for default over there and not waste everyone's time with botched drive-by commits.

@chrisbra
Copy link
Member

chrisbra commented Feb 1, 2024

No one asked you to rush things out in #13663 at all and now another commit is needed to fix that hasty commit of yours

That wasn't rushed, it was after considering feedback I received.

now thousands of devs have to suffer through that crap

Wow, thanks for your honest feedback. :( Can we please calm down a bit and keep it on a professional level? Thank you.

That… and of course Vim's legendary inability to correctly set &background, of course, or deal with italics.

as far as I remember that was not easily portably possible. You cannot blame Vim for this.

That's a lot of things to fix. For now, let's focus on what we can do for default vim/colorschemes#250 and not waste everyone's time with botched drive-by commits.

Fine, I am awaiting updates then.

@chrisbra chrisbra closed this in 59bafc8 Feb 1, 2024
zeertzjq added a commit to zeertzjq/neovim that referenced this pull request Feb 15, 2024
Problem:  Visual highlighting can still be improved
Solution: Update Visual highlighting for 8 color terminals,
          use uniform grey highlighting for dark and light bg
          (Maxim Kim)

Update terminal Visual

1. Use `ctermbg=Grey ctermfg=Black` for both dark and light

This uniforms Visual highlighting between default dark and light colors
And should work for vim usually detecting light background for terminals
with black/dark background colors.

Previously used `ctermfg=White` leaks `cterm=bold` if available colors
are less than 16.

2. Use `term=reverse cterm=reverse ctermbg=NONE ctermfg=NONE`
   for terminals reporting less than 8 colors available

If the terminal has less than 8 colors, grey just doesn't work right

closes: vim/vim#13940

vim/vim@59bafc8

Co-authored-by: Maxim Kim <habamax@gmail.com>
glepnir pushed a commit to glepnir/neovim that referenced this pull request Mar 31, 2024
Problem:  Visual highlighting can still be improved
Solution: Update Visual highlighting for 8 color terminals,
          use uniform grey highlighting for dark and light bg
          (Maxim Kim)

Update terminal Visual

1. Use `ctermbg=Grey ctermfg=Black` for both dark and light

This uniforms Visual highlighting between default dark and light colors
And should work for vim usually detecting light background for terminals
with black/dark background colors.

Previously used `ctermfg=White` leaks `cterm=bold` if available colors
are less than 16.

2. Use `term=reverse cterm=reverse ctermbg=NONE ctermfg=NONE`
   for terminals reporting less than 8 colors available

If the terminal has less than 8 colors, grey just doesn't work right

closes: vim/vim#13940

vim/vim@59bafc8

Co-authored-by: Maxim Kim <habamax@gmail.com>
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.

None yet

4 participants