brownplt / code.pyret.org

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

Configurable, forced contexts #385

Closed jpolitz closed 3 years ago

jpolitz commented 3 years ago

create-and-image

change-namespace

jpolitz commented 3 years ago

What it looks like to load an existing file (Github won't let me upload more huge gifs): https://imgur.com/a/EbW1EbL

Notably, that file does not have use namespace legacy-cpo in it. It was prepended by loading it.

jpolitz commented 3 years ago

@schanzer @shriram for comments on the interactions suggested above.

jpolitz commented 3 years ago

Notably, you can see the definitions of these modules at

https://github.com/brownplt/code.pyret.org/pull/385/files#diff-80148c24be63a862e7006644518ae40f998df86e8b6e373eff345fd9e05817fa

https://github.com/brownplt/code.pyret.org/pull/385/files#diff-f729dd436283f8ee9e240e01b4ce29e9bfa587f27b040f5f664889a261a1e20f

schanzer commented 3 years ago

@jpolitz so... (1) a file without a use namespace XYZ declaration (e.g. - all files right now) will use the default. Nothing breaks. ✔️ (2) a file can specify use namespace XYZ at the top and immediately get a different set of included libraries, satisfying the use-case for my inelegant URL slug. ✔️ (3) there's extra UI attached to that first line to allow for a modal, which is a nice touch!

Overall I like it quite a bit!

Q1: How does someone like me edit or create a namespace? Q2: Can a namespace include symbols from another file (include ...), or can it only be built-ins?

jpolitz commented 3 years ago

Q1/2 – yes, the thing in the place of legacy-cpo or playground could be the shared-gdrive(...) part of a share import line, and it would mean:

Remove all bindings from the namespace entirely, and add only those explicitly provided from that module.

So for e.g. :DS you might want to import playground plus some other stuff (gdrive-sheets, etc), then re-export it all, and use that shared-gdrive as a single-line namespace followed by the table load.

jpolitz commented 3 years ago

To your 1/2/3 list of properties I would add explicitly – all new files have a different, broader default, which has image available (and IMO fixes some other inconsistencies). So there's a difference where files that are freshly created end up with playground but files that exist already get legacy-cpo, and neither user needs to click the use line for the common behavior to work.

There are TODOs here:

[x] Make the modal actually nice, thinking about student/teacher experience. [/ ] Have some kind of escape hatch to just edit the text of the file (not implemented, I think the modal does its job pretty well here) [x] Hardening to make sure the uneditable bit can never get wedged [x] Name choices for the words namespace, legacy-cpo, playground

schanzer commented 3 years ago

Ah, that's nice. Hell yes - I'm all for it! Let me know if there's anything you need from me.

jpolitz commented 3 years ago

I think I need your (@schanzer 's) gut reaction/predictions on how teachers will react to this grayed-out line, and how you'd explain it, and what words/interface you'd want to see there.

schanzer commented 3 years ago

This feels relatively easy to explain, actually -- I mean, it's a lighter lift than explaining include and import, which we already have to explain. So removing those and replacing them with "this special line tells Pyret to load lots of tools that we need for YOUR class" is an unquestionable win. The grayed-out UI is also easy to explain. I wonder - why not use a CM gutter and display a lock icon or something?

jpolitz commented 3 years ago

Another idea to add here (@blerner would appreciate your thoughts). Maybe the forced thing should not just be playground, but be playground2021, for new files. Instead of using include to get everything from Image and friends, we should actually explicitly list out everything. This makes the namespace file rather large, but also makes it future-proof. If we want to change the default to playground2022 next year, then we can add any new names without affecting the old one. Now the “slug” is part of each file, and it's not bad at all to maintain one file per year.

jpolitz commented 3 years ago

I wonder about the word context here. use context pyret2020 would be use namespace legacy-cpo, and use context pyret2021 would be the new thing with image. Then we update the use context line each time we want to introduce new bindings.

“Choose a context” is much more meaningful to put in the interface than “choose a namespace” as well.

It is (slightly) less jargon-y than namespace, it has a lovely equivalence to semantics, and I think it reads reasonably well.

blerner commented 3 years ago

Context isn't bad, and it avoids having to come up with a cutesy name for "beginner" or "playground". If we limit ourselves to one edition per year, we could use "edition" or "version", also.

One serious concern I just had: how to handle the removal of previously exported functions, like bitmap-url? Does the namespace definition file have to replicate those eliminated definitions?

One other thing I'd very much like is the ability to get statistics on how frequently a given namespace is used, so we can eventually deprecate or remove unneeded ones.

On Thu, Jul 1, 2021, 12:10 PM Joe Politz @.***> wrote:

I wonder about the word context here. use context pyret2020 would be use namespace legacy-cpo, and use context pyret2021 would be the new thing with image. Then we update the use context line.

It is slightly less jargon-y than namespace, it has a lovely equivalence to semantics, and I think it reads reasonably well.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brownplt/code.pyret.org/pull/385#issuecomment-872372575, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHAHQB3OF4UM7CP67VVV4DTVSHNTANCNFSM47RPDOZQ .

jpolitz commented 3 years ago

use context essentials2020 maybe.

Re: stats, we can do that. I think that's a safe logging thing to add, good idea!

For removal, that's a really good point. I think the context file gives us more flexibility, though (because the existing system would just break). We do have to update the context file to either (a) remove it [which we'll notice right away because CPO won't compile anymore] or (b) provide a shim. For bitmap-url I think this would work:

essentials2019.arr

import image as I
provide from I:
  image-url as bitmap-url,
  image-url,
  circle,
  square
  ...
end

If it's just argument order shuffling or a rename, we could even write wrappers directly in the context file and export those.

jpolitz commented 3 years ago

@blerner any objections to merging this into horizon (all the Selenium tests pass, current failures here are due to an annoying issue with branches in Travis)? I want to be able to point others at it to feel out the interface/experience, and do more cleanup on horizon if needed.