JuliaGraphics / ColorTypes.jl

Basic color definitions and traits
Other
77 stars 35 forks source link

Add test and followup for `StyledStringsExt` #302

Closed kimikage closed 2 months ago

kimikage commented 2 months ago

I do not understand the need for me to create this PR, but this PR itself might be necessary for sound development.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.64%. Comparing base (142353d) to head (cdd28e1). Report is 5 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #302 +/- ## ========================================== - Coverage 84.31% 83.64% -0.68% ========================================== Files 8 9 +1 Lines 778 746 -32 ========================================== - Hits 656 624 -32 Misses 122 122 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kimikage commented 2 months ago

@tecosaur Is the following intended behavior?

julia> StyledStrings.SimpleColor(RGB(1, 0, 0))
ERROR: MethodError: Cannot `convert` an object of type
  RGB{FixedPointNumbers.N0f8} to an object of type
  Union{@NamedTuple{r::UInt8, g::UInt8, b::UInt8}, Symbol}
The function `convert` exists, but no method is defined for this combination of argument types.
tecosaur commented 2 months ago

@tecosaur Is the following intended behavior?

julia> StyledStrings.SimpleColor(RGB(1, 0, 0))
ERROR: MethodError: Cannot `convert` an object of type
  RGB{FixedPointNumbers.N0f8} to an object of type
  Union{@NamedTuple{r::UInt8, g::UInt8, b::UInt8}, Symbol}
The function `convert` exists, but no method is defined for this combination of argument types.

I'd say so, given I didn't intend for the constructor to work directly with Colorants regardless of the conversion methods implemented.

kimikage commented 2 months ago

I didn't intend for the constructor to work directly with Colorants

If you do not plan to make any changes in this package, I am ok with it. Now let's merge this.