Picker fails to "select" colours in okhsl and okhsv spaces

maegul commented 2 years ago

Please read and fill out this template by replacing the instructions with appropriate information. If the template is not followed, the issue will be marked Invalid and closed.

Before submitting an issue search past issues and read the area of the documentation related to your specific question, issue, or request.

Description

(Another one for you!) ...

... what is the issue / request ?

The color picker tool, when selected to work in either okhsl or okhsv space, crashes when a color is "selected" with the following exception in the console:

Traceback (most recent call last):
  File "/Applications/Sublime Text 4.app/Contents/MacOS/Lib/python33/sublime_plugin.py", line 1488, in run_
    return self.run(edit, **args)
  File "/Users/errollloyd/Library/Application Support/Sublime Text/Installed Packages/ColorHelper.sublime-package/ch_panel.py", line 781, in run
  File "/Users/errollloyd/Library/Application Support/Sublime Text/Installed Packages/ColorHelper.sublime-package/ch_panel.py", line 503, in show_insert
  File "/Users/errollloyd/Library/Application Support/Sublime Text/Installed Packages/ColorHelper.sublime-package/lib/coloraide/color.py", line 151, in __init__
  File "/Users/errollloyd/Library/Application Support/Sublime Text/Installed Packages/ColorHelper.sublime-package/lib/coloraide/color.py", line 235, in _parse
ValueError: '{}' is not a registered color space

This does not occur for all the other spaces (including hwb)

Relevant Settings

... or relevant as far as I can tell.

{
"enabled_color_picker_modes": ["srgb", "hsl", "hsv", "okhsl", "okhsv", "hwb"],
"add_to_default_spaces": [
        "ColorHelper.lib.coloraide.spaces.hsluv.HSLuv",
        "ColorHelper.lib.coloraide.spaces.lchuv.LChuv",
        "ColorHelper.lib.coloraide.spaces.luv.Luv",
        "ColorHelper.lib.coloraide.spaces.okhsl.Okhsl",
        "ColorHelper.lib.coloraide.spaces.okhsv.Okhsv"
    ],
}

Vague issues/requests will be marked with Insufficient Details for about a week. If not corrected, they will be marked Stale for about a week and then closed.

For feature requests or proposals:

Clearly define in as much detail as possible how you imagine the feature to work.

Examples are also appreciated.

For bugs and support questions:

Describe the bug/question in as much detail as possible to make it clear what is wrong or what you do not > understand.

Provide errors from console (if available).

Pictures or screencasts can also be used to clarify what the issue is or what the question is.

Provide links to 3rd party syntax highlighting package you are using if applicable.

Support Info

ST ver.: 4131
Platform: osx
Arch: x64
Plugin ver.: 6.0.1
Install via PC: True
mdpopups ver.: 4.2.2

Run the following command from the menu: Preferences->Package Settings->ColorHelper->Support Info. Post the result here.

Steps to Reproduce Issue

Raise Colour Picker (either through command palette or selecting a preview swatch) a. Ensure okhsl and/or okhsv are enabled as colour picker modes in the settings
Change mode to either okhsl or okhsv
Click select in bottom left corner of picker window.

Provide steps to reproduce the issue. Pictures are fine, but also provide code/text I can copy and paste in order to reproduce. Omit for feature requests and feature proposals.

maegul commented 2 years ago

In case it's relevant ... I find the conversion tool to work perfectly fine for color spaces like oklab.

facelessuser commented 2 years ago

When we switched to these dynamic color objects that can contain whatever color spaces the user selects, we overlooked an area where when we read in the colors from the setting, and it doesn't specify a color object, it will give the default location of ColorHelper.lib.coloraide.Color. We should intercept this and use the default, dynamic color base instead.

That's the one we use as our default which includes the color spaces the user defines in the default, which yes, includes okhsl and okhsv.

    "add_to_default_spaces": [
        // "ColorHelper.lib.coloraide.spaces.aces2065_1.ACES20651",
        // "ColorHelper.lib.coloraide.spaces.acescc.ACEScc",
        // "ColorHelper.lib.coloraide.spaces.acescg.ACEScg",
        // "ColorHelper.lib.coloraide.spaces.acescct.ACEScct",
        // "ColorHelper.lib.coloraide.spaces.cmy.CMY",
        // "ColorHelper.lib.coloraide.spaces.cmyk.CMYK",
        // "ColorHelper.lib.coloraide.spaces.din99o.DIN99o",
        // "ColorHelper.lib.coloraide.spaces.hpluv.HPLuv",
        // "ColorHelper.lib.coloraide.spaces.hsi.HSI",
        // "ColorHelper.lib.coloraide.spaces.hunter_lab.HunterLab",
        // "ColorHelper.lib.coloraide.spaces.ictcp.ICtCp",
        // "ColorHelper.lib.coloraide.spaces.igtgpg.IgTgPg",
        // "ColorHelper.lib.coloraide.spaces.itp.ITP",
        // "ColorHelper.lib.coloraide.spaces.jzazbz.Jzazbz",
        // "ColorHelper.lib.coloraide.spaces.jzczhz.JzCzhz",
        // "ColorHelper.lib.coloraide.spaces.lch99o.LCh99o",
        // "ColorHelper.lib.coloraide.spaces.orgb.oRGB",
        // "ColorHelper.lib.coloraide.spaces.prismatic.Prismatic",
        // "ColorHelper.lib.coloraide.spaces.rec2100pq.Rec2100PQ",
        // "ColorHelper.lib.coloraide.spaces.rlab.RLAB",
        // "ColorHelper.lib.coloraide.spaces.ucs.UCS",
        // "ColorHelper.lib.coloraide.spaces.uvw.UVW",
        // "ColorHelper.lib.coloraide.spaces.xyy.xyY",
        "ColorHelper.lib.coloraide.spaces.hsluv.HSLuv",
        "ColorHelper.lib.coloraide.spaces.lchuv.LChuv",
        "ColorHelper.lib.coloraide.spaces.luv.Luv",
        "ColorHelper.lib.coloraide.spaces.okhsl.Okhsl",
        "ColorHelper.lib.coloraide.spaces.okhsv.Okhsv"
    ],

I simply missed testing this case after the upgrade.

facelessuser commented 2 years ago

The fix is tagged and released. I double-checked similar cases, and I think we should be good. Luckily it's consolidated to only two places.

facelessuser / ColorHelper