Skip to content

Apply fixes for Magik language#4076

Merged
DmitrySharabin merged 17 commits into
PrismJS:v2from
sebastiaanspeck:fix/magik
Apr 21, 2026
Merged

Apply fixes for Magik language#4076
DmitrySharabin merged 17 commits into
PrismJS:v2from
sebastiaanspeck:fix/magik

Conversation

@sebastiaanspeck
Copy link
Copy Markdown
Contributor

@sebastiaanspeck sebastiaanspeck commented Apr 13, 2026

No description provided.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 13, 2026

Deploy Preview for dev-prismjs-com ready!

Name Link
🔨 Latest commit 0c9ed13
🔍 Latest deploy log https://app.netlify.com/projects/dev-prismjs-com/deploys/69e7c4c4f7e2f600085ff64e
😎 Deploy Preview https://deploy-preview-4076--dev-prismjs-com.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 13, 2026

No JS Changes

Generated by 🚫 dangerJS against 0c9ed13

@sebastiaanspeck
Copy link
Copy Markdown
Contributor Author

#4075 and #4073 are included in this PR (and extended with more fixes)

@sebastiaanspeck
Copy link
Copy Markdown
Contributor Author

sebastiaanspeck commented Apr 14, 2026

@DmitrySharabin as requested, a new PR which contains all found fixes.

Comment thread src/languages/magik.js Outdated
Comment thread src/languages/magik.js
Comment thread src/languages/magik.js Outdated
Comment thread src/languages/magik.js Outdated
Comment thread src/languages/magik.js Outdated
Comment thread src/languages/magik.js Outdated
Copy link
Copy Markdown
Member

@DmitrySharabin DmitrySharabin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments/questions. Please, take a look. I'd also suggest adding a few tests to catch the mentioned cases if they make sense.

Comment thread src/languages/magik.js Outdated
Co-authored-by: Sebastiaan Speck <12570668+sebastiaanspeck@users.noreply.github.com>
@sebastiaanspeck
Copy link
Copy Markdown
Contributor Author

@DmitrySharabin I resolved your comments. Can you have a look if it looks good to you?

Comment thread src/languages/magik.js Outdated
sebastiaanspeck and others added 2 commits April 21, 2026 15:47
Co-authored-by: Dmitry Sharabin <dmitrysharabin@gmail.com>
Copy link
Copy Markdown
Member

@DmitrySharabin DmitrySharabin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Nit: Could you run Prettier, please, on the updated files? It seems like it should fix some formatting issues.

@sebastiaanspeck
Copy link
Copy Markdown
Contributor Author

Nit: Could you run Prettier, please, on the updated files? It seems like it should fix some formatting issues.

You mean on the test files?

