Light/dark theming for shikiji's codeblocks #8903

horo-fox · 2023-10-24T01:05:48Z

Changes

This passes through themes from markdown configuration to the markdown usage in the markdown package.

I would mark this as experimental as shikiji's support is "experimental" but I'm not sure how. I don't think normal versioning guarantees should apply.

Testing

I've added a new test case.

Docs

I avoided documentation because this is experimental (according to shikiji) and I don't think Astro wants to look like it's promising any sort of stability.

changeset-bot · 2023-10-24T01:05:52Z

🦋 Changeset detected

Latest commit: c09aa3a

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

horo-fox · 2023-10-24T01:07:39Z

packages/markdown/remark/src/remark-shiki.ts

@@ -30,17 +30,19 @@ const highlighterCacheAsync = new Map<string, Promise<Highlighter>>();
 export function remarkShiki({
 	langs = [],
 	theme = 'github-dark',
+	themes = {},


I could elsewhere try to incorporate theme into themes but I don't think it's worth it.

(something like themes: {light: theme, ...themes})

ematipico

I believe it's OK to update the documentation to document this new feature, although I think it should contain "experimental" in its name and the documentation.

packages/markdown/remark/src/types.ts

sarah11918

Just speaking for docs here, @horo-fox ! As to whether or not new documentation is needed on our Astro Docs site, when we switched to shikiji in the first place, it seemed to be described as just an internal refactor and we did not update any docs. Our docs do talk about setting shiki config options etc.

I'll maybe also loop @bluwy in here: do any of our existing docs need to change, or would our users still set shiki config options, which internally will be processed using shikiji? My big question is, if users want to set their own syntax highlighting options, do they need to know about shikiji or are we still just referring to it and setting it through a user-facing shiki config?

.changeset/tender-suits-glow.md

horo-fox · 2023-10-24T15:26:48Z

My big question is, if users want to set their own syntax highlighting options, do they need to know about shikiji or are we still just referring to it and setting it through a user-facing shiki config?

I suppose they should know about shikiji because the CSS to make this work depends on a bit of implementation detail (the default defaultColor (light) since I didn't expose that, the way the CSS vars get created, and having https://github.com/antfu/shikiji#query-based-dark-mode linked would help).

But I'm not sure if that's what is best. Maybe it might be better to rename the CSS variables (using cssVariablePrefix), expose defaultColor, and talk about the CSS needed to expose this?

bluwy · 2023-10-24T16:29:49Z

I haven't fully digest the PR and will take a deeper look tomorrow, but the general idea make sense to me. (and also the experimental name). Also from a brief look, the PR isn't covering all the cases yet. I'd suggest looking for files that contain the word shiki.ts, remark-shiki.ts, and schema.ts for additional places to tweak.

It's not great that shiki handling is all over the place in our codebase currently but I'll work on refactoring that soon 😅

I'll maybe also loop @bluwy in here: do any of our existing docs need to change, or would our users still set shiki config options, which internally will be processed using shikiji? My big question is, if users want to set their own syntax highlighting options, do they need to know about shikiji or are we still just referring to it and setting it through a user-facing shiki config?

Yes I think we can keep referring to setting shiki options. Technically speaking, this new option is powered by shikiji so we should also mention and link to its docs. But conceptually, we can keep referring the whole idea as shiki as the new API is still using shiki-compatible types.

Talking about this page, there's also the wrap option which isn't a shiki config, but an Astro-specific addition. So in some ways this PR will be similar too.

shikiji also doesn't have documentation for theme and langs so we still need to link to shiki for the docs.

Co-authored-by: Sarah Rainsberger <[email protected]>

horo-fox · 2023-10-24T21:21:55Z

I've gone ahead and made a docs PR over at withastro/docs#5191, only PR review comment left is making sure this is complete...

sarah11918

Just making my review here official! I've also marked the corresponding docs PR as approved, just waiting for this to be released before it's merged live into docs. 🥳

horo-fox · 2023-10-30T02:12:20Z

I've gone ahead and checked the files you mentioned and a bit more (the Code element which I held off on updating... but TBH updating it is probably fine? I'd be OK with reverting that) -- @bluwy mind taking another look?

horo-fox · 2023-11-03T04:43:51Z

Bump for @bluwy in case this got dropped somewhere ^^

bluwy

Awesome work! The implementation and tests looks clean to me 👍 One small suggestion about the changeset below.

.changeset/tender-suits-glow.md

Co-authored-by: Bjorn Lu <[email protected]>

bluwy · 2023-11-08T14:15:42Z

Looks like a merge conflict. I'll resolve it and merge this one shortly

Co-authored-by: Sarah Rainsberger <[email protected]>

horo-fox added 2 commits October 24, 2023 10:00

Pass through themes to shikiji

cb7b9f1

Add changeset

25adb71

github-actions bot added feat: markdown Related to Markdown (scope) pr: docs A PR that includes documentation for review labels Oct 24, 2023

horo-fox commented Oct 24, 2023

View reviewed changes

Run pnpm run format locally

ad1735a

horo-fox changed the title ~~Light dark theming~~ Light/dark theming for shikiji's codeblocks Oct 24, 2023

ematipico requested a review from bluwy October 24, 2023 09:46

ematipico reviewed Oct 24, 2023

View reviewed changes

packages/markdown/remark/src/types.ts Outdated Show resolved Hide resolved

sarah11918 reviewed Oct 24, 2023

View reviewed changes

.changeset/tender-suits-glow.md Outdated Show resolved Hide resolved

horo-fox and others added 3 commits October 25, 2023 04:59

Update .changeset/tender-suits-glow.md

bf4df82

Co-authored-by: Sarah Rainsberger <[email protected]>

Use experimentalThemes instead of themes

031bdd2

Update forgotten themes reference

7f5a73a

horo-fox mentioned this pull request Oct 24, 2023

Update docs for shikiji multiple theming withastro/docs#5191

Merged

sarah11918 approved these changes Oct 25, 2023

View reviewed changes

Update other places I didn't notice

c0327fe

github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Oct 30, 2023

bluwy approved these changes Nov 3, 2023

View reviewed changes

.changeset/tender-suits-glow.md Outdated Show resolved Hide resolved

Update .changeset/tender-suits-glow.md

ed71006

Co-authored-by: Bjorn Lu <[email protected]>

bluwy added the semver: minor Change triggers a `minor` release label Nov 6, 2023

sarah11918 removed the pr: docs A PR that includes documentation for review label Nov 6, 2023

Merge branch 'main' into light-dark-theming

3fc43be

github-actions bot added the pr: docs A PR that includes documentation for review label Nov 8, 2023

Fix build issue

c09aa3a

bluwy removed the pr: docs A PR that includes documentation for review label Nov 8, 2023

bluwy merged commit c5010aa into withastro:main Nov 8, 2023
12 of 13 checks passed

astrobot-houston mentioned this pull request Nov 8, 2023

[ci] release #9021

Merged

horo-fox deleted the light-dark-theming branch November 8, 2023 20:15

natemoo-re pushed a commit that referenced this pull request Nov 22, 2023

Light/dark theming for shikiji's codeblocks (#8903)

2faf803

Co-authored-by: Sarah Rainsberger <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Light/dark theming for shikiji's codeblocks #8903

Light/dark theming for shikiji's codeblocks #8903

horo-fox commented Oct 24, 2023

changeset-bot bot commented Oct 24, 2023 •

edited

Loading

horo-fox Oct 24, 2023

ematipico left a comment •

edited

Loading

sarah11918 left a comment

horo-fox commented Oct 24, 2023 •

edited

Loading

bluwy commented Oct 24, 2023 •

edited

Loading

horo-fox commented Oct 24, 2023 •

edited

Loading

sarah11918 left a comment

horo-fox commented Oct 30, 2023

horo-fox commented Nov 3, 2023

bluwy left a comment

bluwy commented Nov 8, 2023

Light/dark theming for shikiji's codeblocks #8903

Light/dark theming for shikiji's codeblocks #8903

Conversation

horo-fox commented Oct 24, 2023

Changes

Testing

Docs

changeset-bot bot commented Oct 24, 2023 • edited Loading

🦋 Changeset detected

horo-fox Oct 24, 2023

Choose a reason for hiding this comment

ematipico left a comment • edited Loading

Choose a reason for hiding this comment

sarah11918 left a comment

Choose a reason for hiding this comment

horo-fox commented Oct 24, 2023 • edited Loading

bluwy commented Oct 24, 2023 • edited Loading

horo-fox commented Oct 24, 2023 • edited Loading

sarah11918 left a comment

Choose a reason for hiding this comment

horo-fox commented Oct 30, 2023

horo-fox commented Nov 3, 2023

bluwy left a comment

Choose a reason for hiding this comment

bluwy commented Nov 8, 2023

changeset-bot bot commented Oct 24, 2023 •

edited

Loading

ematipico left a comment •

edited

Loading

horo-fox commented Oct 24, 2023 •

edited

Loading

bluwy commented Oct 24, 2023 •

edited

Loading

horo-fox commented Oct 24, 2023 •

edited

Loading