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

Include X11 colors in NAMED_COLORS #175

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

dyuri
Copy link

@dyuri dyuri commented Jan 6, 2023

  • modified the color name parsing to accept basically anything - x11 color names might have spaces and numbers
  • removing spaces from color names, so searching for something like "indian red" would succeed (and return "indianred")
  • added the colors from X11's rgb.txt that are not included in the CSS color list
  • added --color-names argument to select color name list(s) [css, x11]

Closes #138.

@sharkdp
Copy link
Owner

sharkdp commented Jan 14, 2023

Thank you very much for your contribution.

Before I take a closer look, I have one question:

added --css-colors-only flag to show only the CSS colors (in the list, or among similar colors)

Can we maybe come up with a different CLI that is a bit more future proof? Imagine we want to add other lists of named colors in the future. Maybe some of these lists are enabled by default, others not. Instead of "CSS colors only", maybe we could so something like --lists=css,x11? And --css-colors-only would turn into --lists=css?

@dyuri
Copy link
Author

dyuri commented Jan 14, 2023

You're right, I'll take a look at it.

@dyuri dyuri marked this pull request as draft January 14, 2023 15:12
@dyuri
Copy link
Author

dyuri commented Jan 14, 2023

Hmm, I just discovered that there are a few clashes between the CSS and X11 color names, I don't know how to handle them properly:
https://en.wikipedia.org/wiki/X11_color_names#Clashes_between_web_and_X11_colors_in_the_CSS_color_scheme

Currently CSS colors are not duplicated as X11 colors (but basically almost all of them are), I thought the x11 list would show css + x11 colors as well, but those few color names would not be OK.

For example: green

  • CSS green is #008000 (currently available as green@CSS and webgreen@X11
  • X11 green is #00ff00 (currently available as lime@CSS and x11green@X11

Does it worth to duplicate basically all the CSS colors in the X11 list to handle these claches?

@sharkdp
Copy link
Owner

sharkdp commented Jan 15, 2023

worth to duplicate basically all the CSS colors in the X11 list to handle these claches

Hm. That's probably the best option for now. I wouldn't mind the duplication. The question is how to resolve those conflicts if a color name is specified on the input? Maybe we should have a top-level option --color-names=list1,list2,list3 where list1 takes precedence. So if someone does pastel --color-names=x11,css color green, they will get the X11 green. And if you do pastel --color-names=css,x11 (which might be our default), you would get the CSS green.

@dyuri
Copy link
Author

dyuri commented Jan 15, 2023

I've added the conflicting colors as well, but since the colors are deduped, only their "first" name will be displayed, but I think that's OK. (For example using X11 names, #00ff00 will be reported as x11green, that's above in rgb.txt than plain green.)

@dyuri
Copy link
Author

dyuri commented Jan 15, 2023

Ok, now the name resolution of the clashing colors does not work properly, currently the parser checks only the named color list - and does not care about any configuration or arguments - and returns the first matching. So when I issue the color green command, it always parses as CSS green (#008000) disregarding the --color-names argument, and if using only the X11 names it will show webgreen - but it should show x11green.

Shall I try to change parse_color to regard the --color-names argument?

@sharkdp
Copy link
Owner

sharkdp commented Nov 2, 2023

Sorry for leaving this hanging for so long. Are you still interested in finishing this?

@dyuri
Copy link
Author

dyuri commented Nov 2, 2023

Ah, completely forgot about this.
Custom color lists would be great, but I can't see an easy way to change parse_color to support it.

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.

support all named colors from X11 rgb.txt
2 participants