Open shriram opened 9 years ago
That would also avoid "grey" vs "gray" confusion.
For this particular error, the plan is to solve it completely by making Color
be a data type, with a boatload of singleton constructors -- it's a massive enum, basically -- plus one remaining constructor to give RGB triples. Then Color
will have proper documentation, and it'll work out fairly neatly.
Having a hacked-in message for this particular error would be bad architecting, since it's just a regular contract error and we want to handle those uniformly. So a more general solution for contracts to supply a "more info" link might be a good mechanism to design.
Both points here are just fine by me.
So we don't miss it when closing one or the other:
Are we going to change the color library? If not, then we should have a better solution here. It looks like we haven't changed it in the two years we've talked about this…
Did we actually come to a conclusion in the email thread from August 18 ("color proposal summary"), did we just get exhausted with it, or did we decide to defer this until next year because people have been trained on the current busted system and changing it out from under them would be bad?
We came to a conclusion. It's in email and on Slack. Duplicating here.
Shriram:
This is only for Pyret, but with an eye to keeping Pyret beginner-friendly for BS:1-like material.
Color will be a single-variant datatype (i.e., a struct) w/ RGB/alpha (and maybe other) fields. [As it is already.]
There will be one or more functions of type
s2c :: String -> Color # actual name TBD
and maybe
String, Number[for alpha channel] -> Color
and maybe others. Just to show that these functions actually "do some work", we could consider making them be quite forgiving about how a name is given: e.g., `s2c("red")`, `s2c("Red")`, `s2c("ReD")`, and `s2c("RED")` might all work and produce the same result.
Image functions will consume Colors, not Strings. Thus
star(50, color(255, 0, 0, 255))
star(50, s2c("Red"))
are both valid and identical, but
star(50, "red")
will produce "Expected to get a Color but was given the String "red" instead".
[NOTE: Yes, it may be confusing to be told that "red" is not a color. But this is no different from typing `1 + "2"` and being told "Expected to get a Number but was given the String "2" instead": it's just as confusing to be told "2" is not a Number. We should certainly work on making sure the error messages are really good to make these cases as clear and crisp as possible (they're not there yet). But some amount of confusion of this sort is inevitable and has to be taught: it's the essence of representations in CS.]
The upside to this proposal is that we can have a clean, typed interface. Turning on the type checker on existing programs (either those without functions, or those functions have been properly annotated) will Just Work.
The downside is that writing small image code becomes a nuisance: we have to write
star(50, s2c("red"))
To address that, we'll have [as we already do] pre-defined constants such as
red = s2c("red")
One downside Emu has pointed out is the current error message. One mitigation for it is to change the text: instead of saying things about unbound identifiers, it should just say
> hara # Hindi for green
`hara` is not defined
It's not _quite_ as nice as saying "I'm sorry, I don't recognize "hara" as a color", but it's much better than the current error message, and making this fix in fact improves Pyret overall independent of any colors/image business.
Finally, Pyret will pretty-print color values as swatches, and this will be the default display (clicking will give the color struct).
So, to summarize the changes:
- printing of colors shows color swatches with clicking
- "unbound identifier" error becomes neutral (and thus arguably less unhelpful)
- Pyret can have a properly typed interface [in this argument]
- we move the current color interpretation code out of the image functions
[down the road, we may get 3rd party contribs of other languages;
in fact, since the parameter to s2c is a string, which can contain Unicode,
one could actually write `s2c("हरा")`]
Ben:
Several followup points:
1) This doesn't have to preclude convenient named identifiers for predefined colors. We still can have a library called "named-colors", and you can _opt in_ to having a bunch of predefined colors by saying `include named-colors`
Or even better, once we have a "real" IDE, say `import named-colors as NC`, and then typing `NC.` will auto-complete with all the color names you could want.
Having a `named-color("red")` (or more likely given our "type-" prefixing convention, `color-named("red")` function can be orthogonal to having identifiers.
2) Also in line with the algebraic roots of BS:1, we could totally define algebra on colors: `color-mix(0.3, red, 0.7, blue)`, for example (assuming I've already said `include named-colors` for those constants ;-)
3) Extending the rich-rendering of Colors to show swatches is a fascinating engineering task; it is _not_ trivial given our current architecture: unlike images and numbers and strings (which are all defined in the runtime system, and so can cheat like hell), Color will be defined in Pyret (and so we have to engineer a way for it to have multiple renderings). The upside is, that engineering will help other types, too :)
3a) We need to be very careful with how we render swatches -- in particular, it must not render as a rectangle or circle, or else students will likely conflate the rendering of a swatch with the rendering of a small image. Some sort of blobby shape, or some sort of paint-swatch-looking thing, will be better.
Joe:
Now that I see this all laid out, I'm much happier with it. Naming s2c well seems important; right now it's present as `name-to-color`, but it returns false rather than throwing an error, which should be considered a bug.
I much prefer the "not defined" message to what we have, though it's hard to think through other scenarios of it quickly. It's not very actionable, but that message in particular may be difficult to make actionable, and it's at least usefully descriptive in many contexts.
Swatches are a great idea.
I think `color` might be a good name for the function that returns a color. `rgb-color` and `rgba-color` would be good names for the `Color` constructors, and make them more explicit.
Ben:
What's needed are two things: First, a new combinator in the DSL saying "I've got multiple renderings to cycle among". That itself will be tricky to interpret (primarily, how to determine where the cycle-on-click handlers should go). Second, we need to produce the renderings for Colors -- and one of those could just be a Pyret Image that composes a paintbrush icon next to a swatch of color and a name (if one exists).
This is addressed in the new image-structs library
, no?.
@blerner, doesn't your new library address this?
Yes and no -- there are now two image libraries: one legacy (with stringy arguments) and one strongly-typed (with enumerated arguments). Colors fit into both. And finding the list of valid color names is really a docs issue, and maybe a question of how to get links to docs to show up in error messages. At least if you go to the docs and search for "color" there's a link to Predefined Colors, so this is definitely better.
The error message for colors just says
It would be so much more user-friendly to say "here's a link giving the list of legal color names". (For instance, "light blue" is a valid color, but "light pink" is not...)