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

Bicep: Resolved TODO + improvements #3028

Merged
merged 4 commits into from Aug 5, 2021

Conversation

RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Aug 5, 2021

This PR resolves the TODO for interpolated strings and multiline strings.

I also added highlighting for object properties, data types, and arbitrary functions.

image


@johnnyreilly
I never used Bicep. Does this highlighting make sense?

Also, this is a bit late, but what are these function names? Will the more general function detection I used (/\b[a-z_]\w*(?=[ \t]*\()/i) find them as well?

@github-actions
Copy link

github-actions bot commented Aug 5, 2021

JS File Size Changes (gzipped)

A total of 1 files have changed, with a combined diff of +156 B (+35.5%).

file master pull size diff % diff
components/prism-bicep.min.js 439 B 595 B +156 B +35.5%

Generated by 🚫 dangerJS against 18ff75f

@johnnyreilly
Copy link
Contributor

This is brilliant! And yes the highlighting makes sense.

Also, this is a bit late, but what are these function names

These are the array functions for ARM templates

If the regex you mention covers camel case words without spaces (and I think it does) then it could work

@johnnyreilly
Copy link
Contributor

johnnyreilly commented Aug 5, 2021

Question: how do you get to see the highlighting? Is there a way to test locally? I was just basing my work on the test results - I didn't realise there was an alternative!

Also, would it be better to have a different name for the file tests/languages/bicep/class-name_feature.test? The file seems to be testing param and output - Bicep doesn't have classes

@RunDevelopment
Copy link
Member Author

If the regex you mention covers camel case words without spaces (and I think it does)

Yes, it does. I'll remove the ARM template functions then since they are already highlighted by the function regex.

Is there a way to test locally?

Yes, you can use the local version of the test page. Just open test.html in your browser.

Bicep doesn't have classes

True, it has data types. However, we use the token name class-name for all type-like names (classes, interfaces, built-in types, any data type really). Tests are named after the token name, so I named the test file class-name_feature.test.

I'll just name the token datatype and give it a class-name alias.

@johnnyreilly
Copy link
Contributor

Great work!

@RunDevelopment RunDevelopment merged commit 748bb9a into PrismJS:master Aug 5, 2021
@RunDevelopment RunDevelopment deleted the bicep-impr branch August 5, 2021 21:07
@johnnyreilly
Copy link
Contributor

Hey @RunDevelopment ! Do you have an idea when you're likely do the next release of prism? I'm really looking forward to using Bicep - but can't until a new version ships!

@RunDevelopment
Copy link
Member Author

@johnnyreilly In about a month. In the meantime, it is available on our download page and you can configure npm to get prism directly from this repo. Our master branch is always stable.

@johnnyreilly
Copy link
Contributor

Thanks - in my case I'm going to be consuming it via Docusaurus and so it won't be until the new version that I'll be able to consume this.

I look forward to next month!

@johnnyreilly
Copy link
Contributor

Interestingly I did give adding this dependency to my Docusaurus package.json:

    "prismjs": "PrismJS/prism",

and updating my docusaurus.config.js to add bicep in:

    prism: {
      additionalLanguages: ["bicep"],
    },

But when I ran I experienced this issue:

Uncaught Error: Cannot find module './prism-bicep'

Not sure if this is a sign of an issue - or just user error on my behalf. Thought I'd mention it just in case this is significant.

@RunDevelopment
Copy link
Member Author

That should work, I think. It might be some dependency problem. Could you please check the following:

  1. Does "prismjs": "PrismJS/prism" work? Basically does node_modules/prismjs/components/prism-bicep.js exist?
  2. Does Docusaurus use that version of Prism? It might be that Docusaurus has its own version Prism given to it by npm. You can see this in package-lock.json. If yes, then npm dedup might fix it.

@johnnyreilly
Copy link
Contributor

yes - it looks like it's a transitive dependency issue with @docusaurus/theme-classic

I was able to workaround it by adding the following to the package.json:

  "resolutions": {
    "prismjs": "PrismJS/prism"
  },

@johnnyreilly
Copy link
Contributor

I ended up writing this up here: https://blog.johnnyreilly.com/2021/08/19/bicep-syntax-highlighting-with-prismjs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants