Support conversion to a StyledStrings.SimpleColor

[tecosaur]

This enables easy interoperability with the Face constructor of StyledStrings.

With this, it's easy to do fun things like:

[timholy]

Thanks!

[kimikage]

I have no intention to oppose this PR, but I think the color parsing this kind of conversion should be in Colors.jl. I saw a possible discussion regarding this on Colors.jl. https://github.com/JuliaGraphics/Colors.jl/issues/514 https://github.com/JuliaGraphics/Colors.jl/issues/473

[tecosaur]

There's no colour parsing going on though?

[kimikage]

I used "parsing" in an abstract sense. Sorry for the confusion. This is a kind of "notation" conversion, not a color space or color model conversion. Considering that Colors.jl is responsible for conversion from more basic string types, I personally think that conversion to types that do not come under Colorant should be outside of ColorTypes.jl.

[johnnychen94]

Keeping ColorTypes as small as possible is a good comment; I've seen people thinking 0.2s loading time is too large (#270)

From the engineering and maintenance-ship part, if some codes already sit here and there's no "obvious" impact on the user experiences, there's no need to take the risk for the re-factoring unless someone is willing to respond to whatever happens during the transferring. Since this PR has only been merged and has not yet been released, I'm okay with either ColorTypes or Colors if anyone wants to make the change.

The real problem with this PR is that StyledStrings is a stdlib package in Julia 1.11+, and this makes it impossible for ColorTypes to register a new version if we want to maintain backward compatibility (https://github.com/JuliaGraphics/ColorTypes.jl/commit/5eea76d589f4f3c3012e3d16199a9bb1f1cabcbb#comments).

@tecosaur I'm not sure what the best solution to this is. Maybe StyledStrings should be registered in the General as well?

[tecosaur]

@kimikage the way I see it, support for using Colorants in StyledStrings should either be in the package that defines Colorant (this one, ColorTypes), or StyledStrings.

Doing it in StyledStrings isn't really an option, since it's a stdlib, and since the required code is pretty minimal I'm not sure there's enough of a "cost" to having it here to justify worrying about this any more than the basic thought process I've just mentioned.

---

@johnnychen94 Keeping ColorTypes small and focused is indeed good, but given the simplicity of this specialisation, I'd be amazed to hear if this has any appreciable effect on codeimage size, or load time. Seperately, thinking of a "where does this functionality best sit", other than the usual "not doing type piracy is good", Colors.jl seems a lot more complex than this functionality is.

The real problem with this PR is that StyledStrings is a stdlib package in Julia 1.11+, and this makes it impossible for ColorTypes to register a new version if we want to maintain backward compatibility.

This shouldn't be a problem for much longer. StyledStrings has a julia1-compat branch that's going to be registered in General as soon as the version that's going to come with 1.11 and the implementation of the Base.Abstract{String,Char,IOBuffer} types is settled. See https://github.com/JuliaLang/StyledStrings.jl/issues/5.

I think we're down to a single blocker in that regard, and that's waiting for SubString{AnnotatedString} equality to be worked out: https://github.com/JuliaLang/julia/issues/53042.

[kimikage]

I still think Colors.jl is a more appropriate place than ColorTypes.jl (of course than StyledStrings.jl too), but is there another option, e.g. via a more unambiguous type such as Tuple? In that case, it becomes difficult to use color models other than RGB. Note, however, that the actual implementation of the color conversions is in Colors.jl.

[tecosaur]

is there another option, e.g. via a more unambiguous type such as Tuple?

I can't say I actually know what you mean by this.

[kimikage]

Here is the code for the concept (which is by no means practical)

# ColorTypes.jl side
using ColorTypes
Base.Tuple(c::Colorant{T, 3}) where {T} = (comp1(c), comp2(c), comp3(c))::NTuple{3, T}

(cf. PR #260)

# StyledStrings.jl side
using StyledStrings
Base.convert(::Type{StyledStrings.SimpleColor}, c) = StyledStrings.SimpleColor((round.(Int, Tuple(c) .* 255))...)
Base.convert(::Type{StyledStrings.SimpleColor}, c::StyledStrings.SimpleColor) = c

julia> col = RGB(1, 0.5, 0)
RGB{Float64}(1.0,0.5,0.0)

julia> styled"{(fg=$col):colored}"
"colored"

[tecosaur]

Hmmm, this seems built on too many assumptions for me to feel comfortable with it at a glance.

[kimikage]

I understand your concern. However, are you making any assumptions?

julia> using ColorTypes, StyledStrings

julia> hsv = HSV(300, 0.4, 0.5)
HSV{Float64}(300.0,0.4,0.5)

julia> styled"{(fg=$hsv):Color conversion is implemented in Colors.jl}"
ERROR: No conversion of HSV{Float64}(300.0,0.4,0.5) to RGB24 has been defined

If this PR is for package developers other than stdlib, they should know what type of colors they are dealing with. If colors are given from an external source, such as an end user or another package, the package should require Colors.jl to avoid such errors.

[kimikage]

Also, if in the future julia users request support for the X11 or CSS color specifications, should we add another extension to Colors.jl as well? Of course, having extensions is not so bad in itself.

[kimikage]

Since this PR has already been merged, this discussion is somewhat ”closed" in terms of discoverability in GitHub. If the discussion is to continue, it should be on a new issue or Discourse.

[tecosaur]

If the discussion is to continue, it should be on a new issue or Discourse.

Frankly, I'm not sure there's much value in continuing it :sweat_smile:. I still think https://github.com/JuliaGraphics/ColorTypes.jl/pull/293#issuecomment-2041072731 sums up the situation well.

I understand your concern. However, are you making any assumptions?

To me, this is behaving exactly as expected, with ColorTypes.jl and StyledString.jl loaded, we see that a non-RGB colour can be used with StyledStrings if conversion methods are implemented. Package authors would need Colors.jl if they wanted to convert colour spaces before, and the situation is unchanged.

Also, if in the future julia users request support for the X11 or CSS color specifications, should we add another extension to Colors.jl as well? Of course, having extensions is not so bad in itself.

I don't really follow, are you thinking of a future where somehow X11/CSS named colour specifications get integrated with the Julia stdlib via some dedicated type? This seems like rather extreme speculation.

[kimikage]

https://github.com/JuliaLang/StyledStrings.jl/issues/33 is not the issue you filed? Of course, we can still extend StyledStringsExt to support alpha!

That's all! Thank you for listening to my gripe.

(BTW, wouldn't it be better if the extension name was globally unique? Edit: to distinguish from the future ColorsStyledStringsExt? 😝)

[tecosaur]

https://github.com/JuliaLang/StyledStrings.jl/issues/33 is not the issue you filed?

Yep, it just (1) wouldn't need any changes to StyledStringsExt (SimpleColor is public API) (2) likely won't involve any changes to SimpleColor anyway (going with the method of adding an alpha channel to Face).

That's all! Thank you for listening to my gripe.

I don't mind a bit of griping :slightly_smiling_face:

BTW, wouldn't it be better if the extension name was globally unique?

I don't see why...

[kimikage]

xref: https://github.com/JuliaGraphics/ColorTypes.jl/issues/299#issuecomment-2077235975

This PR on its own causes problems with "Project.toml", so I labeled it breaking. (cf. https://github.com/JuliaGraphics/ColorTypes.jl/pull/302)

[tecosaur]

I don't think this is breaking?

[kimikage]

If you have any objections, please comment at #299.