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.

Sign up for GitHub

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

Jump to bottom

Add parsing color_t #2015

Merged
merged 20 commits into from
Mar 24, 2025
Merged

Add parsing color_t #2015

merged 20 commits into from
Mar 24, 2025

Conversation

VittorioSanchez
Copy link
Contributor

@VittorioSanchez VittorioSanchez commented Feb 24, 2025 •
edited by mwestphal
Loading

Added parsing for:

  • #RRGGBB
  • rgb(R, G, B)
  • hsl(H, S%, L%)
  • hsv(H, S%, V%)
  • hwb(H, W%, B%)
  • cmyk(C%, M%, Y%, K%)
  • CSS color names

These are case insensitive and whitespace insensitive. % symbol is also optional.
Limits are checked and throws error if out of range.
Updated doc and added tests

Fix: #2003

@VittorioSanchez VittorioSanchez marked this pull request as ready for review February 27, 2025 14:44
@VittorioSanchez VittorioSanchez changed the title [DRAFT] Add parsing color_t Add parsing color_t Feb 27, 2025
@codecov Codecov
Copy link

codecov bot commented Feb 27, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.78%. Comparing base (9dbb696) to head (9161557).
Report is 58 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2015      +/-   ##
==========================================
- Coverage   95.79%   95.78%   -0.02%     
==========================================
  Files         128      125       -3     
  Lines       10396    10652     +256     
==========================================
+ Hits         9959    10203     +244     
- Misses        437      449      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mwestphal mwestphal mentioned this pull request Mar 1, 2025
Closed
@VittorioSanchez VittorioSanchez requested a review from snoyer March 4, 2025 13:23
snoyer
snoyer reviewed Mar 4, 2025
View reviewed changes
Copy link
Contributor

@snoyer snoyer left a comment

Choose a reason for hiding this comment

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

some nitpicks as per tradition but look good :)

mwestphal
mwestphal requested changes Mar 8, 2025
View reviewed changes
Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

some questions and remarks

@Meakk
Copy link
Member

Meakk commented Mar 10, 2025

I made a quick review and it looks super great! Thank you @VittorioSanchez! 👍
I'll let @mwestphal make the final approval when the small remarks are fixed.

@mwestphal mwestphal mentioned this pull request Mar 15, 2025
Merged
@mwestphal
Copy link
Member

Hi @VittorioSanchez

Do you need help moving forward ? :)

mwestphal
mwestphal reviewed Mar 24, 2025
View reviewed changes
mwestphal
mwestphal requested changes Mar 24, 2025
View reviewed changes
Copy link
Member

@mwestphal mwestphal left a comment

Choose a reason for hiding this comment

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

small changes needed

@mwestphal mwestphal requested review from Meakk and snoyer and removed request for snoyer March 24, 2025 10:05
Meakk
Meakk approved these changes Mar 24, 2025
View reviewed changes
Copy link
Member

@Meakk Meakk left a comment

Choose a reason for hiding this comment

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

LGTM

@mwestphal mwestphal merged commit c236c82 into f3d-app:master Mar 24, 2025
53 checks passed
@mwestphal
Copy link
Member

Thanks for your contribution @VittorioSanchez !

@VittorioSanchez VittorioSanchez deleted the parsing_color_t branch March 24, 2025 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwestphal mwestphal mwestphal approved these changes

@Meakk Meakk Meakk approved these changes

@snoyer snoyer Awaiting requested review from snoyer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add proper parsing for color_t
4 participants
@VittorioSanchez @snoyer @mwestphal @Meakk