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 Basic CSS Color Parsing #24

Merged
merged 5 commits into from
Apr 2, 2021
Merged

Conversation

cedrickcooke
Copy link
Contributor

For now, only parses color keywords and hex notation (#rgb, #rgba, #rrggbb, #rrggbbaa). In the future we should include rgb() function call syntax. As such, I'm considering this only a partial implementation of #22.

Also wrote a few bonus tests since I was looking at the coverage report and they seemed fast.

@cedrickcooke cedrickcooke requested review from a team, twyatt and sdonn3 April 2, 2021 19:14
@codecov
Copy link

codecov bot commented Apr 2, 2021

Codecov Report

Merging #24 (b140fbc) into main (2cd38aa) will increase coverage by 24.20%.
The diff coverage is 97.76%.

Impacted file tree graph

@@              Coverage Diff              @@
##               main      #24       +/-   ##
=============================================
+ Coverage     42.70%   66.91%   +24.20%     
- Complexity       37       54       +17     
=============================================
  Files            16       27       +11     
  Lines           384      807      +423     
  Branches         52       66       +14     
=============================================
+ Hits            164      540      +376     
- Misses          197      238       +41     
- Partials         23       29        +6     
Impacted Files Coverage Δ Complexity Δ
color/src/commonMain/kotlin/Constants.kt 100.00% <ø> (ø) 0.00 <0.00> (?)
color/src/commonMain/kotlin/Color.kt 93.54% <87.50%> (ø) 14.00 <2.00> (?)
color/src/commonMain/kotlin/ToColor.kt 89.65% <89.65%> (ø) 0.00 <0.00> (?)
color/src/commonMain/kotlin/WebColors.kt 100.00% <100.00%> (ø) 0.00 <0.00> (?)
color/src/commonMain/kotlin/Operations.kt 100.00% <0.00%> (ø) 0.00% <0.00%> (?%)
kanvas/src/commonMain/kotlin/Kanvas.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
kanvas/src/commonMain/kotlin/Font.kt 80.00% <0.00%> (ø) 3.00% <0.00%> (?%)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cd38aa...b140fbc. Read the comment docs.

Copy link
Member

@twyatt twyatt left a comment

Choose a reason for hiding this comment

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

This is awesome! Thanks so much for integrating this so soon! ❤️

Comment on lines +5 to +8
private val RGB = """($HEX)($HEX)($HEX)""".toRegex(RegexOption.IGNORE_CASE)
private val RGBA = """($HEX)($HEX)($HEX)($HEX)""".toRegex(RegexOption.IGNORE_CASE)
private val RRGGBB = """($HEX{2})($HEX{2})($HEX{2})""".toRegex(RegexOption.IGNORE_CASE)
private val RRGGBBAA = """($HEX{2})($HEX{2})($HEX{2})($HEX{2})""".toRegex(RegexOption.IGNORE_CASE)
Copy link
Member

@twyatt twyatt Apr 2, 2021

Choose a reason for hiding this comment

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

I suspect that regex approach is slower than processing by simple iteration.
But we can definitely postpone that optimization (and should benchmark to verify that regex is even slower).

Created #25 (super low priority though) to potentially address this later.

Copy link
Contributor

@sdonn3 sdonn3 left a comment

Choose a reason for hiding this comment

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

looking good to me

@cedrickcooke cedrickcooke merged commit 52c530a into main Apr 2, 2021
@cedrickcooke cedrickcooke deleted the cedrickc/css-color-parsing branch April 2, 2021 22:00
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.

3 participants