adamharrison / lite-xl-terminal

A terminal plugin for lite-xl.
MIT License
41 stars 6 forks source link

Adapt terminal bg/fg colors to the editor's style #30

Closed franko closed 9 months ago

franko commented 10 months ago

Without changing the terminal colors adjust the visual appearence of the terminal to blend with the editor's style.

Guldoman commented 10 months ago

This gets problematic even with our default colors: Default, OK: Schermata del 2024-01-27 22-20-49 Fall, OK: Schermata del 2024-01-27 22-20-53 Summer, we lose the white text on default bg, and default text on black bg: Schermata del 2024-01-27 22-20-56 Textadept, same as summer, but a bit less: Schermata del 2024-01-27 22-21-00

The default colors should probably closely match Lite XL's default theme, and let the user and/or the color scheme customize the terminal main colors.

Edit: for reference, the script used is the one at the bottom of this page https://tldp.org/HOWTO/Bash-Prompt-HOWTO/x329.html.

franko commented 10 months ago

Hi @Guldoman, thank you for looking at that.

I agree, some colors can get "lost" due to low contrast in some cases but I am not sure this is an hard requirement. After all the black color is invisible when using the venerable xterm and I think that's the same with the original colors Adam's hard coded.

In an ideal world each color's theme should have a complete palette of colors that plays well with the theme's colors but that's a little bit too much to ask for practical reasons.

TorchedSammy commented 10 months ago

Summer, we lose the white text on default bg, and default text on black bg: Schermata del 2024-01-27 22-20-56 Textadept, same as summer, but a bit less: Schermata del 2024-01-27 22-21-00

The default colors should probably closely match Lite XL's default theme, and let the user and/or the color scheme customize the terminal main colors.

Edit: for reference, the script used is the one at the bottom of this page tldp.org/HOWTO/Bash-Prompt-HOWTO/x329.html.

You can't put white text on a white background and expect it to be readable.

Guldoman commented 10 months ago

I agree, some colors can get "lost" due to low contrast in some cases but I am not sure this is an hard requirement.

This will actively break people's prompts/autosuggest/whatever, which we should avoid doing.

The current color scheme results in: image which only has problems with black text on default background, for which just changing one of them to a different color would fix things (like #32).

You can't put white text on a white background and expect it to be readable.

Sure, but this is white text on default background.

Example of thing that can go wrong (taken by a user from GNOME Builder, which does what this PR suggests): image

TorchedSammy commented 10 months ago

Sure, but this is white text on default background.

The default background is also white. It'll always be a problem with light themes (or any theme), so I don't see any real issue.

Example of thing that can go wrong (taken by a user from GNOME Builder, which does what this PR suggests):

This is a result of bad coloring on the user side (user error) rather than what would happen if this was merged..?

I don't even have real brights on my terminal and this has a better result:

This makes me think that maybe colors 0 and 7 should be changed as well, so 0 can be background2 or something and then 7 can be the dim text color

kra-mo commented 10 months ago

This is a result of bad coloring on the user side (user error) rather than what would happen if this was merged..?

It is not. It's the same issue. Builder only recolors the background/foreground colors and not the palette and so stuff like this can happen. This is specifically with one of the default themes there.

Their palette colors are more opinionated so this would likely happen more often but it would also be a case with some more saturated themes in lite-xl.

adamharrison commented 10 months ago

So, I've asked around at work, and to a few programmers I know, and the consensus seems at home here seems to be that the terminal should follow the editor's colour scheme, even if there are issues with saturation.

One thing we could do is quickly loop through background1, 2, 3, as well as text normal, accent, dim, keyword1, keyword2 in that order, and pick the pair that has the largest difference, and only fall back to black/white if there're no pairs > than a certain threshold? That way we kinda sidestep this issue.

Never mind; the issue is with the alternate colors. Maybe we should grab alternate colors from the list of general property values from a style so long as they're distinct from the background sufficiently, and then funnel them into the first 16 specified (the customizable ones)?

Guldoman commented 10 months ago

xterm.js (vscode) checks the contrast ratio between fg and bg (config variable MinimumContrastRatio) and applies this function https://github.com/xtermjs/xterm.js/blob/99df13b085aecb051f1373c5b7f8e819c4f41442/src/common/Color.ts#L285.

To be honest I don't think dealing with color theory is a good idea, but a "weirdly" colored terminal is better than an illegible one.

Can we also find a way to let color schemes decide the colors in a relatively generic way?

TorchedSammy commented 10 months ago

Can we also find a way to let color schemes decide the colors in a relatively generic way?

Named color variables (red, blue, etc.)

adamharrison commented 9 months ago

Named color variables (red, blue, etc.)

I'm actually OK with this. We could just set them as the base styles for the 16 primary terminal colors (which should give enough variety; or maybe just the first 8, since the last 8 are just brighter versions), and themes can tweak these as needed.

Anyone have any objections to this?

adamharrison commented 9 months ago

OK, so @Guldoman made the point that this, combined with the specified contrast threshold function should be good enough. I'm OK with that. Any objections?

adamharrison commented 8 months ago

OK! This has been adopted as 52e76593500a1664ebaf709a054732330f656aa0!