Skip to content

fix: use rehype-shikiji #6054

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 27, 2023
Merged

fix: use rehype-shikiji #6054

merged 5 commits into from
Oct 27, 2023

Conversation

ovflowd
Copy link
Member

@ovflowd ovflowd commented Oct 27, 2023

Description

This PR temporarily moves from rehype-pretty-code to rehype-shikiji, since right now, rehype-pretty-code would need shikiji-compat to work with rehype-pretty-code, and unfortunately, that uses Node.js specific APIs.

The functionality of rehype-shikiji is slightly different; hence let's keep an 👀 once rehype-pretty-code moves to shikiji.

Validation

All syntax highlighting should still work. Keep in mind empty lines will have a different effect for the meantime.

@ovflowd ovflowd requested a review from a team as a code owner October 27, 2023 08:39
@vercel
Copy link

vercel bot commented Oct 27, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 27, 2023 9:17am

@ovflowd ovflowd added fast-track Fast Tracking PRs infrastructure Issues/PRs related to the Repository Infra labels Oct 27, 2023
@ovflowd ovflowd requested a review from a team as a code owner October 27, 2023 08:52
@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Oct 27, 2023
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Oct 27, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 27, 2023 •
edited
Loading

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 94%
 95% (304/320) 76.38% (55/72) 93.84% (61/65)

Unit Test Report

Tests Skipped Failures Errors Time
38 0 💤 0 ❌ 0 🔥 6.728s ⏱️

@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Oct 27, 2023
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Oct 27, 2023
@ovflowd ovflowd added the github_actions:pull-request Trigger Pull Request Checks label Oct 27, 2023
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Oct 27, 2023
@canerakdas
Copy link
Member

Hey @ovflowd,
Empty lines look like deleted in the preview. Is this an expected situation? I read the validation but didn't get whether this effect is expected or not 🤔

Vercel preview

nodejs.org

Markdown file

@ovflowd
Copy link
Member Author

ovflowd commented Oct 27, 2023

Hey @ovflowd,

Empty lines look like deleted in the preview. Is this an expected situation? I read the validation but didn't get whether this effect is expected or not 🤔

Vercel preview

nodejs.org

Markdown file

Right, it is expected. Sorta. We can always make a css fix on a follow up PRs. The empty lines are still there, but just have no styling.

I have the feeling thats what rehype-pretty-code was doing + a bunch of other things like the line numbers.

So either we abandon rehype-pretty-code and implement these things by ourselves or eventually we move back to that library once they moved to shikiji

@ovflowd
Copy link
Member Author

ovflowd commented Oct 27, 2023 •
edited
Loading

Also @canerakdas feel free to tackle the style empty line issue if you want :) (at least as a hot fix on the legacy css)

canerakdas
canerakdas approved these changes Oct 27, 2023
View reviewed changes
Copy link
Member

@canerakdas canerakdas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ovflowd
Copy link
Member Author

ovflowd commented Oct 27, 2023

Also @devjvao as this might be interesting for your Codebox PR, i.e., line numbers and empty line spacing/padding

@ovflowd ovflowd added this pull request to the merge queue Oct 27, 2023
Merged via the queue into main with commit b66971e Oct 27, 2023
@ovflowd ovflowd deleted the fix/use-rehype-shiki branch October 27, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@canerakdas canerakdas canerakdas approved these changes

Assignees

No one assigned

Labels

fast-track Fast Tracking PRs infrastructure Issues/PRs related to the Repository Infra

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ovflowd @canerakdas