Blocks error highlighting

[asolove]

This PR adds basic error highlighting to the Blocks UI.

• When an error is shown in the REPL side, hovering the highlighted bits trigger a matching highlight on the top-level block where the error occured:

• It also upgrades Snap to the latest version, which fixes a bug we encountered with Snap not re-running our code generation after some UI interactions.

[asolove]

^ Rebased on horizon to make the eventual merge easier and to upgrade to the latest version of Snap.

[asolove]

Just so folks can see what is possible with the new outline and bubble message APIs, here is a quick preview.

• I've used the error color as the outline of the highlighted block.
• I've put the error message in a help balloon contextually on the relevant block. Note that the bubble location is a bit fuzzy: centered on the entire block it's relevant to, not necessarily the exact location within it. (This may or may not be a good idea, either by itself or in addition to the error messages showing up on the REPL side. Just wanted to show that it's an option we have now.)

[jpolitz]

👀 this is awesome, the bubble interface is super cool. Need anything from me here as you work through it?

[asolove]

Not yet. I think we need a bit longer to decide what we want the experience to be. Once we have that, I'd love to get you to review a plan on what layer in the code we branch between the blocks and non-block handling, to keep things maintainable.

[asolove]

^ rebased with other recent blocks work, refactored the error code to be a bit simpler, and upgraded to latest snap to get a bug fix that we needed.

[schanzer]

@asolove this is seriously cool. Do you think we're at a place where we could merge this horizon, in time for the 21st? It's non-invasive to horizon, since without loading the blocks editor it's impossible for anyone else to stumble into this code path.

[asolove]

@schanzer I think what's here is now mergeable.

We've discussed lots of options, so a quick update on what's done and what's still up next.

• What's in here now is that when you compile a blocks program and get an error, you can hover the highlighted section of the error message and the top-level block where that error occurred is highlighted in a matching color:

That's clearly helpful and better than it not being there. We should merge this. But it's not as good as it could be.

Other stuff I'd like to do:

• finer-grained highlighting: in this case, it'd be really nice to highlight just the usage of b rather than the full context where it's used
• using borders: right now, the blocks highlighting only shows once you hover to interact with the error message. I'd like to add in usage of the new Snap outlining so that we show a visual indication as soon as the error message is displayed in the REPL.
• bubble popups for errors: using the Blocks bubble UI to show error messages right over the block where they were triggered. (This is tricky because we have very verbose error messages designed for extensive reading in the REPL side, and may need to go provide short summaries for a bunch of cases so it works nicely in the bubbles. Or just use a generic "the problem is near here")

[asolove]

@jpolitz: I think this is ready for review. The blocks side is working well and I reviewed that the non-Blocks case isn't broken in any obvious way.

Medium-term, I'd like to push the logic for blocks v regular mode error handling lower down into the hint/position/anchor code, rather than high-up here at the event handlers, but this seemed like the most "clearly not breaking the world" way to do it for now. (And I have no idea how this will mix with anchor, which I guess we should talk about at some point.)

[schanzer]

@asolove The finer-grained highlighting will depend on Jens' continued improvements on the Snap! side, right? He's already made some strides in that direction, but ultimately there's nothing else you can do besides sending the source locations.

Outlining is definitely a must. It doesn't block a demo on the 21st, but it would be a killer feature to have in time. Do you think this is possible, now that Jens has added the outlining code?

100% agree on the bubbles. I think the right solution for now is to put up a bubble that says "be sure to read the error message in the Interactions Area!" I suspect whatever code you write here will have to change for Anchor anyway, so I'd rather put our efforts into Anchor once we reach "good enough" on this side.

[asolove]

@schanzer agreed on all points:

• Fine-grained error locations: I need to experiment a bit more to see if I can trick Snap a bit, but think the Snap side is currently the limiting factor on this.
• Outlining: it's already possible but there's some weird interactions I need to iron out. Could be ready by the 21st, just depends if I can track down what's breaking for me now.
• Error bubbles: Agreed to not handle lots of shorthand error cases for now and figure it out later.

[jpolitz]

Wow, this is really cool.

[jpolitz]

Just a heads up, Chrome had a new version today but chromedriver hasn't so the build is waiting for https://github.com/giggio/node-chromedriver/pull/459 to happen, so we got https://app.travis-ci.com/github/brownplt/code.pyret.org/builds/269572980. I can kick Travis to re-build once chromedriver updates.

(This is a detail of the CI that we expect chromedriver to uphold the property that its major version matches Travis CI's major Chrome version, which is often not true for a few hours each month. Totally expected and not a big deal.)

[jpolitz]

OK cool this is up on https://pyret-horizon.herokuapp.com/blocks. Looks quite good.

[shriram]

Wow, this is a huge win! However, I'm not quite seeing this working on my browser? (Chrome 122.0.6261.129 on macOS)

Eg,

produces an unbound x error, but hovering over things causes blinking in the REPL but doesn't seem to highlight anything in the block region.

Say I change the bound variable to x (so the function is fine), but in the application, instead of using the variable f I just type f, which is the string "f":

Again, there's an error:

but hovering doesn't result in anything happening to the blocks.

Maybe I've misunderstood what I should be seeing?

[asolove]

Hmm, +1 that the blocks highlighting behavior doesn't seem to be working for me on horizon.

[asolove]

I don't think this change got launched, or there may have been some build or cache problem. When I search through https://s3.amazonaws.com/pyret-horizon/new-horizons/cpo-main.jarr.gz.js I can see highlight_anchor_hover (which is in the code right next to this change) but not flashSpriteScriptAt, which is from the diff for this PR.

[jpolitz]

Ugh you're right. But it should be updating...

https://app.travis-ci.com/github/brownplt/code.pyret.org/builds/269594551#L671 https://app.travis-ci.com/github/brownplt/code.pyret.org/builds/269594551#L1145

Everything seems happy with build and deploy, I ran it again this morning to make sure (that's from this morning's build). Frustrating. Nothing has changed about this in a long time, and the other updates are going through just fine.

[jpolitz]

And in the AWS console...

Sorry this is happening! I'm very confused.

[jpolitz]

OK now I see that code in all places:

joe@rooibos code.pyret.org % curl -s https://s3.amazonaws.com/pyret-horizon/new-horizons/cpo-main.jarr.gz.js | gunzip | grep -on flashSpriteScriptAt
1:flashSpriteScriptAt

[jpolitz]

Seems to be working for me now.

[shriram]

OMG it so totally works! This is amazing!

[jpolitz]

1. I reran both CI jobs by clicking "Restart Build", but out of order, so the defs changes clobbered the highlighting ones
2. There was some real cache delay either in my browser or from S3 this morning that confused me a bit

So I should always git commit --allow-empty -m "redeploy" to trigger Travis, the UI lets you do weird time travel w.r.t. how I think about deploying.