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

bold/bright foreground [1m is fixed to white #244

Closed
hgkamath opened this issue Jul 16, 2020 · 9 comments
Closed

bold/bright foreground [1m is fixed to white #244

hgkamath opened this issue Jul 16, 2020 · 9 comments
Labels
bug Something isn't working Windows Issue applies to Microsoft Windows

Comments

@hgkamath
Copy link

hgkamath commented Jul 16, 2020

Describe the bug

  • The bright-text/bold-text, term escape code [1m in the formatted output of man-pages that is paged by pagers such as "less" is fixed to white. As a result, bold-text on light background (dark text on light background ) terminal themes the text is unreadable. This is the 2nd row in the output of the color-test script.
  • Also, the 2nd to last row [37m has dark foreground instead light. It is as though there is a swap, seems to be fixed to black.
  • Also, the 2nd column [40m has invisible background, seems to be fixed to background color.

I suspect, but could be wrong, that this is an upstream bug with color mapping as they pass through conpty layer.
Filing this bug so as ascertain if it really is upstream and how a fix will come into wezterm at some point of time.

render: defer conversion of TextColor into COLORREF until actual rendering time (avoid ConPTY narrowing bugs) #2661
color "brightWhite" and its usage in WSL man pages #2864
bold characters are always brightWhite regardless of foreground color #3022
Under Solarized Light, the text returned by findstr is not visible #3630
ConPTY does not support black background #293
Improve the propagation of color attributes over ConPTY #6506
My understanding is that it has something to do with color-mapping happening inside conpty.

Workaround:

  • Use a dark background theme (light foregrounds on dark background)
  • Find and use a relatively dark light background theme (dark foregrounds on light background) such as "Novel"

Environment (please complete the following information):

  • OS: Windows-10 (2004) 19041.388
  • Version: test-build built on July 12th
    Wezterm -V
    wezterm 20200620-160318-e00b076c-29-g8266d409

To Reproduce

  • Set any/various light background theme from iterm2colorschemes.com
  • Download Giles Orr's terminal color test script color-test.sh
  • Execute the color-test script and observe the 1m row

Configuration

nothing unusual. same as issue #235

Expected behavior

  • A man page, that uses a pager less, should be more readable. The bold foreground term color [1m should significantly stand out, if making brighter does not make sense, then it should be bolder and heavier.
  • Term color tests should match designers' sample renderings.

Screenshots

theme : Novel
Screenshot (6)

Screenshot (5)

Session Recording

NA
If the issue is with the way that escape sequences are processed it can be helpful
to capture the terminal output using the wt-record
script to run wezterm and record a transcript. This requires the script utility
to be installed on your system (this is part of macOS and available in the util-linux
package on linux systems).

In the example below a file named 20180225161026.tgz is produced. Please attach that
file to this issue, or if it contains private or sensitive issue that you don't want the
public to see on GitHub, please find some other way to get that file to a project
contributor (perhaps Dropbox or email?).

$ ./wt-record
Transcript recorded in 20180225161026.tgz

You can use wt-replay 20180225161026.tgz to replay that file.

wt-record can only record the terminal output; it cannot record the input events going
in to the terminal, so if you are having an issue with input, please be sure to describe
it below!

Additional context

Add any other context about the problem here.

@hgkamath hgkamath added the bug Something isn't working label Jul 16, 2020
@wez wez added the Windows Issue applies to Microsoft Windows label Jul 17, 2020
@wez
Copy link
Owner

wez commented Jul 17, 2020

Yeah, this sounds like a conpty issue amongst the various issues you linked, but I hadn't noticed it because I'm always either running in the default config (which a dark background) or with my personal config which overrides the color of bold text:

return {
  font_rules= {
    {
      intensity = "Bold",
      font = font_with_fallback("Operator Mono SSm Lig Bold", {
         -- you can override the default bold text color with this
         foreground = "tomato"
       }),
    },
  }
}

@DHowett
Copy link

DHowett commented Jul 17, 2020

FWIW: The latest builds of OpenConsole out of that repository can be used for testing the fix to this issue. 😄

@wez
Copy link
Owner

wez commented Jul 17, 2020

@DHowett ah, good to hear! I'll make a point of updating those and trying them out over the weekend

@hgkamath
Copy link
Author

hgkamath commented Jul 17, 2020

I looked at links like askubuntu.com-bold and stackoverflow-bold and tried some things out. I confirmed that the bold term escape sequence is what leads to the near invisible color.

I also confirmed that, in the wezterm configuration, the font rule foreground override had no effect on bold, though it would work for italic. The bold font could be changed to say "Courier New" but not its color. The italic font, would take both color and font.

:
  font_rules={
    {
      intensity="Bold",
      font=wezterm.font("Courier New", {
        -- you can override the default bold text color with this
        foreground="tomato",
      })
    },
    {
      italic=true,
      font=wezterm.font("Consolas Italic", {
        -- you can override the default italic text color with this
        foreground="tomato",
      })
    },
  },
:

From the below screen captures, it seems like the 1m row foreground is dark when inside openconsole.exe

wezterm

weztern_slol_ight

openconsole_orig.exe that came bundled with wezterm

had to rename the exe to _orig.exe as I was readying to copy another openconsole.exe from windows-terminal to wezterm folder
colortest.exe
E__apps_win_wezterm_WezTerm-windows-20200620-160318-e00b076c-29-g8266d409_OpenConsole_orig exe 7_16_2020 9_36_40 PM
the bash script
E__apps_win_wezterm_WezTerm-windows-20200620-160318-e00b076c-29-g8266d409_OpenConsole_orig exe 7_16_2020 11_02_28 PM

openconsole.exe that is bundled in the windows-terminal cascadia package 1.1.1812.0 :

I copied the exe to the wezterm folder, after unzipping its installer.
openconsole_20200716

@DHowett
Copy link

DHowett commented Jul 17, 2020

Sorry about that! I was referring to the top of tree latest state in the repository, though I see now how that was unclear. We'll be preparing a release pretty soon with the new OpenConsole. :)

wez added a commit that referenced this issue Jul 17, 2020
@wez
Copy link
Owner

wez commented Jul 17, 2020

I updated to a build of openconsole from microsoft/terminal@efb1fdd
I can confirm that with this build that the bold rendering is just plain bold and that eg: my local bold rendering override works (it is tomato colored).

Now, when it comes to the color schemes that @hgkamath was comparing with screenshots, there's a bit more to unpick and I haven't time to dig in more fully right now, but there are some factors:

  • wezterm has a default foreground setting in the color schemes that applies when the foreground is set to Default
  • alternatively, the color can be set to an explicit palette index
  • or an RGB value

When bold is enabled and the color is set to one of the first 8 palette indices, the index is shifted into the set of brights (this brightening behavior is contentious for some).

In the screenshots of the color schemes I believe that we're seeing the brightened bold, but in wezterm we're not triggering the brightening because the foreground color is default rather than the default foreground index, so we just get the foreground color from the theme when text is bold.

I'll look into this a bit more later!

@hgkamath
Copy link
Author

hgkamath commented Jul 17, 2020

I downloaded testbuild from artifact of Run 172934664

Wezterm -V
wezterm 20200620-160318-e00b076c-30-gf64b268e

Confirming that said upstream fixes from windows terminal seem to be there.
Confirming that man-pages with less pager in remote-linux is more readable with the black (or tomato) color. The bold text is visibly bolder than the regular foreground text and stands out as such.
Also, confirming that bold-intensity font-rule for tomato color is applied to [1m, more on that below.

Light-background terminals are tricky as terminals were originally dark-background, and bold face was just brighter/thicker. When terms that now have light-background, the meaning behind what is bold and brighter inverts, requiring bold face to be darker/thicker. Yes, your statement "When bold is enabled and the color is set to one of the first 8 palette indices, the index is shifted into the set of brights (this brightening behavior is contentious for some)." is correct. Perhaps the user preference as to what to do is configurable using a config option.

The bundled openconsole.exe is still so-n-so with fix for [40m background, but the [1m, [37m dark-light inversion exists. Openconsole.exe has its own set of different bugs, and the man-page/less-pager prints text in reverse-video on the light background, which is an aberration but renders text readable.

These below screen captures of wezterm are ok and perhaps how they are supposed to be.

  • man page/less
    4_4  root@fedoraminimalm13__home_gana_tmp_weztest 7_17_2020 7_57_38 PM

  • without font rule:
    C__Program Files_PowerShell_7_pwsh exe 7_17_2020 3_50_51 PM

  • with font rules: bold->courier new, tomato
    C__Program Files_PowerShell_7_pwsh exe 7_17_2020 3_48_50 PM

With regard to the font rule, the tomato color is applied only to the [1m, the courier-new seems to be applied to all bold/bright-colors. I'm not complaining, seems useful that way. Perhaps it makes sense that text with specified color attributes should not have override-able colors. Perhaps the font rule for the intensity selector, may require additional match specifiers for the font and other parameters such as color to be applied to one, some or all bold/bright characters. I don't have an opinion on this.

That was fast. I didn't expect a fix this soon.

@wez
Copy link
Owner

wez commented Jul 18, 2020

I added a config option to allow disabling the brightening behavior. It is explained here in the docs: https://wezfurlong.org/wezterm/config/fonts.html

I've opted not to try to brighten the scheme foreground color at this time, as the foreground color is often not one of the ansi palette colors in a lot of the color schemes and it's not super clear how that should be brightened. It could be that we do some color manipulation and brighten by some configurable percentage. I'd prefer to punt on this until someone with opinions and good rationale behind shares their thinking on this :)

I'm going to tag a release this weekend, so I'm closing this issue!

@wez wez closed this as completed Jul 18, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2023

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working Windows Issue applies to Microsoft Windows
Projects
None yet
Development

No branches or pull requests

3 participants