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

@sebastiaanspeck sebastiaanspeck commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@netlify

netlify Bot commented Apr 13, 2026

Copy link
Copy Markdown

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

github-actions Bot commented Apr 13, 2026

Copy link
Copy Markdown

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

sebastiaanspeck commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

@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

@DmitrySharabin DmitrySharabin left a comment

Copy link
Copy Markdown
Member

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>

@DmitrySharabin DmitrySharabin left a comment

Copy link
Copy Markdown
Member

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