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

Fractional percentage notation in color parser #431

Closed
kimikage opened this issue Jun 26, 2020 · 4 comments · Fixed by #478
Closed

Fractional percentage notation in color parser #431

kimikage opened this issue Jun 26, 2020 · 4 comments · Fixed by #478
Milestone

Comments

@kimikage
Copy link
Collaborator

The color parser has only accepted integer percentages since Color.jl was separated from Compose.jl.(cf. 6738401)

julia> colorant"rgb(100%,50%,0%)"
RGB{N0f8}(1.0,0.502,0.0)

julia> colorant"rgb(100%,50%,3.14%)"
ERROR: LoadError: Unknown color: rgb(100%,50%,3.14%)

This is practical enough as long as the users write the percentages with @colorant_str. So I added the specification to the document in PR #406.

However, other programs may generate a fractional decimal percentage.For example, https://github.com/JuliaLang/julia-logo-graphics/blame/168fb6c1164e341df360ed6ced519e1e0cb7de3a/images/julia-logo-color.svg.

I do never intend to make the parser fully CSS compliant, but I'd like to remove the restriction.

@kimikage
Copy link
Collaborator Author

Julia v1.5 will be released soon, so I'd like to "cancel" the deprecation warning.

Colors.jl/src/parse.jl

Lines 143 to 147 in 53c2acb

Base.depwarn(
"""
The X11 color names with spaces are not recommended because they are not allowed in the SVG/CSS.
Use "$camel" or "$wo_spaces" instead.
""", :parse)

Instead, in Colors v0.13 I'll add a deprecation warning to:

Base.parse(::Type{C}, c::Colorant) where {C<:Colorant} = c

(cf. #406 (comment))

@timholy
Copy link
Member

timholy commented Jun 26, 2020

So by "cancel," do you mean "turn the warning into an error" or something else?

I agree with adding a depwarn to that method in Colors. But there's no conservation of depwarns, you can have as many as you want 😄.

@kimikage
Copy link
Collaborator Author

I'm thinking about purely removing the five lines of depwarn above, leaving the note in the docstring. (cf. #419)

@kimikage
Copy link
Collaborator Author

you can have as many as you want 😄.

Yes. I think FixedPointNumbers/ColorTypes/ColorVectorSpace are starting to move towards the next minor (not patch) level updates. I'm just trying to enumerate the (trivial) fixes to piggyback on the update.

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 a pull request may close this issue.

2 participants