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

feat: add handling for FontStyle.Strikethrough #976

Merged
merged 4 commits into from
Apr 10, 2025

Conversation

dhruvkb
Copy link
Contributor

@dhruvkb dhruvkb commented Mar 25, 2025

Description

This PR adds support for handling the FontStyle.Strikethrough in a few places where it was not present. The primary motivation was to support ANSI strikethrough which is supported by the underlying ansi-sequence-parser package but not by Shiki.

Linked Issues

Fixes #975 by myself

Additional context

I would appreciate any help in pointing out the new test cases which should be added to cover the changed code (and where to add them).

Copy link

netlify bot commented Mar 25, 2025

Deploy Preview for shiki-matsu ready!

Name Link
🔨 Latest commit 1a6bb6f
🔍 Latest deploy log https://app.netlify.com/sites/shiki-matsu/deploys/67e25e8561bebf0008651eb7
😎 Deploy Preview https://deploy-preview-976--shiki-matsu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Mar 25, 2025

Deploy Preview for shiki-next ready!

Name Link
🔨 Latest commit 1a6bb6f
🔍 Latest deploy log https://app.netlify.com/sites/shiki-next/deploys/67e25e857b6900000838d0ab
😎 Deploy Preview https://deploy-preview-976--shiki-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@antfu
Copy link
Member

antfu commented Mar 25, 2025

Oh nice catch! LGTM. Anything left for it to be ready?

Copy link
Contributor Author

@dhruvkb dhruvkb left a comment

Choose a reason for hiding this comment

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

It was actually pointed out to me by @hippotastic in expressive-code/expressive-code#316.


.vp-code,
.vp-code span {
/* background-color: var(--shiki-bg, inherit); */
Copy link
Contributor Author

@dhruvkb dhruvkb Mar 25, 2025

Choose a reason for hiding this comment

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

When this is enabled, ANSI background colors become visible but this breaks all other code examples in the docs.

That's why I've set this disabled currently, but then ANSI background colors don't render at all (and consequently the reversed style).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To work around this, had to write a bunch of inelegant CSS in 1a6bb6f. @antfu if you have any advice on what can be done about that, please do share!

@dhruvkb dhruvkb marked this pull request as ready for review March 25, 2025 07:53
@antfu antfu changed the title Add handling for FontStyle.Strikethrough featdd handling for FontStyle.Strikethrough Apr 10, 2025
@antfu antfu changed the title featdd handling for FontStyle.Strikethrough feat: add handling for FontStyle.Strikethrough Apr 10, 2025
@antfu antfu merged commit f14f239 into shikijs:main Apr 10, 2025
12 checks passed
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.

Add support for strikethrough in ANSI
2 participants