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

Refactor shikiji syntax highlighting code #9083

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Nov 13, 2023

Changes

We used to have 3 places that handle shiki/ji syntax highlighting:

  1. Code.astro
  2. remark-shiki (markdown and MDX shared)
  3. Markdoc

This PR combines all into a single API - createShikiHighlighter. You can now do something like this instead:

const highlighter = await createShikiHighlighter({ /* options object */ })
const html = highlighter.highlight(code, 'js', { /* optional */ inline: true })

I also removed caching for the remark plugin and markdoc plugin for shiki. I don't think they're as useful as these plugins are usually only inited once and reused. Only Code.astro needs caching as the user can pass all kinds of configuration.

Testing

Existing tests should all pass. And added new test for createShikiHighlighter.

I've also manually tests them, including the error overlay and markdoc, that they work.

Docs

n/a. internal refactoring and shouldn't be visible to users

Copy link

changeset-bot bot commented Nov 13, 2023

🦋 Changeset detected

Latest commit: 3b0d7d0

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added feat: markdown Related to Markdown (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Nov 13, 2023
Comment on lines 94 to 99
const highlighter = await getCachedHighlighter({
langs: [lang],
themes: Object.values(experimentalThemes).length ? Object.values(experimentalThemes) : [theme],
theme,
experimentalThemes,
wrap,
});
Copy link
Member Author

Choose a reason for hiding this comment

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

With the abstracted function, we can pass the individual options directly, and it'll handle and process them internally.

Copy link
Member Author

Choose a reason for hiding this comment

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

Everything is moved to this file. Code is mostly taken from Code.astro which uses the transforms option to replace certain properties. Compared to other implementations that used regexes.

Comment on lines +78 to +101
},
line(node) {
// Add "user-select: none;" for "+"/"-" diff symbols.
// Transform `<span class="line"><span style="...">+ something</span></span>
// into `<span class="line"><span style="..."><span style="user-select: none;">+</span> something</span></span>`
if (lang === 'diff') {
const innerSpanNode = node.children[0];
const innerSpanTextNode =
innerSpanNode?.type === 'element' && innerSpanNode.children?.[0];

if (innerSpanTextNode && innerSpanTextNode.type === 'text') {
const start = innerSpanTextNode.value[0];
if (start === '+' || start === '-') {
innerSpanTextNode.value = innerSpanTextNode.value.slice(1);
innerSpanNode.children.unshift({
type: 'element',
tagName: 'span',
properties: { style: 'user-select: none;' },
children: [{ type: 'text', value: start }],
});
}
}
}
},
Copy link
Member Author

Choose a reason for hiding this comment

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

Most of the code in this file is copied, except this part which is new. I ported it from the regex version.

// Add "user-select: none;" for "+"/"-" diff symbols
if (node.lang === 'diff') {
html = html.replace(
/<span class="line"><span style="(.*?)">([\+|\-])/g,
'<span class="line"><span style="$1"><span style="user-select: none;">$2</span>'
);
}

It's slightly more code now, but with access to the hast, this should be a liitle more performant and robust.

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@bluwy bluwy merged commit 4537ecf into main Nov 14, 2023
13 checks passed
@bluwy bluwy deleted the syntax-highlight-refactor branch November 14, 2023 15:00
@astrobot-houston astrobot-houston mentioned this pull request Nov 14, 2023
peng added a commit to peng/astro that referenced this pull request Nov 17, 2023
* main:
  feat(i18n): add `Astro.currentLocale` (withastro#9101)
  [ci] release (withastro#9107)
  Add compatibility with cloudflare node (withastro#8925)
  [ci] format
  Cancel response stream when connection closes (withastro#9071)
  [ci] format
  feat(i18n): apply specific routing logic only to pages (withastro#9091)
  feat(dev-overlay): Hide plugins into a separate menu when there's too many enabled (withastro#9102)
  [ci] format
  Support Svelte 5 (experimental) (withastro#9098)
  [ci] release (withastro#9078)
  [ci] format
  Refactor shikiji syntax highlighting code (withastro#9083)
  [ci] format
  fix: Query params trigger the trailingSlash error in preview mode (withastro#9045)
  fix(assets): bundling regression for specific config on non-Node runtimes (withastro#9087)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: markdown Related to Markdown (scope) pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants