Skip to content

GDScript code highlighting#2892

Open
DavidCooperWCU wants to merge 4 commits into
PreTeXtBook:masterfrom
DavidCooperWCU:master
Open

GDScript code highlighting#2892
DavidCooperWCU wants to merge 4 commits into
PreTeXtBook:masterfrom
DavidCooperWCU:master

Conversation

@DavidCooperWCU
Copy link
Copy Markdown

Made necessary changes for GDScript 2.0 highlighting.

Documentation was updated to include runestone support, but runestone activecode isn't working yet. (Right now it's set to work with JOBE, but I'm leaning towards a client side solution more like sql.)

Prism has GDScript 1.0 highlighting, so I had to add some custom javascript to handle code with @.

Latex's listings library supports python, which is pretty close, but not a perfect match.

@rbeezer
Copy link
Copy Markdown
Collaborator

rbeezer commented Jun 5, 2026

Visually, I think this looks good. A few comments, some about undocumented conventions (sorry!):

  • Generally, we would prefer you rebase your contribution on master privately before making a PR. I think the merge commits are fine, they'll go away when I rebase, but usually we do not want them in the way. Rebase conflicts are pretty rare, the code is quite modular.
  • With one (of few) commits, new work can be added on, I can combine as part of merging. A force-push is usually fine. We generally do not have others adding on to a PR.
  • Let's not inject the JS this way. For one thing, it does not read so well, with CDATA, etc. But more important, we have never used this device. It should be in its own file that gets read in via a script tag. Location is good, and the before-tokenize should be kept.
  • We could have a boolean variable like $b-has-gdscript that is a stricter test than $b-has-program and only load the JS as needed.
  • Let's add a code comment before we use Python for LaTeX listings - just so there is no confusion in the future.
  • Timing? Should I wait for an all-clear on Runestone work, or should this go in as soon as it is ready?

Holler if any of this needs more explanation. With changes, I can do a more thorough review.

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.

2 participants