➜  prism git:(fix/magik) npx prettier tests/languages/magik/*
tests/languages/magik/boolean_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/boolean_feature.test".
tests/languages/magik/builtin_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/builtin_feature.test".
tests/languages/magik/char_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/char_feature.test".
tests/languages/magik/comment_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/comment_feature.test".
tests/languages/magik/constant_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/constant_feature.test".
tests/languages/magik/declaration_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/declaration_feature.test".
tests/languages/magik/dynamic-variable_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/dynamic-variable_feature.test".
tests/languages/magik/function_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/function_feature.test".
tests/languages/magik/global-reference_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/global-reference_feature.test".
tests/languages/magik/global-variable_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/global-variable_feature.test".
tests/languages/magik/keyword-operator_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/keyword-operator_feature.test".
tests/languages/magik/keyword-variable_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/keyword-variable_feature.test".
tests/languages/magik/keyword_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/keyword_feature.test".
tests/languages/magik/number_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/number_feature.test".
tests/languages/magik/operator_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/operator_feature.test".
tests/languages/magik/pragma_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/pragma_feature.test".
tests/languages/magik/punctuation_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/punctuation_feature.test".
tests/languages/magik/regex_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/regex_feature.test".
tests/languages/magik/self_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/self_feature.test".
tests/languages/magik/slot_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/slot_feature.test".
tests/languages/magik/string_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/string_feature.test".
tests/languages/magik/symbol_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/symbol_feature.test".
tests/languages/magik/unset_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/unset_feature.test".
tests/languages/magik/variable_feature.test
[error] No parser could be inferred for file "/Users/sebastiaan.speck/src/prism/tests/languages/magik/variable_feature.test".

@DmitrySharabin
Copy link
Copy Markdown
Member

Yeah, I think it won't work on the test files. Let's just run Prettier on magik.js. It should be enough.

@sebastiaanspeck
Copy link
Copy Markdown
Contributor Author

Yeah, I think it won't work on the test files. Let's just run Prettier on magik.js. It should be enough.

Which issues does it solve then? I ran it, but IMO the output was less readable than what we have now.

@DmitrySharabin
Copy link
Copy Markdown
Member

Yeah, I think it won't work on the test files. Let's just run Prettier on magik.js. It should be enough.

Which issues does it solve then? I ran it, but IMO the output was less readable than what we have now.

One is that if anyone ever wants to change the file, save the changes, and Prettier kicks in automatically (as it should according to the recommended IDE setup), they should undo the changes made by Prettier or commit/submit unrelated changes.
For example, if we introduce a new feature in Prism and we need the languages to adopt it, we tweak the language definitions. And before we added Prettier, we had many unrelated changes in the corresponding commits (that was a pain; trust me, once, I had to update all the languages Prism supports 😅). We try to avoid it. And yes, Prettier rules are opinionated, but we are fine with them in this project.

I should've asked you to do that in the first PR, but I somehow missed it.

@sebastiaanspeck
Copy link
Copy Markdown
Contributor Author

Yeah, I think it won't work on the test files. Let's just run Prettier on magik.js. It should be enough.

Which issues does it solve then? I ran it, but IMO the output was less readable than what we have now.

One is that if anyone ever wants to change the file, save the changes, and Prettier kicks in automatically (as it should according to the recommended IDE setup), they should undo the changes made by Prettier or commit/submit unrelated changes.

For example, if we introduce a new feature in Prism and we need the languages to adopt it, we tweak the language definitions. And before we added Prettier, we had many unrelated changes in the corresponding commits (that was a pain; trust me, once, I had to update all the languages Prism supports 😅). We try to avoid it. And yes, Prettier rules are opinionated, but we are fine with them in this project.

I should've asked you to do that in the first PR, but I somehow missed it.

I'll run it then and put aside my own opinion.

@DmitrySharabin DmitrySharabin merged commit ded4a65 into PrismJS:v2 Apr 21, 2026
13 of 14 checks passed
@DmitrySharabin
Copy link
Copy Markdown
Member

Merged! Thank you.

@sebastiaanspeck
Copy link
Copy Markdown
Contributor Author

@DmitrySharabin I backported the changes to a branch for v1 so we can use Magik already in several plugins, but with the current grammar, the lint breaks, while in v2 it doesn't.

https://github.com/sebastiaanspeck/prism/actions/runs/24758384791

@DmitrySharabin
Copy link
Copy Markdown
Member

@DmitrySharabin I backported the changes to a branch for v1 so we can use Magik already in several plugins, but with the current grammar, the lint breaks, while in v2 it doesn't.

https://github.com/sebastiaanspeck/prism/actions/runs/24758384791

The root cause is the lookbehind assertion ((?<!\|)) in the number token. This is an ES2018 feature. In Prism v1, we use ESLint v7. To make it support the lookbehind assertions in regexes, you can add

"parserOptions": {
  "ecmaVersion": 2018
}

to .eslintrc.js (I suppose, before the rules property should work).

@sebastiaanspeck sebastiaanspeck deleted the fix/magik branch April 22, 2026 14:11
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