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

Vim in MS Terminal mistakes color of ctermfg=7 #15408

Open
3N4N opened this issue May 23, 2023 · 13 comments
Open

Vim in MS Terminal mistakes color of ctermfg=7 #15408

3N4N opened this issue May 23, 2023 · 13 comments
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@3N4N
Copy link

3N4N commented May 23, 2023

Windows Terminal version

1.18.1241.0

Windows build number

10.0.22621.1702

Other Software

Vim 9.0.1472

Steps to reproduce

I have the following colorscheme in MS Terminal:

        {
            "background": "#FAFAFA",
            "black": "#383A42",
            "blue": "#0184BC",
            "brightBlack": "#4F525D",
            "brightBlue": "#61AFEF",
            "brightCyan": "#56B5C1",
            "brightGreen": "#98C379",
            "brightPurple": "#C577DD",
            "brightRed": "#DF6C75",
            "brightWhite": "#FAFAFA",
            "brightYellow": "#E4C07A",
            "cursorColor": "#383A42",
            "cyan": "#0997B3",
            "foreground": "#383A42",
            "green": "#50A14F",
            "name": "Yellowish",
            "purple": "#A626A4",
            "red": "#E45649",
            "selectionBackground": "#61AFEF",
            "white": "#FCF4DC",
            "yellow": "#C18301"
        }

With this colorscheme activated, run vim as follows.

vim --clean -c 'se ls=2 | hi StatusLine cterm=none ctermbg=8 ctermfg=7'

You'll see a statusline with black blacground, which is expected with ctermbg=8, and brightBlack foreground, whereas you should expect a white foreground with ctermfg=7.

Expected Behavior

With help of colortool, I modified Console Host to have the same colorscheme as MS Terminal. Here is the .ini scheme for easy reproduction:

[info]
name = Custom
author = enan

[table]
DARK_BLACK     = 56,58,66
DARK_BLUE      = 1,132,188
DARK_GREEN     = 80,161,79
DARK_CYAN      = 9,151,179
DARK_RED       = 228,86,73
DARK_MAGENTA   = 166,38,164
DARK_YELLOW    = 193,131,1
DARK_WHITE     = 252,244,220
BRIGHT_BLACK   = 79,82,93
BRIGHT_BLUE    = 97,175,239
BRIGHT_GREEN   = 152,195,121
BRIGHT_CYAN    = 86,181,193
BRIGHT_RED     = 223,108,117
BRIGHT_MAGENTA = 197,119,221
BRIGHT_YELLOW  = 228,192,122
BRIGHT_WHITE   = 250,250,250

[screen]
FOREGROUND = DARK_BLACK
BACKGROUND = BRIGHT_WHITE

[popup]
FOREGROUND = DARK_MAGENTA
BACKGROUND = DARK_WHITE

If you run the same command to start Vim, you'll now see a statusline with white foreground, which is what's expected from ctermfg=7.
expected

Actual Behavior

In MS Terminal, ctermfg=7 does NOT use the white color defined in the color table/scheme.

actual

In Console Host, ctermfg=7 DOES use the white color defined in the color table/scheme.

@3N4N 3N4N added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 23, 2023
@3N4N
Copy link
Author

3N4N commented May 23, 2023

Here is an email thread on vim_dev mailing list where I tried to investigate this: https://groups.google.com/g/vim_dev/c/SuVLCni9WCw/m/loPupdoZBAAJ.

The emails didn't reveal anything. There were too many miscommunications and misunderstandings and I eventually gave up. But since then I found out that this problem happens only in MS Terminal, not Console Host. That's why I decided to create this ticket.

@zadjii-msft
Copy link
Member

Thoughts from the dome without digging in too deep - I bet this is the powershell default background compatibility shim. That's my hunch. @DHowett might be sure.

@DHowett
Copy link
Member

DHowett commented May 23, 2023

Thoughts from the dome without digging in too deep - I bet this is the powershell default background compatibility shim. That's my hunch. @DHowett might be sure.

That shim only works for processes named powershell.exe! I'll have to look at this.

@j4james
Copy link
Collaborator

j4james commented May 23, 2023

Yeah, I don't think the compatibility shim is a factor here. This is just standard behavior for apps using the legacy console APIs (which I think is the case for the Windows version of vim).

It's a little more complicated than this, but VT applications generally have access to 18 colors (default foreground and background, along with normal and bright variants of the 8 ANSI colors), while the legacy console APIs only have access to 16 (just the 16 ANSI colors). The "default" colors for legacy console apps are typically 7 on 0 (white on black), so we map those to the default foreground and background in your Terminal color scheme, since that's what most people expect to see. The actual 7 and 0 colors in the color scheme aren't used by legacy console apps.

