bootdotdev / bootdev

A CLI used to complete coding challenges and lessons on Boot.dev
https://www.boot.dev
MIT License
374 stars 15 forks source link

Handle terminal color schemes #17

Closed wagslane closed 4 months ago

wagslane commented 4 months ago

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

Waldeedle commented 4 months ago

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: https://github.com/microsoft/terminal/issues/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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

this will be resolved in the next release

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

related commit: https://github.com/bootdotdev/bootdev/commit/28fa134e4f4b3ae28e9a4387ab888010450a4060