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

feat(syntax-highlight): add additional languages #6831

Merged
merged 10 commits into from
Aug 19, 2022
Merged

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Aug 8, 2022

This PR adds a number of additional languages for syntax highlighting. This will allow our React tutorials to be formatted properly (since there are some fundamental differences between it and JavaScript that will cause highlighting errors). Since syntax highlighting is performed at build time, adding more languages to syntax highlighting will not cause any additional loading times or load additional resources on the reader's end.

Additionally, this points both sh and shell to bash, and py to python. A comment is also added to clarify which languages are already loaded to reduce confusion if this function is used as a reference for what syntax languages are available.

@github-actions github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Aug 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2022

This pull request has merge conflicts that must be resolved before it can be merged.

@queengooborg queengooborg changed the title Add JSX as syntax language; update syntax aliases Add additional syntax languages for highlighting Aug 8, 2022
@github-actions github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Aug 8, 2022
@hamishwillee
Copy link
Contributor

The pointers from py > python are good. IMO whatever github uses we should mirror.

So do we actually have cases for all these types or are they speculative? I.e. ini files don't really have a particular format do they? When is "ignore" used? diff would be great in some cases but is it a standardized format? I can imagine using this to show when you make changes in a file.

As @wbamberg suggests here https://github.com/orgs/mdn/discussions/170#discussioncomment-3352639 we need to make sure that the supported set is documented.

Who can review/merge this nowdays?

@queengooborg
Copy link
Collaborator Author

The pointers from py > python are good. IMO whatever github uses we should mirror.

I'd be happy to add the alias back in! I removed it because both py and python are interchangeable with Prism, so we don't need to map it ourselves.

So do we actually have cases for all these types or are they speculative?

A few of these are speculative (such as SQL, URI, SCSS and Less). I basically skimmed through the Prism supported list to find web-related and Git-related languages, and added them in.

ini files don't really have a particular format do they?

They do, actually! Though this one's a speculative add, so I'm happy to take it out!

When is "ignore" used?

Ignore includes .gitignore, which we have included sample content for a few times.

diff would be great in some cases but is it a standardized format?

It is a standard format, yes!

@hamishwillee
Copy link
Contributor

The pointers from py > python are good. IMO whatever github uses we should mirror.

I'd be happy to add the alias back in! I removed it because both py and python are interchangeable with Prism, so we don't need to map it ourselves.

If we don't need to map it ourselves then it doesn't need to be added back in. But the docs need to make clear what is supported.

So do we actually have cases for all these types or are they speculative?

A few of these are speculative (such as SQL, URI, SCSS and Less). I basically skimmed through the Prism supported list to find web-related and Git-related languages, and added them in.

The thing that kicked this off was nginx and apache, and I'd been hoping for "pug". My original thinking was add everything, but since Will has spoken against this, we really need to agree the policy before we put this in. I'll comment the set on the issue and lets see if we can get broad agreement.

@Josh-Cena
Copy link
Member

FYI, prism-react-renderer has its own list of default-included language list, and it seems to be aiming for web-related ones.

@hamishwillee
Copy link
Contributor

hamishwillee commented Aug 16, 2022

@queengooborg So I think we have settled about what languages can be added, and under what conditions: https://github.com/orgs/mdn/discussions/170#discussioncomment-3403112

Do these rules make sense for you?

If so,

  • the alias list will need to contain just one identifier for the languages that we support (even if Prism supports them natively),
  • web idl needs to be removed or even better COMMENTED out with a note "block list"
  • are you willing to add these to the docs?

build/syntax-highlight.js Outdated Show resolved Hide resolved
build/syntax-highlight.js Outdated Show resolved Hide resolved
@caugner caugner changed the title Add additional syntax languages for highlighting feat(syntax-highlight): add additional languages Aug 18, 2022
queengooborg and others added 2 commits August 19, 2022 04:29
@caugner
Copy link
Contributor

caugner commented Aug 19, 2022

@queengooborg Ready for me to merge?

@queengooborg
Copy link
Collaborator Author

Yep, go for it! If we want to add or remove any syntax languages, we can do so down the line. 👍

@caugner caugner merged commit def733d into mdn:main Aug 19, 2022
@hamishwillee
Copy link
Contributor

Thanks very much!!!!

@hamishwillee
Copy link
Contributor

hamishwillee commented Aug 22, 2022

That's awesome, so which one of you is updating https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Howto/Markdown_in_MDN#example_code_blocks in line with https://github.com/orgs/mdn/discussions/170#discussioncomment-3403112 ?

I already added pug and yaml in mdn/content#19806, and wasm was already in the list.

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.

5 participants