Skip to content

fix: restored p5.js keyword highlighting for CM6 #3868#4141

Merged
khanniie merged 2 commits into
processing:develop-codemirror-v6from
parthjadhao01:fix/p5-syntax-highlighting-cm6
Jun 2, 2026
Merged

fix: restored p5.js keyword highlighting for CM6 #3868#4141
khanniie merged 2 commits into
processing:develop-codemirror-v6from
parthjadhao01:fix/p5-syntax-highlighting-cm6

Conversation

@parthjadhao01
Copy link
Copy Markdown
Contributor

@parthjadhao01 parthjadhao01 commented Jun 1, 2026

Issue:

Fixes #3868

This PR restores p5.js-specific syntax highlighting in the CodeMirror v6 branch. In the CM5 editor, p5 functions (like setup, draw, background) appeared bold blue and p5 constants/variables (like PI, mouseX, WEBGL) appeared pink. This styling was missing in the CM6 branch.

Demo:

after fix :

image

befor fix :

image

Changes:

  • Added p5Highlight.js — a CM6 ViewPlugin that walks the Lezer syntax tree over visible ranges, checks each VariableName and VariableDefinition node against the p5 keyword lists from p5-keywords.js, and applies cm-p5-function or cm-p5-variable decorations
  • Updated _editor.scss — added styles for .cm-p5-function (bold blue) and .cm-p5-variable (pink) using existing theme variables so all three themes (light, dark, high-contrast) are supported
  • Updated stateUtils.js — registered p5Highlight in the extensions array

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • has no typecheck errors (npm run typecheck)
  • is from a uniquely-named feature branch and is up to date with the develop-codemirror-v6 branch.
  • is descriptively named and links to an issue number, i.e. Fixes #3868
  • meets the standards outlined in the accessibility guidelines

@yugalkaushik
Copy link
Copy Markdown
Contributor

@parthjadhao01 I tested your PR locally and the code works, but I do not think the approach is correct. In the develop branch, variable coloring is defined inside the utils folder, and with the current implementation the overall structure changes. Please check the exact implementation in the develop branch where the variable color changes to pink. I dug into it a bit and some of the logic seems to be inside the utils folder. It would be better to follow the same approach and implement it in the codemirror branch. Otherwise, this can create unnecessary inconsistency and make future contributions harder to maintain.

@parthjadhao01
Copy link
Copy Markdown
Contributor Author

Thanks @yugalkaushik for the detailed feedback! I checked the develop branch and now I understand — the CSS for .cm-p5-function and .cm-p5-variable already exists in the theme files (_p5-light-codemirror-theme.scss, _p5-dark-codemirror-theme.scss, _p5-contrast-codemirror-theme.scss), and the highlighting logic belongs inside p5JavaScript.js following the same pattern as utils/p5-javascript.js in the develop branch. I'll move the ViewPlugin into p5JavaScript.js, remove the separate p5Highlight.js file, and remove the duplicate CSS I added to _editor.scss. Will push the update shortly.

@parthjadhao01
Copy link
Copy Markdown
Contributor Author

@yugalkaushik After checking the theme files in the develop branch, I found that theming works differently between CM5 and CM6.

In CM5, the theme class is added directly to the editor element, so this CSS in the theme files works:
https://github.com/processing/p5.js-web-editor/blob/develop/client/styles/components/_p5-light-codemirror-theme.scss#L124-L131

But in CM6, EditorView.theme() generates scoped classes internally — the .cm-s-p5-* class is never added to the editor element, so those selectors have no effect. This is documented in the CM6 migration guide: https://codemirror.net/docs/migration/#themes

The CM6 branch already handles theming through themify() in _editor.scss. For example, this is how .cm-variable is styled:
https://github.com/processing/p5.js-web-editor/blob/develop-codemirror-v6/client/styles/components/_editor.scss#L225-L229

So the equivalent CM6 styles for .cm-p5-function and .cm-p5-variable would follow the same pattern:

- .cm-s-p5-light .cm-p5-function {
-   color: $p5-light-blue;
-   font-weight: bold;
- }
- .cm-s-p5-light .cm-p5-variable {
-   color: $p5-light-pink;
- }

+ .cm-p5-function,
+ .cm-p5-function .cm-variable {
+   @include themify() {
+     color: getThemifyVariable('highlight-style-variable-color');
+     font-weight: bold;
+   }
+ }
+ .cm-p5-variable,
+ .cm-p5-variable .cm-variable {
+   @include themify() {
+     color: getThemifyVariable('highlight-style-atom-color');
+   }
+ }

Please let me know if you have a different approach in mind, I'll go ahead with this.

@yugalkaushik
Copy link
Copy Markdown
Contributor

Yes go ahead with this approach.

@parthjadhao01
Copy link
Copy Markdown
Contributor Author

@yugalkaushik I kept the highlighting logic in Editor/p5JavaScript.js instead of utils/ because it imports p5CompletionPreview which is Editor-specific. Moving it to utils/ would create a circular dependency. The pattern is the same though — all p5-specific behavior lives in one file, just like utils/p5-javascript.js does in the CM5 branch.

@khanniie
Copy link
Copy Markdown
Member

khanniie commented Jun 2, 2026

Thanks for the PR! looks great, merging it in now. appreciate the work you both put into it!

@khanniie khanniie merged commit cb96dcb into processing:develop-codemirror-v6 Jun 2, 2026
1 check passed
@parthjadhao01 parthjadhao01 deleted the fix/p5-syntax-highlighting-cm6 branch June 3, 2026 05:08
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.

3 participants