Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add FromStr for DynamicColor and static colors #111

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

waywardmonkeys
Copy link
Collaborator

This makes it easier to parse them from a string with more concise code and not requiring a separate import of parse_color.

@waywardmonkeys waywardmonkeys added the enhancement New feature or request label Dec 21, 2024
@waywardmonkeys waywardmonkeys added this to the 0.2.1 milestone Dec 21, 2024
@waywardmonkeys
Copy link
Collaborator Author

If we like this idea, then I'd add tests to the PR.

This makes it easier to parse them from a string with more concise
code and not requiring a separate import of `parse_color`.
@waywardmonkeys
Copy link
Collaborator Author

This would improve code in Vello's pico_svg as well as my current patches to svgtypes.

Copy link
Contributor

@nicoburns nicoburns left a comment

Choose a reason for hiding this comment

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

+1 from me on the idea of FromStr impls

@waywardmonkeys waywardmonkeys added this pull request to the merge queue Dec 27, 2024
Merged via the queue into linebender:main with commit 9df0825 Dec 27, 2024
16 checks passed
@waywardmonkeys waywardmonkeys deleted the from_str branch December 27, 2024 02:02
fn from_str(s: &str) -> Result<Self, Self::Err> {
parse_color(s)
.map(DynamicColor::to_alpha_color)
.map(AlphaColor::discard_alpha)
Copy link
Member

Choose a reason for hiding this comment

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

This one I'm less sure of. If the alpha isn't fully opaque, I'd maybe... expect this to error?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants