brownplt / code.pyret.org

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

prototype snap integration with CPO #462

Closed pcardune closed 1 year ago

pcardune commented 1 year ago

This PR adds a /blocks page (which is mostly a copy paste of /editor) that integrates Snap into the IDE.

Here's a screenshot:

Screenshot 2023-01-11 at 9 24 42 AM

Snap isn't packaged in any way, so I resorted to using git submodules to add it in, and copying the entire git directory into the build/ directory in the Makefile.

Some improvements that could be made:

Thanks to @jmoenig for making the changes to Snap that allow for this integration. cc @schanzer who would like to try this out.

pcardune commented 1 year ago

cc @blerner @jpolitz, I'm told that you guys are the people who should review this. Looks like I don't have permission to assign reviewers, so mentioning your name here!

shriram commented 1 year ago

WHAT!!!!! WOW!!!!!!

(Sorry, that is not a code review.)

jpolitz commented 1 year ago

Very cool!

Yeah, refactoring /editor for shared code would be hard at this point.

I guess the next step is some kind of configuration file to do the code mappings? I can add them manually with right-click to get for to mean for each, and I'm sure @shriram knows how to configure that from the stack trace work.

image
shriram commented 1 year ago

Yeah, we can do that. @Elijah E Rivera @.***> has really become the master of this since then.

How do we connect up the blocks with how they translate into Pyret?

jmoenig commented 1 year ago

@shriram the mappings are handled by the transpile.xml Snap project. You can edit that project in Snap to explore how it's done. There will eventually be documentation about this, in the meantime I'm more than happy to explain the mechanism in person to anyone who's interested.

pcardune commented 1 year ago

My last commit hooks up @jmoenig's most recent support for exporting/loading just the additional blocks that are being added. For now this just saves and loads from local storage.

A couple questions:

  1. For @jmoenig: we'll want to call synchScriptsFrom at page load time as soon as the IDE is ready. From my testing, it seems like I have to wait some period of time after the ide is setup before I can load the scripts. Is there some kind of "onready" event I can hook into on the ide to know when it's ready to receive a script to load?
  2. For @schanzer and @jpolitz, I looked into CPO's create/open/save workflow with google drive and it seems like we have a few options for supporting create/open/save with blocks.

    a) We refactor CPO's workflow to support multiple file types. When creating a new file you can choose between a pyret .arr file or a blocks .xml file, and when opening a file you can choose to load .arr or .xml files. b) We fork / duplicate this functionality for blocks and if you go to /blocks we use the blocks version that only works with .xml files and if you go to /editor everything stays the same. c) We overload the meaning of .arr files and at file load time try to determine whether this is an xml file that should be loaded as blocks, or a pyret file that should be loaded as text and "do the right thing."

    Do y'all have any thoughts on what the best option would be?

schanzer commented 1 year ago

I always defer to @jpolitz on all things Pyret-related, but I'm guessing he's going to veto (c) right away. I'm inclined to say option (b) allows us to get this out into the world faster and simplifies our UI complexity considerably. Merging the UIs and refactoring the workflow to support multiple filetypes would need to be done together, and I suspect there are UI questions we won't be able to answer until we get this into teachers' hands and see how they're using it.

jpolitz commented 1 year ago

(a) felt like the right thing to do at first, but I worry that CPO doesn't have much of a well-defined interface for what a “file” is, and there'd be a bunch of weird stuff about source locations / error messages that makes assumptions about there being a Pyret file. That could be a real mess; I wouldn't look forward to that refactor at all.

That said, if someone was excited to take that on and kept all the UI tests running while doing the refactor, it's not a terrible risk.

(b) is what I'd suggest, though, to get user feedback. Long-term the right thing might be to put this into the new editor at /repartee in some way, too. Based on the changes as-is this looks like it's really a pretty standalone prototype of “embed the Snap editor” with little dependence on CPO so far, so that could be really nice.

@schanzer is pretty much right about (c), though I think the detection is only medium-scary. It's really that having another way of running programs on the same page is just, well, challenging given the state and complexity of that system.

jmoenig commented 1 year ago

@pcardune I've updated the Snap api so you can provide a callback to the configuration dictionary that gets called when the transpile microworld is loaded and ready:

https://github.com/jmoenig/Snap/blob/f24b00c1d43917ac03b989c6229daef8f8ffe336/docs/API.md?plain=1#L114

I've also updated my little POC page with an example how this could be used:

https://github.com/jmoenig/Snap/blob/f24b00c1d43917ac03b989c6229daef8f8ffe336/pyret/inline.html#L35

This lets you synchScriptsFrom an existing xml string as soon as the Snap IDE and its transpile program is loaded and as soon as you've fetched the user xml that's to be edited.

Please let me know if you encounter any issue or if there's anything I can contribute / help you with! (have I mentioned lately how excited I am about this project?)

pcardune commented 1 year ago

Thanks @jmoenig! I've updated this branch to that use that loader.

@jpolitz I hacked together a fork of beforePyret.js (now called beforeBlocks.js) to save/load block xml files to/from google drive. I also added a hack to check the contents of the file for the blocks xml and redirect to /blocks in order to support opening files from the root dashboard without copying and pasting a lot of code. See this commit for the changes. I'm sure there are bugs, because it's very difficult to figure out all the entry points.

Anyway, I think the next step is to update the transpile.xml file that @jmoenig mentioned to do the code mappings for all the other blocks. Is that something you want to take care of @jpolitz? @schanzer was also talking about changing the set of blocks that are available to choose from (including potentially their appearance?)

jmoenig commented 1 year ago

yay, thanks @pcardune ! BTW if y'all need help with updating and extending the transpile.xml project I'm more than happy to help with that. As a general guideline I'd suggest create a custom block for every Pyret function you want to offer in the blocks palette, i.e. not to try to somehow map any Snap primitives. Those custom blocks don't need to define any behavior in Snap as we're not ever going to evaluate them inside Snap, so we're free to make them just right for Pyret. Also, we probably can get rid of all the stuff in the transpile.xml project that tests and runs these blocks inside Snap, because we don't need any of that. This will make the Snap side of the project much less cluttered and straightforward to maintain.

schanzer commented 1 year ago

Apologies for the delay - February and March contain a 6-week stretch where I'm running flat-out on multiple projects, so my latency is spiking.

@pcardune can you share the client_id information (and anything else I need) to properly connect to Google Drive? @jmoenig how do I import the transpile.xml file? Or should I just try editing this in normal Snap and saving the result back to my machine to see how it performs with CPO blocks?

Finally - minor, unimportant nit - the blocks window is tiny and fixed, rather than filling up the Definitions Area and resizing with it. It also sits atop CPO's menus. I tried appending the canvas to replMain after the page loaded, but my jQuery Foo is weak and I'm clearly not doing it correctly.

jmoenig commented 1 year ago

Same here, @schanzer, I'm myself pretty much tied up for the next 2-3 weeks, so no worries. It's best to edit transpile.xml in regular Snap and export it back to your machine, yes. That's how I always do it myself. Again, this is the fun part, and I'm totally eager to help, or do it together with you! Wrt to resizing widgets I'm probably the world's least competent web programmer. The whole point of creating Morphic.js was to avoid having to deal with the DOM and with JQuery ;)

jpolitz commented 1 year ago

@pcardune @jmoenig extremely cool to see how well this fit together, many thanks + kudos. More to do + design, I've just made my first few block definitions for this and can see a lot of cool ideas.