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

Handle terminal color schemes #17

Closed
wagslane opened this issue May 12, 2024 · 4 comments
Closed

Handle terminal color schemes #17

wagslane opened this issue May 12, 2024 · 4 comments
Labels
enhancement New feature or request priority-low Pick this up when you're bored or just want to

Comments

@wagslane
Copy link
Member

wagslane commented May 12, 2024

Feedback:

I have my windows terminal color scheme set to Solarized Dark. Several lines of the output from "bootdev run 65e6780d-fdde-447a-9898-b30b73793a3a" are unreadable.

Should we maybe add a --no-color option? Idk the "normal" way to handle this, maybe @cgsdev0 has some ideas

@wagslane wagslane added enhancement New feature or request priority-low Pick this up when you're bored or just want to labels May 12, 2024
@Waldeedle
Copy link

var gray = lipgloss.NewStyle().Foreground(lipgloss.Color("8"))
This var is specifying a color based on the ANSI value, whereas in another file you are using the hex value. The problem seems to be a known one in the case of the windows terminal as can be seen here: microsoft/terminal#14684
I do see what the user means by it basically being unreadable, as shown in my screenshot below:
image

there are two thoughts that come to mind for potential fixes:

  1. have a box surrounding the content output by the cli, think of a div with a background. Then also use hex values for all coloring to essentially have 1 uniform color palette for all users. It's not as user friendly but ensures consistency and would be less to manage on bootdev's side.
    image

  2. Use a different ANSI value and potentially try to accommodate as many user configurations as possible. Less ideal for maintenance in my mind, but its an option. You could try to do some calculations with the calculated color and see if its too close visually to the background color then have a fallback ANSI value.

I personally feel like 1 would be the better pick here when thinking about UX. Less of "it works on my machine" 😆

@cgsdev0
Copy link
Contributor

cgsdev0 commented May 15, 2024

image

facepalm nice find

@Waldeedle colors in the terminal are a tricky beast. to your point of number 2, we can't actually do calculations on the colors, because we can't know what the colors are. (There are some control codes to query color schemes, but they are non-standard and unimplemented in a lot of terminals. plus, they don't always give you a definitive answer anyway)

I think the best option would be to just add some configurability here - then people can tweak the colors if needed.

for now, we do respect the NO_COLOR=true environment variable

@Waldeedle
Copy link

Ah right, I totally overlooked that since this is a CLI application, it can allow users to use a config file to override colors. That would also allow users additional customization. Customizability FTW

As for your comment about point 2, definitely was thinking the solution would be very abstract (was hoping lipgloss might surface hex codes or rgb values even from ANSI values).

👏👏👏

@cgsdev0
Copy link
Contributor

cgsdev0 commented May 15, 2024

this will be resolved in the next release

i have added a bootdev configure command that allows customizing the colors we use

related commit: 28fa134

@cgsdev0 cgsdev0 closed this as completed May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority-low Pick this up when you're bored or just want to
Projects
None yet
Development

No branches or pull requests

3 participants