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 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

Loading

@johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Aug 5, 2021

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

Loading

@johnnyreilly
Copy link
Contributor

@johnnyreilly 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

Loading

@RunDevelopment
Copy link
Member Author

@RunDevelopment RunDevelopment commented Aug 5, 2021

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.

Loading

@johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Aug 5, 2021

Great work!

Loading

@RunDevelopment RunDevelopment merged commit 748bb9a into PrismJS:master Aug 5, 2021
11 checks passed
Loading
@RunDevelopment RunDevelopment deleted the bicep-impr branch Aug 5, 2021
@johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Aug 11, 2021

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!

Loading

@RunDevelopment
Copy link
Member Author

@RunDevelopment RunDevelopment commented Aug 11, 2021

@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.

Loading

@johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Aug 11, 2021

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!

Loading

@johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Aug 15, 2021

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.

Loading

@RunDevelopment
Copy link
Member Author

@RunDevelopment RunDevelopment commented Aug 15, 2021

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.

Loading

@johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Aug 15, 2021

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"
  },

Loading

@johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Aug 20, 2021

Loading

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

Successfully merging this pull request may close these issues.

None yet

2 participants