Skip to content

prototype snap integration with CPO #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 22, 2023
Merged

Conversation

pcardune
Copy link
Contributor

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:

  • stop copying the entire git directory since this includes the entire git history which is 1.4GB
  • reuse common code between /blocks and /editor, or combine the two. I opted to skip this since it will probably break something and I'm not sure what the use case is. This is just a prototype after all.

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

@pcardune
Copy link
Contributor Author

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!

@schanzer schanzer requested a review from jpolitz January 11, 2023 17:39
@shriram
Copy link
Member

shriram commented Jan 11, 2023 via email

@jpolitz
Copy link
Member

jpolitz commented Jan 12, 2023

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
Copy link
Member

shriram commented Jan 12, 2023 via email

@jmoenig
Copy link

jmoenig commented Jan 12, 2023

@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
Copy link
Contributor Author

pcardune commented Feb 5, 2023

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
Copy link

schanzer commented Feb 7, 2023

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
Copy link
Member

jpolitz commented Feb 7, 2023

(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
Copy link

jmoenig commented Feb 10, 2023

@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
Copy link
Contributor Author

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
Copy link

jmoenig commented Feb 13, 2023

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
Copy link

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
Copy link

jmoenig commented Mar 1, 2023

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 jpolitz mentioned this pull request Aug 22, 2023
@jpolitz jpolitz merged commit 8d7a314 into brownplt:horizon Aug 22, 2023
@jpolitz
Copy link
Member

jpolitz commented Aug 22, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants