fix: restored p5.js keyword highlighting for CM6 #3868#4141
Conversation
|
@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. |
|
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. |
|
@yugalkaushik After checking the theme files in the In CM5, the theme class is added directly to the editor element, so this CSS in the theme files works: But in CM6, The CM6 branch already handles theming through So the equivalent CM6 styles for - .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. |
|
Yes go ahead with this approach. |
|
@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. |
|
Thanks for the PR! looks great, merging it in now. appreciate the work you both put into it! |
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 (likePI,mouseX,WEBGL) appeared pink. This styling was missing in the CM6 branch.Demo:
after fix :
befor fix :
Changes:
p5Highlight.js— a CM6ViewPluginthat walks the Lezer syntax tree over visible ranges, checks eachVariableNameandVariableDefinitionnode against the p5 keyword lists fromp5-keywords.js, and appliescm-p5-functionorcm-p5-variabledecorations_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 supportedstateUtils.js— registeredp5Highlightin the extensions arrayI have verified that this pull request:
npm run lint)npm run test)npm run typecheck)develop-codemirror-v6branch.Fixes #3868