processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.3k stars 1.26k forks source link

Keyword Syntax highlighting still outdated #3163

Open bojidar-bg opened 2 weeks ago

bojidar-bg commented 2 weeks ago

p5.js version

1.9.4

What is your operating system?

Linux

Web browser and version

Firefox 126.0

Actual Behavior

A few new p5.js function like beginClip are not being highlighted. https://github.com/processing/p5.js-web-editor/blob/develop/client/utils/p5-keywords.js was last updated c. #1873 and hasn't been touched since.

Expected Behavior

All p5.js functions are highlighted correctly. client/utils/p5-keywords.js is automatically generated when building the p5 editor and never drifts out of sync, or is at least kept in sync whenever the editor switches to the latest p5 version.

Steps to reproduce

Steps:

  1. Open https://editor.p5js.org
  2. Write beginClip()
  3. beginClip is not bolded like the other built-in functions.
welcome[bot] commented 2 weeks ago

Welcome! đź‘‹ Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

lindapaiste commented 1 week ago

I see that there's a bunch of PRs that re-run the generate script and update with the latest version. This is good but it's only a temporary solution. It brings us up to date with right now but does not consider future changes. (Also, all 3 PRs only update the p5-keywords.js file and not the p5-hinter.js file). I would like to see us focus on the "expected behavior" which is to update automatically.

Expected Behavior

All p5.js functions are highlighted correctly. client/utils/p5-keywords.js is automatically generated when building the p5 editor and never drifts out of sync, or is at least kept in sync whenever the editor switches to the latest p5 version.

I have some thoughts here but not a concrete solution in mind.

  1. My first idea was to add something before the build command is run. But if that's executed by the server when deploying to production then the changes won't be committed to this repo?
  2. We can add something to the husky pre-commit hook in the package.json file. That will run any time that any user commits anything so it could be excessive or annoying?
  3. Assume that these files only need to be updated when the p5 version is updated. Maybe we make it so that the p5 version is a variable in some file rather than hard-coding it into the createDefaultFiles.js. And instead of updating it manually, we have some command that will handle everything at once, updating the p5 version while also running the update-syntax-highlighting and update-p5-hinter scripts. Both scripts get their data by fetching https://p5js.org/reference/data.json, and that file does contain the version number. Additionally, we may want some sort of hook (pre-commit sort of thing) that fetches that file and checks to see if the version has changed.

I'm now realizing an interesting thought. If the user is editing an old sketch it might have a lower version of p5.js. We're going to show highlighting for these new functions, but those aren't actually available in the p5 version that they are using. That's a whole rabbit hole so let's ignore that for now. We would need to know what version a particular function was introduced, but that is not in the source data file which we are using.

bojidar-bg commented 1 week ago

But if that's executed by the server when deploying to production then the changes won't be committed to this repo?

Just like we don't commit node_modules, we don't need to commit any of the other generated files—unless they are really complicated to generate and/or it's important that everybody has the exact same version, neither of which is the case here. But I would defer the decision of whether this file should be part of the repository to the maintainers.

We're going to show highlighting for these new functions, but those aren't actually available in the p5 version that they are using.

Curious note, but the non-minified versions of p5.js actually contain the documentation within the script itself. There is probably no "nice" way to load it from there (chances are, doing it "safely" requires loading the script in an iframe; but we shouldn't be doing even that before the preview button is pressed), but it's definitely an interesting idea to explore.

Chaitanya1672 commented 1 week ago

@lindapaiste I think we can move the p5 version to the .env file, like this: P5JS_VERSION=1.9.4 , We can then pass it to createDefaultFiles.js. What do you think? and Should I try using a pre-commit hook to run those update scripts?