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

Should all Colors class define the same color? #47

Open
gabalafou opened this issue Mar 27, 2024 · 6 comments
Open

Should all Colors class define the same color? #47

gabalafou opened this issue Mar 27, 2024 · 6 comments
Assignees
Labels
type: enhancement 💅🏼 New feature or request

Comments

@gabalafou
Copy link
Contributor

          I don't see a better way, I though maybe all Colors class define the same color, but no:
  16     black
  16     blue
  16     comment
   2     cyan
  16     green
  16     orange
  16     purple
  13     red
  10     yellow

Maybe this is something we should think about and have all define the same set of keys ?

Originally posted by @Carreau in #42 (comment)

@Carreau
Copy link
Collaborator

Carreau commented Mar 27, 2024

for what it is worth the 8 colors defined by ansi for terminals are, Black Red Green Yellow Blue Magenta Cyan White, From what I can see the main difference is here we have commentand orange in extra;purple(instead of magenta); andwhite` missing.

@Carreau
Copy link
Collaborator

Carreau commented Mar 27, 2024

(also I should check but I don't think the Color class has any particular needs, so we might as well also make it am Enum, and will have proper way of listing fields and type checking.

@Carreau
Copy link
Collaborator

Carreau commented Mar 27, 2024

See #35 as well.

@trallard
Copy link
Member

trallard commented Apr 9, 2024

Do all the themes have something like 8 ANSI colours + comment? @gabalafou @Carreau since you both looked at this recently.

If so that would give us a relatively uniform palette for the themes.
If we decide to add different support per #32, then that would make ANSI + comment + extras (purple/orange).

If not -- as in the themes have really different colours and no apparent base palette - then chaos reigns.

(also I should check but I don't think the Color class has any particular needs, so we might as well also make it am Enum, and will have proper way of listing fields and type checking.

AFAIK there are no particular needs/requirements that must be met here so we cal always go down the enum route.

@trallard trallard added the type: enhancement 💅🏼 New feature or request label Apr 9, 2024
@Carreau
Copy link
Collaborator

Carreau commented Apr 9, 2024

Well roughly yes the themes use about the same colors. Things is the Color class itself is unnecessary for the theme, it is just used as a convenience to refer to a limited set of colors.

I think it would make sens to have some consistency. I'll try to look at which themes differs from others.

@Carreau Carreau self-assigned this Apr 11, 2024
@Carreau
Copy link
Collaborator

Carreau commented Apr 12, 2024

Colors used "only" in a few themes:

  • magenta
    a11y_high_contrast_light
  • cyan
    greative, pitaya_smoothie

Colors "missing" in a few themes:

  • red
    greative, blinds_light, blinds_dark

"Yellow" is present in 9 themes out of 16, so meh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement 💅🏼 New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants