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

doc: all <pre> tags are highlighted as JavaScript #33363

Closed
1 task done
DerekNonGeneric opened this issue May 12, 2020 · 4 comments
Closed
1 task done

doc: all <pre> tags are highlighted as JavaScript #33363

DerekNonGeneric opened this issue May 12, 2020 · 4 comments
Labels
doc Issues and PRs related to the documentations.

Comments

@DerekNonGeneric
Copy link
Contributor

DerekNonGeneric commented May 12, 2020

📗 API Reference Docs Problem

  • Version: n/a
  • Platform: n/a
  • Subsystem: n/a

Location

Section of the site where the content exists

Affected URL(s):

Problem description

Concise explanation of what you found to be problematic

While looking into #32938, I discovered that currently, every single <pre> tag gets highlighted as JavaScript regardless of the info string specified (and even if left unspecified). The following line executes the code that does this.

<script>highlight(undefined, undefined, 'pre');</script>

This behavior can be observed in the C++ code blocks in the published API reference documents. The following line is the reason for this — the language has been hard-coded.

sh_highlightElement(element, sh_languages["javascript"]);


  • I would like to work on this issue and submit a pull request.

^ I'm currently on summer vacation, so I would have the time to work on this.

/cc @Trott

@DerekNonGeneric DerekNonGeneric added the doc Issues and PRs related to the documentations. label May 12, 2020
@Trott
Copy link
Member

Trott commented May 12, 2020

Yes, please fix this! 😀

@DerekNonGeneric
Copy link
Contributor Author

@Trott, it is my impression that this would be an excellent opportunity to switch to highlight.js for a few reasons:

  1. SHJS hasn't been updated in 6 years
  2. SHJS is GPL v3 licensed
  3. SHJS doesn't understand modern language syntax
  4. highlight.js is currently well-maintained (and popular)
  5. highlight.js is BSD licensed
  6. highlight.js is able to highlight the modern syntax of several languages
  7. we'd have syntax highlighting on par w/ the Markdown docs rendered on github.com

I think there was a reason why @zeke mentioned it in passing — it's the route to go nowadays. If GitHub's syntax highlighting library (Prettylights) were OSS, we'd ideally use that, but that doesn't seem to be one of our options. I can give it a day or two before proceeding in case others want to shop around and propose a superior frontend syntax highlighting library, but this seems like the best bet as far as I can tell.

/cc @nodejs/documentation

@zeke
Copy link
Contributor

zeke commented May 14, 2020

I think there was a reason why @zeke mentioned it in passing — it's the route to go nowadays

☝️ Yep that is why I mentioned it.

codebytere pushed a commit that referenced this issue Jun 18, 2020
Prior to this commit, all <pre> tags were being
highlighted as JavaScript. This has been corrected
to syntax highlight all languages appearing in the
API reference docs. This was accomplished by using
highlight.js instead of SHJS for the frontend lib.

* remove SHJS JavaScript code
* add highlight.js bundle
* fix script tags to reflect replacement
* migrate CSS to use highlight.js classes
* add appropriate documentation
* ensure api_assets README.md stays interal

Fixes: #33363

PR-URL: #33442
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
codebytere pushed a commit that referenced this issue Jun 18, 2020
Prior to this commit, all <pre> tags were being
highlighted as JavaScript. This has been corrected
to syntax highlight all languages appearing in the
API reference docs. This was accomplished by using
highlight.js instead of SHJS for the frontend lib.

* remove SHJS JavaScript code
* add highlight.js bundle
* fix script tags to reflect replacement
* migrate CSS to use highlight.js classes
* add appropriate documentation
* ensure api_assets README.md stays interal

Fixes: #33363

PR-URL: #33442
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
codebytere pushed a commit that referenced this issue Jun 30, 2020
Prior to this commit, all <pre> tags were being
highlighted as JavaScript. This has been corrected
to syntax highlight all languages appearing in the
API reference docs. This was accomplished by using
highlight.js instead of SHJS for the frontend lib.

* remove SHJS JavaScript code
* add highlight.js bundle
* fix script tags to reflect replacement
* migrate CSS to use highlight.js classes
* add appropriate documentation
* ensure api_assets README.md stays interal

Fixes: #33363

PR-URL: #33442
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
codebytere pushed a commit that referenced this issue Jul 8, 2020
Prior to this commit, all <pre> tags were being
highlighted as JavaScript. This has been corrected
to syntax highlight all languages appearing in the
API reference docs. This was accomplished by using
highlight.js instead of SHJS for the frontend lib.

* remove SHJS JavaScript code
* add highlight.js bundle
* fix script tags to reflect replacement
* migrate CSS to use highlight.js classes
* add appropriate documentation
* ensure api_assets README.md stays interal

Fixes: #33363

PR-URL: #33442
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
@wooorm
Copy link

wooorm commented May 14, 2022

Hi folks! As this discussion mentions ideally using PrettyLights by GH, I wanted to drop a suggestion for a project which I made that does exactly that: https://github.com/wooorm/starry-night. HLJS is much lighter, but if y’all are noticing problems, starry-night might be a good choice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants