brownplt / code.pyret.org

Website for serving Pyret to folks.
Other
24 stars 45 forks source link

Error messages need to wrap! #334

Closed schanzer closed 2 years ago

schanzer commented 4 years ago

This is just a CSS rule fix, but it's an important one. Some teachers are peeved that our wonderful errors aren't entirely readable. ;)

image

blerner commented 4 years ago

Unfortunately this is not "just a CSS rule to fix" -- it's a deliberate design decision and one that's more generally useful than you might expect. Pyret's raise function allows you to throw any Pyret value as an exception, e.g. image

The way we do that is by rendering the resulting value as a VS.vs-value, and that very much deliberately renders its results as a Pyret string, just as it would if the string appeared in any other Pyret value. However, this means that you can fix these error messages without making any changes to Pyret or CPO. Instead of saying raise("foo"), verbatim, you would raise a Pyret object with a _output method. Consider the following two functions:

import valueskeleton as VS

fun raise-msg(msg :: String):
  raise(msg)
  # This is visually equivalent to raise({ method _output(self): VS.vs-value(msg) })
end

fun raise-raw-msg(msg :: String):
  raise({ method _output(self): VS.vs-str(msg) end })
end

Calling these two functions results in image

So you can edit the background BS:DS code from saying raise("Cannot make a box plot, because...") to raise-raw-msg("Cannot make a box plot, because..."), using the definition of raise-raw-msg above, and you'll get a prettier-looking error message. Indeed all BS:* libraries ought to do something like this everywhere they raise a customized user-facing exception.

Technically, these error messages won't look 100% like built-in Pyret error messages -- contrast the blue, monospace font with the black, sans-serif text of a division-by-zero error: image The latter is a message-exception, while the results of raise are always user-exceptions. Message exceptions render via ED.text, while user exceptions render via ED.embed. There is no public API that exposes throwing a message-exception from user code. IIRC, we had wanted that restriction, so that a sufficiently assiduous observer could tell the difference between 3rd-part user-generated errors and 1st-party built-in errors. Having "2nd-party" teachpacks, that --- to our BS:* audience --- are indiscernible from built-in code, blurs that line a bit, and maybe we want to come up with an API that allows a user to raise an exception that directly renders an ErrorDisplay value (@jpolitz I don't want to make a unilateral decision here, but it seems to me that we could either add a new variant to RuntimeError that expected objects that could render their own ErrorDisplays, e.g. rich-user-exception, or we could somehow dynamically test whether the object in the user-exception can produce an ErrorDisplay, but this latter approach requires either exposing has-field or permitting multiple return types from a method, which I don't much want to do...)

(In general, honestly, Pyret libraries shouldn't really ever call raise with a string value directly; we can do better. That does require a change in Pyret and in CPO, and is a worthwhile PR for someone to take on.)

schanzer commented 2 years ago

Now that #286 is closed, I'm comfortable closing this one.