So what's happening here is you've selected color number 8 (#4F525D) as the background, and color number 7, which maps to the default foreground (#383A42) as the foreground. Hence the black on black.

@j4james
Copy link
Collaborator

j4james commented May 23, 2023

The actual 7 and 0 colors in the color scheme aren't used by legacy console apps.

Actually this bit isn't entirely correct. Color 7 may still be used as a background color, and color 0 may still be used as a foreground color. Color 7 only maps to the default foreground when it's used as a foreground color, and color 0 only maps to the default background when it's used as a background color.

Not that this makes any difference to your particular situation here, but I don't want to mislead you with any incorrect information.

@3N4N
Copy link
Author

3N4N commented May 24, 2023

So what's happening here is you've selected color number 8 (#4F525D) as the background, and color number 7, which maps to the default foreground (#383A42) as the foreground. Hence the black on black.

That was initial assumption, before I looked into anything. But I thought it was Vim that was doing it -- it being mapping "color number 7 [...] to the default foreground (#383A42)." I genuinely thought it must be something as simple as an if conditional in Vim source code (e.g., if (cmd=="ctermfg" && color=7) color=black), but now that you say it boils down to the difference in legacy API and VT, that makes more sense.

Thanks for the quick responses. Let me know if you need any testing done. I have a passable knowledge of Vim codebase; maybe I can help.

@j4james
Copy link
Collaborator

j4james commented May 25, 2023

@3N4N Just from a brief scan of the code, it appears that vim does at least have the potential to render these color correctly, because there's an option in the code to use VT sequences instead of the legacy console APIs. For example, see here:

https://github.com/vim/vim/blob/097c5370ea8abab17ceb0f3bcd74f57b1655c7f7/src/os_win32.c#L6690-L6698

But that support looks incomplete, as there are still a number of places where they appear to be using the old console APIs. If that is the case, it wouldn't be surprising to see some issues with the colors.

But if you're in a position to compile and debug vim, you can confirm if it's doing something wrong by putting breakpoints wherever you can find a SetConsoleTextAttribute call. Possibly also FillConsoleOutputAttribute. If you find those breakpoints are being hit, then that's potentially something that vim might need to fix.

@3N4N
Copy link
Author

3N4N commented May 25, 2023

But if you're in a position to compile and debug vim [...]

I am. I'll do as you suggested. And just to write down my thoughts: after grepping for '(SetConsoleTextAttribute|FillConsoleOutputAttribute)', I see that some of these are wrapped in conditionals of if(!vtp_working) with the else branch using the VT sequences. This tells me that at one point someone started adding VT support, but didn't complete the process.

@3N4N
Copy link
Author

3N4N commented May 25, 2023

I put breakpoint on textcolor() and noticed that when I do hi StatusLine ctermfg=7, it uses the VT alternative, not the legacy SetConsoleTextAttribute() method. But it still results in a black foreground when color code 7 should be white. So j4james' hunch, that maybe the problem was being caused by using legacy API, is wrong. Am I right, @j4james?

Maybe the problem is in how VT sequencing is implemented in Vim? Probably not. I disable VTP support in Vim, and the problem still exists in MS Terminal and still isn't raised in Conhost. Which means both API and VT sequence behaving the same way.

@j4james
Copy link
Collaborator

j4james commented May 25, 2023

I've just downloaded and built the latest vim, and I've been doing some experimenting from within the debugger. While it seems to be using VT sequences for some things, it's also definitely calling the console APIs some of the time as well. The vtp_working variable is set to true, so that triggers VT sequences in some places, but the USE_VTP macro evaluates to false, so that results in SetConsoleTextAttribute being called in other places.

One way you can confirm that it's using console attributes is if you set your cterm variables to something like ctermbg=4 ctermfg=1. That should be red on blue when using VT sequences, but it'll be blue on red when using the console APIs, because they use a different color numbering scheme, and vim doesn't appear to take that into account.

So the next question is, how do you get USE_VTP to be true? One way is to set the number of colors to 256 with set t_Co=256. As soon as you do that, you should see the blue on red colors flip to red on blue. And if you're using your original 7 on 8 cterm variables, you should see it switch from the almost invisible black text to the expected yellowish white on black.

@3N4N
Copy link
Author

3N4N commented May 25, 2023

So the next question is, how do you get USE_VTP to be true? One way is to set the number of colors to 256 with set t_Co=256. As soon as you do that, you should see the blue on red colors flip to red on blue. And if you're using your original 7 on 8 cterm variables, you should see it switch from the almost invisible black text to the expected yellowish white on black.

I found :set t_Co=256 method earlier. But it had another issue. With :hi! clear Normal, it uses black bg and white fg, even though the terminal emulator's color scheme is set to use white as bg and black as fg. I have to do hi Normal ctermbg=7 ctermfg=0. That's a solution, but the utility of using :hi! clear Normal (which is the default, btw) is that it adapts automatically when you change your terminal emulator's color scheme.

I knew about :set t_Co=256 but I didn't know that it sets USE_VTP. Now I know that. Thanks.

@carlos-zamora carlos-zamora added Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Issue-Task It's a feature request, but it doesn't really need a major design. and removed Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 31, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label May 31, 2023
@carlos-zamora carlos-zamora added this to the Backlog milestone May 31, 2023
@carlos-zamora carlos-zamora added Product-Conpty For console issues specifically related to conpty Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. and removed Needs-Tag-Fix Doesn't match tag requirements labels May 31, 2023
@3N4N
Copy link
Author

3N4N commented Jun 7, 2023

@j4james do you have any response to the vim-bg-black-in-white-terminal-with-t_Co-256 issue? (See my previous comment if the hyphenated phrase doesn't make sense.)

@carlos-zamora I'm not familiar with the labels on this repository. Is this issue (in this ticket) a bug? Cause I'm not sure this is a bug. I see that @zadjii-msft linked it as related to another issue, but being a layman in the domain of terminal stuff I can't follow it. Lemme know if I need to do or provide anything.

@j4james
Copy link
Collaborator

j4james commented Jun 7, 2023

@3N4N Sorry, I didn't do any further investigation into what was going on there. But from what I've already seen of the code, I'm just assuming it's vim's problem until proven otherwise.

If it were up to me, I'd dump their current win32 code, and replace it with the whatever they're using on Linux, since that clearly already works in Windows Terminal when vim is run from WSL. It's probably a little more complicated than that, but there really should be no need for them to be using this weird mix of legacy console APIs interspersed with VT sequences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

5 participants