Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Phix #3728

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Phix #3728

merged 5 commits into from
Apr 15, 2024

Conversation

petelomax
Copy link
Contributor

CHANGES and SUPPORTED_LANGUAGES for Phix

#3727

Changes

Minimal

Checklist

  • Added markup tests, or they don't apply here because...
  • Updated the changelog at CHANGES.md

@joshgoebel
Copy link
Member

Broken link? Do you have the repo elsewhere and were wanting to move it here?

@joshgoebel joshgoebel added the awaiting changes Awaiting changes by PR author label Mar 19, 2023
@petelomax
Copy link
Contributor Author

Yes, my copy is at https://github.com/petelomax/highlightjs-phix

@joshgoebel
Copy link
Member

https://github.com/highlightjs/highlightjs-phix/

Repo created, sorry for losing track of this one.

@joshgoebel
Copy link
Member

Could you please add a dist folder with the CDN build artifacts?

@petelomax
Copy link
Contributor Author

I could probably add an empty dist folder, but I have absolutely no idea what "CDN build artifacts" are.

@joshgoebel
Copy link
Member

but I have absolutely no idea what "CDN build artifacts" are.

The files that build cdn drops into your repo if you're using the blessed development structure. The 3rd party developer document should explain how all that works.

https://github.com/highlightjs/highlight.js/blob/main/extra/3RD_PARTY_QUICK_START.md

@robloxiandemo
Copy link
Contributor

I could probably add an empty dist folder, but I have absolutely no idea what "CDN build artifacts" are.

I already created a PR for you found here, which has already sorted this for you, so feel free to merge it anytime.

@petelomax
Copy link
Contributor Author

What's happening on this?

Add C3, see if that helps
@joshgoebel
Copy link
Member

The issue is still:

#3728 (comment)

You need to use our build process to create the distributables (as well as scan for security issues)... it's all documented in the docs I link to. The build system creates the dist folder, then you add it to your github repo.

@petelomax
Copy link
Contributor Author

node ./tools/build.js -t node
Starting build.
Finished build.
(node:5646) UnhandledPromiseRejectionWarning: /home/pete/highlight.js/node_modules/rollup/dist/shared/rollup.js:58
textEncoder ??= new TextEncoder();
^

SyntaxError: Unexpected token '?'
at wrapSafe (internal/modules/cjs/loader.js:915:16)
at Module._compile (internal/modules/cjs/loader.js:963:27)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
at Module.load (internal/modules/cjs/loader.js:863:32)
at Function.Module._load (internal/modules/cjs/loader.js:708:14)
at Module.require (internal/modules/cjs/loader.js:887:19)
at require (internal/modules/cjs/helpers.js:74:18)
at Module. (/home/pete/highlight.js/node_modules/rollup/dist/rollup.js:14:16)
at Module._compile (internal/modules/cjs/loader.js:999:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
(node:5646) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:5646) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@joshgoebel
Copy link
Member

joshgoebel commented Apr 14, 2024

Are you perhaps using some very old version of Node.js?

@petelomax
Copy link
Contributor Author

petelomax commented Apr 14, 2024

node -v said v12.22.9 (despite being ionstalled today) so after quite some struggle I've updated that to 20.12.2, and yay, it works!

@petelomax
Copy link
Contributor Author

petelomax commented Apr 14, 2024

There is now a dist folder in https://github.com/petelomax/highlightjs-phix

CHANGES.md Outdated
@@ -3,6 +3,8 @@
New Grammars:

- added 3rd party Lang grammar to SUPPORTED_LANGUAGES [AdamRaichu][]
- added 3rd party Phix grammar to SUPPORTED_LANGUAGES [PeteLomax][]
- added 3rd party C3 grammar to SUPPORTED_LANGUAGES [aliaegik][]
Copy link
Member

Choose a reason for hiding this comment

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

Is this unintended?

Copy link
Contributor Author

@petelomax petelomax Apr 15, 2024

Choose a reason for hiding this comment

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

Not intended. Github Desktop insists I did that 13 days ago but it's far more likely something I tried 13 months ago.

@axtens
Copy link

axtens commented Apr 15, 2024

So does that mean that euphoria's config.json can be changed to

  "online_editor": {
    "indent_style": "space",
    "indent_size": 2,
    "highlightjs_language": "phix"
  },

?

CHANGES.md Outdated Show resolved Hide resolved
Copy link

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

3 files changed

Total change +4 B

View Changes
file base pr diff
es/core.min.js 8.2 KB 8.2 KB +1 B
es/highlight.min.js 8.2 KB 8.2 KB +1 B
highlight.min.js 8.23 KB 8.23 KB +2 B

@joshgoebel joshgoebel merged commit 583cb33 into highlightjs:main Apr 15, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes Awaiting changes by PR author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants