Allow brightening bold + default fg, and bolding bright colors

John-Gee commented 1 year ago

What Operating System(s) are you seeing this problem on?

Linux X11

Which Wayland compositor or X11 Window manager(s) are you using?

Kwin

WezTerm version

20230326-111934-3666303c

Did you try the latest nightly build to see if the issue is better (or worse!) than your current version?

No, and I'll explain why below

Describe the bug

(Following your reply in #3414 )

In my usual terminal Konsole

$ echo -e "\033[1mTEXT...\033[0m"

is bold, and bright aka in this case white.

In wezterm it's bold and not bright so grey.

Now it's not that I care about this exact command, but that some programs that I run seem to expect that behavior. For instance running yay on Konsole is a lot easier to read than on wezterm because Konsole does auto brightening the default color when it's bold. I'm not saying wezterm is wrong but it's nicer with this extra behavior (same as bold->bright). This is why I originally made a feature request, as I don't believe there's any actual issue per se (unless the issue is me not knowing how to configure this?)

Ex

vs

It's just easier to focus on the white and sort of ignore the grey. (Now it might be good to show when the terminal is overriding the color, so we can still think of opening bug requests for the programs that don't have the best code.)

Thank you for the quick reply!

To Reproduce

No response

Configuration

no config

Expected Behavior

No response

Logs

No response

Anything else?

Well, maybe we don't need the bright version of background as I asked in the FR, I don't really know what would use it.

wez commented 1 year ago

Ah interesting! I never noticed this before.

There's a slightly tricky nuance to this, which is that the cell in this scenario has "default fg color" as its color.

There's no explicit relation between "default fg color" and any of the ANSI color indices, and I'm fairly certain that there are at least a handful of color schemes that rely on this.

What that means is that it's conceptually problematic to assume that "default fg color" is equivalent to, say, ANSI color index 1, because that could be incorrect if the color scheme is light or dark.

Strategies for resolving this are:

Could search for a matching color in the 0-7 range and use that. But what if there is no match? Should we use DeltaE to find similar colors? What is the threshold? What if there is still no match?
Could directly compute a brighter color using something like color:lighten(). The factor could be configurable and have a reasonable default. But we may end up with the brighter color not matching anything in the color palette, and surely some eagle-eyed user will file a bug.
Maybe a hybrid approach is to use the first technique to find an exact match, but fallback to lightening the color?

John-Gee commented 1 year ago

Well that's why I suggested the Konsole behavior of having the ability to set 2 colors for foreground, say foreground and foreground_bright, that way it's easy to go from standard to bright (or vice versa if there was a need). And as a sane default we could start with if foreground_bright unset then foreground_bright = foreground, that doesn't break previous behavior.

John-Gee commented 1 year ago

Maybe the issue's title should be more: "Unable to exactly replicate Konsole's colors"? Because I think to do so we may also need bright->bold (unless you already have that and I missed it).

Qyriad commented 1 year ago

iTerm2's approach simply uses an optional separate configuration value to set the bold-foreground color — perhaps a simple config.colors.foreground_bold option or similar would acceptable?

wez commented 1 year ago

Note that you can use font_rules to override the color of bold (or other styles of) text.

Qyriad commented 1 year ago

Oh! Good point, apologies!

John-Gee commented 1 year ago

Alright, thanks to your link I was able to do it (albeit it took me a lot longer than I expected, but maybe it's just me not understanding the docs...)

-- Function to convert from ansi to bright
local function convertAnsiToBright(v)
  for index, value in ipairs(config.colors.ansi) do
    if value == v then
      return config.colors.brights[index]
    end
  end
  return v
end

config.font_rules = {
  {
    intensity = 'Bold',
    font = wezterm.font (
        'FiraCode Nerd Font',
        {
          weight = 'Bold',
          foreground = convertAnsiToBright(config.colors.foreground),
        }
    ),
  },
}

I do wish I wouldn't have to specify the font though, and just stick to the current one. Maybe something like this is already possible?

config.font_rules = {
  {
    intensity = 'Bold',
    font = wezterm.font (
        config.font.family,
        {
          weight = 'Bold',
          foreground = convertAnsiToBright(config.colors.foreground),
        }
    ),
  },
}

Thank you!

bskydive commented 4 months ago

@wez , can you, please, add some new fields into config.color_scheme.colors data structure?

To fix this behavior, you can allow to assign bold/bright color value, as it works in Konsole or Tilix.

Example of using the bold color(the #d445d2 value) in iTerm color scheme converter^ https://github.com/mbadolato/iTerm2-Color-Schemes/blob/master/dynamic-colors/Sakura.sh https://github.com/mbadolato/iTerm2-Color-Schemes/blob/master/tools/templates/dynamic-colors.sh

As you can see, it's not the ansii bright variant, it's a new field and value.

Also, in Konsole it named as "[ForegroundIntense]" https://github.com/mbadolato/iTerm2-Color-Schemes/blob/master/tools/templates/konsole.colorscheme

I've enhanced your example:

local COLOR_SCHEME_NAME = 'Sakura'
-- local COLOR_BOLD = nil
local COLOR_BOLD = '#d445d2'

local SCHEME_COLORS = wezterm.color.get_builtin_schemes()[COLOR_SCHEME_NAME]
-- local my_default = wezterm.color.get_default_colors()
config.color_scheme = COLOR_SCHEME_NAME
config.font = wezterm.font 'JetBrains Mono'

local function convertAnsiToBright(color_value)

    if COLOR_BOLD ~= nil then
        return COLOR_BOLD
    end

    for index, value in ipairs(SCHEME_COLORS.ansi) do
        if value == color_value then
            return config.colors.brights[index]
        end
    end

    return color_value
end

config.font_rules = {
    {
    intensity = 'Bold',
    font = wezterm.font (
        'JetBrains Mono',
        {
            weight = 'Bold',
            foreground = convertAnsiToBright(SCHEME_COLORS.foreground),
        }
    ),
    },
}

If you will add a new field, we can do it like that:

config.color_schemes = {
['Some scheme name'] = {
    bold = '#d445d2',
}

Curently, wezterm config parser throws an error in that case.

bskydive commented 4 months ago

Found it. https://wezfurlong.org/wezterm/config/lua/config/bold_brightens_ansi_colors.html

wez / wezterm