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

Look at supporting multiple syntax highlighters #438

Closed
JoelMarcey opened this issue Feb 8, 2018 · 13 comments
Closed

Look at supporting multiple syntax highlighters #438

JoelMarcey opened this issue Feb 8, 2018 · 13 comments
Assignees
Labels
feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. status: claimed Issue has been claimed by a contributor who plans to work on it.

Comments

@JoelMarcey
Copy link
Contributor

Is this a bug report?

No

Have you read the Contributing Guidelines?

Yes

Environment

N/A

Steps to Reproduce

https://twitter.com/notbrent/status/961436860928049152

Expected Behavior

JSX is supported

Actual Behavior

JSX is not supported

Reproducible Demo

N/A

@JoelMarcey JoelMarcey added the feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. label Feb 8, 2018
@JoelMarcey
Copy link
Contributor Author

cc @brentvatne

@brentvatne
Copy link

closing this related issue: #388

@ahmadalfy
Copy link
Contributor

@JoelMarcey @yangshun should we switch to Prism? I looked at highlight.js and couldn't find support for JSX.

@endiliey endiliey self-assigned this Jun 6, 2018
@endiliey endiliey added the status: claimed Issue has been claimed by a contributor who plans to work on it. label Jun 6, 2018
@endiliey
Copy link
Contributor

endiliey commented Jun 6, 2018

I have investigated this. One thing to note is that unlike highlight.js, prism.js doesn't support auto language detection.

Since highlight.js couldn't support JSX, do we want to:

  1. Support two highlighters - highlight.js and prism and let the user choose as a configuration option.
    or
  2. Still use highlight.js for everything but use prism.js for jsx ??

My mockup for now

Highlight.js (before)
highlightjs

Prism.js (after)
prismjs

@iRoachie
Copy link
Contributor

iRoachie commented Jun 6, 2018

What do you mean by auto-language detection?

You mean when if the language is not explicitly set? e.g ```js

@endiliey
Copy link
Contributor

endiliey commented Jun 6, 2018

@iRoachie yes
PrismJS/prism#1313

@iRoachie
Copy link
Contributor

iRoachie commented Jun 6, 2018

Is this a requirement? The highlighter github uses for markdown doesn't do auto-language detection either.

@endiliey
Copy link
Contributor

endiliey commented Jun 6, 2018

I think it's not required 😄. Anyway, would you prefer to see option 1 or option 2 ?

@iRoachie
Copy link
Contributor

iRoachie commented Jun 6, 2018

Think for now option 2 might be better. Don't think we have tests to check which languages might break using prismjs outside of jsx as compared to highlightjs.

@endiliey
Copy link
Contributor

endiliey commented Jun 6, 2018

Any thoughts @yangshun @JoelMarcey ?

@JoelMarcey
Copy link
Contributor Author

Option 2 might work ok. It's a little odd to have a second highlighter that is only used for one language.

Could we do something like...

Use highlight.js as the default. Then in siteConfig.js have an option called usePrism where we can have an array of languages where we want to use that highlighter? A little more configurable. What do you think?

@endiliey endiliey added the RFC label Jun 7, 2018
@endiliey
Copy link
Contributor

endiliey commented Jun 7, 2018

Something like this @JoelMarcey ?

usePrism: ['js', 'jsx']

Giving some examples:

usePrism: ['js', 'jsx'],
highlight: {
  theme: 'atom-one-dark',
}

atom-one-dark

usePrism: ['js', 'jsx'],
highlight: {
  theme: 'default',
}

default

usePrism: ['js', 'jsx'],
highlight: {
  theme: 'tomorrow-night-blue',
}

tomorrow-night-blue

@JoelMarcey
Copy link
Contributor Author

@endiliey 👍 Yep, something similar to that. I wonder if option 1 is easier or more clean 😆 But I think this should be ok. Let me look at the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. status: claimed Issue has been claimed by a contributor who plans to work on it.
Projects
None yet
Development

No branches or pull requests

5 participants