facebook / docusaurus

Easy to maintain open source documentation websites.
https://docusaurus.io
MIT License
56.68k stars 8.52k forks source link

Additional prism language (shell-session) is not colored. #3559

Closed Martinsos closed 3 years ago

Martinsos commented 4 years ago

🐛 Bug Report

As per Docusaurus docs, I added

prism: {
  additionalLanguages: ['shell-session']
}

under themeConfig in docusaurus.config.js.

I can see that the block of code where I use shell-session is being correctly tokenized, by inspecting it in the browser via "inspect element". However, no colors are applied! I see that in docusaurus, again via inspect element, other language blocks have colors applied directly via inline style tag on the elements of the code that should be colored. This is not happening for code I marked with shell-session. On the other hand, I see on PrismaJs website that in their example, shell-session code is colored by CSS coming from prisma.css, while that is not happening in Docusaurus.

So, it seems to me that shell-session is mostly working due to tokenization working, but for some reason nor inline styling nor styling via prisma.css is happening.

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

  1. Add prism: { additionalLanguages: ['shell-session'] } under themeConfig in docusaurus.config.js.
  2. Create block of code in a markdown doc file, and mark its language as shell-session:
$ ls
some output

Expected behavior

I expected code to be colored, as in example at PrismJs webpage, where $ was yellow and command was red.

Actual Behavior

Code was not colored, it is just grey, as if no known language was specified. But I can see tokenization happening.

Your Environment

Reproducible Demo

Link to file containing the problematic code block: https://github.com/wasp-lang/web/blob/tutorial/docs/tutorials/todo-app.md . As seen from the link, repo is https://github.com/wasp-lang/web, and branch is tutorial.

Repo is docusaurus, so just do npm install and npm start and then in the Sidebar, go to Tutorials > Todo App and you will see the code blocks.

slorber commented 4 years ago

Hi,

I've debugged this issue.

image

The tokens are in the DOM, you should add the shell-session flag to all your codeblocks, many don't have it.

It's not a highlighter problem, it's just that Docusaurus default syntax highlighting themes don't have anything to highlight properly the shell-session tokens.

Replacing or importing some additional prism theme classes do solve the problem.

import 'prismjs/themes/prism-tomorrow.css'
$ cd TodoApp
$ wasp start
$ cd ..

image

If you don't know where to load such theme, you can try the customCSS

image

Martinsos commented 4 years ago

@slorber thank you for quick response!

Yes, as I mentioned, tokens are in the DOM, but no style was being applied to them. I had shell-session only on one code block because I wanted to compare the difference between one working and one just being plain once I get it working.

Great, your suggestion solved the problem, thanks! I wonder though, how is Docusaurus user supposed to figure this out on their own? Nothing is mentioned about possibly missing CSS when explaining how to add additional languages, and figuring out what to add and where (and that it even should be added) seems to require pretty good understanding of how both docusaurus and prismjs work. Maybe it should be mentioned in the docs, next to the part where addition of additional prism languages is explained?

slorber commented 4 years ago

@Martinsos great to know your problem is solved.

how is Docusaurus user supposed to figure this out on their own?

we are alpha, it's open source, our doc is not perfect and we rely on other open-source projects whose doc is not perfect either .

I had no idea that default prism themes had missing classes for specific languages, I don't know what to do about it right now, and have other more important things to fix and missing docs to write.

If you do not submit a doc PR with possible enhancements about what you have learned, you are also part of the problem 😅 and the reason others will stumble and complain in the future for this missing doc. You can be part of the solution instead, contribute code, doc or money to open-source projects you rely on.

Martinsos commented 4 years ago

@Martinsos great to know your problem is solved.

how is Docusaurus user supposed to figure this out on their own?

we are alpha, it's open source, our doc is not perfect and we rely on other open-source projects whose doc is not perfect either .

I had no idea that default prism themes had missing classes for specific languages, I don't know what to do about it right now, and have other more important things to fix and missing docs to write.

If you do not submit a doc PR with possible enhancements about what you have learned, you are also part of the problem sweat_smile and the reason others will stumble and complain in the future for this missing doc. You can be part of the solution instead, contribute code, doc or money to open-source projects you rely on.

Hi @slorber, I am maintaining an open source project myself, and I often try to contribute to other projects when I encounter an issue like this, so I very much understand what you are saying -> when I asked how is user supposed to figure this out on their own, I did it because I was interested if I am missing something or if this information is genuinely missing. I did not ask the question in order to nag or to complain, although I get a feeling from your last comment that this is how you perceived it, and I think it would be a wasted opportunity if the conversation continued in that tone. In my l last sentence I asked if it should be mentioned in the docs because I thought I could add it via PR if you say that makes sense.

Since you said even you don't know about this, and this information is actually missing, I will look into creating a PR for this, to add something to documentation.

slorber commented 4 years ago

Sorry for misinterpreting as a complaint then.

I don't know if this should be in the doc or not. We have setup prism, loaded a theme. I think user should rather look at prism doc. We can't duplicate doc of prism, MDX webpack etc in docusaurus doc without cluttering our own doc.

Maybe we could just mention that the default themes may not include highlighting classes for all languages or something. Honestly I don't even know if this is the problem, maybe we did something wrong, or maybe it's a React-prism-renderer issue

If you want to submit a doc PR please do, we rarely reject them

Martinsos commented 4 years ago

Sorry for misinterpreting as a complaint then.

I don't know if this should be in the doc or not. We have setup prism, loaded a theme. I think user should rather look at prism doc. We can't duplicate doc of prism, MDX webpack etc in docusaurus doc without cluttering our own doc.

Maybe we could just mention that the default themes may not include highlighting classes for all languages or something. Honestly I don't even know if this is the problem, maybe we did something wrong, or maybe it's a React-prism-renderer issue

If you want to submit a doc PR please do, we rarely reject them

Got it -> I created PR, it is very simple, so let's see, if that makes sense great, if not maybe it will at least start discussion about the correct solution for this. PR: https://github.com/facebook/docusaurus/pull/3563 . Thanks!

felipecrs commented 3 years ago

Hi, just following up here as I faced this issue today. I don't understand why it got closed.

It is still valid, and it was not addressed in any way: nor through a fix in docusaurus, or through a documentation change as proposed by https://github.com/facebook/docusaurus/pull/3563.

The official docs says:

To add syntax highlighting for any of the other Prism supported languages, define it in an array of additional languages.

That's not true, as this issue states, doing only this is not going to work. More and more people will face the same challenge.

So, I suggest to reopen the issue, until it's properly fixed or documented.

slorber commented 3 years ago

@felipecrs so let's re-open the issue then. However, I don't plan to work on this myself so if you know how to fix it or document it properly, please submit a PR

felipecrs commented 3 years ago

I think I discovered the root cause of the issue:

Palenight:

image

GitHub:

image

So as it seems, the themes (https://github.com/FormidableLabs/prism-react-renderer) are not complete enough. I've opened https://github.com/FormidableLabs/prism-react-renderer/issues/121 to track.

For now, as workaround, I would suggest using Palenight as theme.

Josh-Cena commented 3 years ago

Closing because this is related to upstream deps