Skip to content
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

Keyword Syntax highlighting still outdated #3163

Open
bojidar-bg opened this issue Jun 14, 2024 · 4 comments
Open

Keyword Syntax highlighting still outdated #3163

bojidar-bg opened this issue Jun 14, 2024 · 4 comments

Comments

@bojidar-bg
Copy link

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.
@bojidar-bg bojidar-bg added the Bug label Jun 14, 2024
Copy link

welcome bot commented Jun 14, 2024

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

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

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

Chaitanya1672 commented Jun 18, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment