Skip to content

Avoid using white color in REPL #372

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

Closed
wants to merge 1 commit into from
Closed

Avoid using white color in REPL #372

wants to merge 1 commit into from

Conversation

saghul
Copy link
Contributor

@saghul saghul commented Apr 11, 2024

It's not visible on white background terminals.

New colors:

Screenshot 2024-04-11 at 13 24 29 Screenshot 2024-04-11 at 13 24 07

It's not visible on white background terminals.
@saghul saghul requested a review from chqrlie April 11, 2024 20:43
@@ -58,21 +57,20 @@ import * as os from "os";
bright_blue: "\x1b[34;1m",
bright_magenta: "\x1b[35;1m",
bright_cyan: "\x1b[36;1m",
bright_white: "\x1b[37;1m",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not remove the color definitions, just change the style definitions.
At some point it should be possible to customize the styles so the colors should all be defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll revert that change.

Comment on lines +68 to 70
'keyword': 'bright_magenta',
'function': 'bright_yellow',
'type': 'bright_magenta',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keywords should not be the same color as types.
is there a way to test the terminal default color so we can use black on a white background for the default style?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if that is possible at all. I made the choice based on how VSCode seems to color stuff. No strong opinion, so I'm open to suggestions!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 ways to retrieve the default colors from the terminal:

  • using the COLORFGBG environment variable which might be maintained to reflect the terminal settings. Editors such as vim supposedly use this to adjust their color settings. This is the easiest.
  • using OSC escape sequences: sending \e]11;?\a to the terminal should trigger a terminal response for the default background color. iTerm2 sends me this: \e]11;rgb:0000/0000/0000\a for a black background. Similarly, \e]10;?\a responds with the foreground color, namely \e]10;rgb:c7f1/c7f1/c7f1\a in may case for a medium grey.
    I am going to implement the first method along with VSCode default colors for a light theme.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, shall I drop this then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, shall I drop this then?

I just filed PR #383 with dark and light selectable themes and autoselect the light scheme if the terminal background is not black, based on the COLORFGBG environment variable. So I guess you can drop this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thank you!

@saghul saghul closed this Apr 16, 2024
@saghul saghul deleted the repl-colors branch June 7, 2024 20:12
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.

2 participants