brownplt / code.pyret.org

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

Blocks definitions improvements #514

Closed asolove closed 4 months ago

asolove commented 4 months ago

Now that the core Blocks demo is mostly working on Horizon, I expect a long list of tiny nitpicks to the block definitions as we try things out. Creating this as a tracking issue to get them cleaned up.

Please send requests!

schanzer commented 4 months ago

@asolove aside from the bug I reported via email and the error highlighting improvements, I can't find anything else to complain about! I suspect we'll get an influx of such items after 3/21, once the Pyreteers start digging in again...

asolove commented 4 months ago

Yeah, I was similarly expecting to find more, but haven't yet, so let's just get this out.

@jpolitz this small change can merge to make a slight improvement to blocks on horizon

jpolitz commented 4 months ago

Looks good!

jpolitz commented 4 months ago

I know this is closed, but had a few comments

  1. I find strings weird. Maybe this is a Snap! thing. But it's bad IMO that we have:
image image
  1. When editing, I have to click somewhere on the Snap! canvas to unfocus and "commit" my edit before clicking the Run button. If I just typed a change to a constant and want to see the updates, so I click "Run", it runs the version before I made any changes.
asolove commented 4 months ago
  1. I 100% agree that the distinction between the string value boxes (rectangles) and number-value boxes (slightly rounded rectangles) doesn't seem clear enough to me. I make this mistake a lot. The Snap UI was designed for a language where you are almost always using literal strings or numbers, it's obvious from context which is right, and you're rarely using variables or nesting another computation to generate the value. I think I'd prefer a single box type that represents "a Pyret value literal", where digits become numbers and strings have to be in quotation marks.

  2. Yep, this is very annoying and I have a fix planned. (Today, we listen to a Snap event to run codegen when an edit operation finishes. Because of async fun, that event fires a few ticks after the blur event, which is fired only just before the event on our button. The fix is to just forcibly re-run codegen, possibly waiting a few ticks, when the run button is pressed.)

schanzer commented 4 months ago
  1. Yeah - I would really like to find a way to enforce that string values have quotes around them. Presumably Snap is adding them automatically during translation, so maybe we could check for Strings that are missing escaped quotes before passing the code to Pyret?
  2. @asolove this is the same issue we talked about a while back, right? Where deleting a column name from the demo wouldn't trigger codegen?
asolove commented 4 months ago

Nope, there are two separate issues: in the delete column case, Snap wasn't firing an event at all after the delete. (That is now fixed.) This is now a separate but related issue where if you're editing a value in the Snap text boxes, it only takes effect after you move the cursor somewhere else. So if you click the run button right while in the middle of an edit, we don't get the edits.

schanzer commented 4 months ago

@asolove got it. Different issues.

Did you say the first one is now fixed? Seems like it's still an issue for me.

asolove commented 4 months ago

@schanzer the snap update was in the same PR as error highlighting, and appears not to have been deployed.