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

Add #rrggbb string parsing #54

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

kilpkonn
Copy link

I find it more convenient to keep colors in #rrggbb format so added support for that

Thanks for the grate lib btw!

Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am open to doing this, but I want to make sure this has no breaking changes. I don't think there are any, but could you please think through that and provide an argument as to why?

Also, this will need:

@kilpkonn
Copy link
Author

I added some tests and limited the string length to be 7 so #0011FF1 would fail rather than act like #0011FF.

My reasoning for why it's not braking change:

  1. It only has effect where the command previously "failed" eg. is supposed to be ANSI code but actually is not.
  2. It requires string to have fixed length of 7 and start with # so I does not conflict with other formats.
  3. It doesn't panic if something is wrong but just return Err as it should.

For documentation, did you mean a line there or would you like some short example usage?

@BurntSushi
Copy link
Owner

For documentation, did you mean a line there or would you like some short example usage?

It should document the format, at minimum. e.g., "A # followed by 6 hexadecimal digits of any case, with each octet representing red, green and blue, respectively."

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.

None yet

2 participants