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

v15.1.0.md: fix wrong syntax highlighting #3695

Merged
merged 3 commits into from
Feb 14, 2021
Merged

v15.1.0.md: fix wrong syntax highlighting #3695

merged 3 commits into from
Feb 14, 2021

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Feb 14, 2021

So, this is a tricky one.

  1. the build errors aren't real errors; the build continued and resulted in wrong highlighting, see for example https://github.com/nodejs/nodejs.org/runs/1896479847#step:6:31. AFAICT this is by design in the metalsmith-prism package: https://github.com/Availity/metalsmith-prism/blob/8db23a433d600b3fd13e33779cbd264e053d7caf/lib/index.js#L41-L47
  2. maybe this should be implemented in the changelog maker or something?

Before:

image

After:

image

Any of these should do the job: shell-session, sh-session, shellsession

@targos
Copy link
Member

targos commented Feb 14, 2021

@XhmikosR
Copy link
Contributor Author

Is the rule to always use shell-session instead of console ?

Any of shell-session, sh-session, shellsession will work with prismjs, but yeah, this isn't what GitHub's syntax highlighting works with.

Hence why I think there should be some kind of enforcement for this specific case which we have hit more than once (#2923)

@XhmikosR
Copy link
Contributor Author

The other options would be

  1. ask upstream in prismjs to add an alias for this
  2. see if we can do this manually
  3. switch to another solution altogether

@targos
Copy link
Member

targos commented Feb 14, 2021

Maybe we can do something in the release post script to transform occurences of ```console to ```shell-session ?

@XhmikosR
Copy link
Contributor Author

Yup, that could work, but I'm not sure I have the time to propose a patch :)

That being said, a fix there along with a test case should be a pretty good fix.

@XhmikosR XhmikosR merged commit 9e9b463 into master Feb 14, 2021
@XhmikosR XhmikosR deleted the XhmikosR-patch-3 branch February 14, 2021 18:16
Wai-Dung added a commit that referenced this pull request Feb 23, 2021
"Metal-Prism" needs a range of coding languages to be rendered with,
however the script will generate some languages that don't match what it
really needs. This is a fixture for it.

For more, please see: #3695.
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.

None yet

4 participants