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

Change font colour based on terminal background #1068

Open
dinanz opened this issue Apr 3, 2023 · 9 comments
Open

Change font colour based on terminal background #1068

dinanz opened this issue Apr 3, 2023 · 9 comments

Comments

@dinanz
Copy link
Contributor

dinanz commented Apr 3, 2023

Suggestion to add feature to change text colour based on terminal background colour or system theme, if possible. Currently when the background is black, the text that are in dark blue and red are almost unreadable. I'd like to suggest and try working on this: if the terminal background is detected to be black or system is set to dark theme, changee the colours to be much brighter so they are easier to read.
image

@pnhofmann pnhofmann pinned this issue Apr 5, 2023
@pnhofmann
Copy link
Collaborator

pnhofmann commented Apr 5, 2023

Yep, that would be nice.

I actually don't really know if detecting background color of Terminals works corss-platform (and in all kind of terminal-emulators?).
If not, manual configuration of "light" and "dark" mode would also work fine for me.

Pinned this issue, since I agree we actually need this feature. I'm happy for any input ;).

Somehow related: #485

@dinanz
Copy link
Contributor Author

dinanz commented Apr 7, 2023

I looked at #485 and I can see that it's a little complex to configure personalisations especially with storage... I can definitely look into detecting OS and then different functions for detecting theme based on OS if that's possible at all, and if not I suppose I can try (at least to begin with) perhaps a temporary configuration for the text colour.
Thanks! :)

@dinanz
Copy link
Contributor Author

dinanz commented Apr 17, 2023

@pnhofmann Hi, I'm wondering if it's acceptable if the text color itself can just be changed to a more middle-ground color that works on both white and black backgrounds? After some testing cyan seems to be a good replacement for the dark blue, and magenta to replace red.
In the below cyan and magenta are pretty readable on both light/dark theme and much more readable than blue/red on a black background.
image
image
Let me know if this is okay, or the color still needs tweaking, or detection of theme/manual config is still preferred.
Thanks! :)

@pnhofmann
Copy link
Collaborator

That's actually a good compromise

@JustinMLu
Copy link

I'm gonna try to make a plugin that allows Jarvis to accept text commands to change text color (or just automatically does it), if that's okay with you guys? Looking to be done by either tomorrow or the day after

@dinanz
Copy link
Contributor Author

dinanz commented Apr 18, 2023

@JustinMLu Hi, I'm in the middle of changing the default text colors so I'm going to finish this, but it's only minor fixes in the main file and in pluginManager. But afterwards I think if you can work on a manual config that would be great, and as a separate plugin I don't think it's going to conflict with my changes either. Thanks!

@JustinMLu
Copy link

JustinMLu commented Apr 18, 2023

No worries! I'm actually nearly finished with my plugin that lets you change colors manually & I don't wanna step on your toes.

I don't think our changes will conflict unless you're changing the default text colors by hard-coding the colorama constants (my default text colors are pulled from a JSON file so that user changes can persist). Even if they do, that's a trivial fix.

My plugin also adds support for preset themes (and resetting to default), so I could make your new theme the 'default theme' afterwards if you want!
example-themes

@JustinMLu
Copy link

JustinMLu commented Apr 18, 2023

Update: I might make a more formal issue later, but I've noticed some issues with the "first_reaction" boolean (in Jarvis.py and CmdInterpreter.py).

Not only does it stay True after first_reaction_text is outputted by Jarvis, it gets set to False after the second prompt is queued. This leads to an inability to update the theme for following commands, but is simply solved by bypassing the if-statement within the postcmd() function in Jarvis.py entirely. Could I get a second opinion on this?

@speedhs
Copy link

speedhs commented Jul 27, 2023

@JustinMLu Have you already fixed this? I was thinking if not maybe I can take it up

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

No branches or pull requests

4 participants