MycroftAI / mycroft-skills-kit

Mycroft Skills Kit
Apache License 2.0
28 stars 12 forks source link

Add icon and color getting when creating a skill #25

Closed LinusSkucas closed 5 years ago

LinusSkucas commented 5 years ago

This will allow skill developers, when developing with msk, to be able to add an image, and set its color when using msk create

krisgesling commented 5 years ago

Hey Linus this looks excellent, thank you!

Love the color highlights too.

I was wondering if we can also add a test for the length of the color string? My understanding is that a hex code can only be 4 or 7 characters long (eg #CCC or #BADA55). So maybe something like:

validator=lambda x: "#" in x[0] and len(x) in [4, 7],
on_fail="\n{red}Check that you entered a valid Hex color code including the #, and try again.{reset}\n"
forslund commented 5 years ago

My 0.05 cent: Don't use a lambda if you can avoid it. Use an inner function if something like that's needed.

LinusSkucas commented 5 years ago

Alright, I can add that check but, I don't know how to make inner functions/how to use them in this particular case...

forslund commented 5 years ago

An inner function is basically declaring a function in the local scope...

def my_func():
    def my_inner(thing):
        return "hello " + thing

    print(my_inner("world"))

In the validator lambda example it would be replaced with a named function

    def validate(x):
        """Validator for color strings."""
        return x[0] == "#" and len(x) in [4, 7]

in the same scope as you declare the other things. Might be overzealous and definitely not a required change...but maybe later we'd like to add a more rigid check that the color code can be parsed as a hex value or something.

LinusSkucas commented 5 years ago

Alright, I added the length checking. I wasn't able to figure out the inner function, so I just used lambda.

forslund commented 5 years ago

Looks good, merging

forslund commented 5 years ago

Realized why an inner function wouldn't work here...this is not run in a function/method, it's in a class scope... Sorry for the wild goose chase.