Skip to content

Conversation

Rich-Harris
Copy link
Member

alternative to #560. This is just a test, we would need to make the same change in the svelte and kit repos

Copy link

vercel bot commented Oct 23, 2024

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

Name Status Preview Comments Updated (UTC)
kit-svelte-dev 🔄 Building (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 7:59pm
learn-svelte-dev 🛑 Canceled (Inspect) Oct 23, 2024 7:59pm
omnisite ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 7:59pm
svelte-5-preview 🔄 Building (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 7:59pm
svelte-dev 🔄 Building (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 7:59pm

const cache = await caches.open(CACHE);

// `build`/`files` can always be served from the cache
// `build` / `files` can always be served from the cache
Copy link
Member

Choose a reason for hiding this comment

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

Presumably not strictly necessary here. I don't know whether this still makes sense to do anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we may as well be consistent

@dummdidumm
Copy link
Member

dummdidumm commented Oct 23, 2024

can we automate this somehow? Like, the markdown renderer sees "there's a ` followed by a / followed by a `, I'm putting a space inbetween that ". I feel like we're going to make this mistake over and over again if we're not super careful

@Conduitry
Copy link
Member

Automating this via the renderer sounds good, if we can make it work properly. As Rich's initial mass-find-replace indicated, there are a lot of false positives if you're just looking for those three specific characters in a row. But maybe it might more doable in a renderer if we have access to more AST-y stuff.

@Rich-Harris
Copy link
Member Author

The first question to answer is whether we like this change:

image image

Personally I think it's actually an improvement, but want to be sure others agree.

As for automating: are we imagining a warning or actually transforming it? Not sure off the top of my head if there's a single place to put that logic

@dummdidumm
Copy link
Member

Actually transform it.
Agree it looks better

@dummdidumm dummdidumm merged commit 23c976a into main Oct 23, 2024
7 checks passed
@dummdidumm dummdidumm deleted the separate-code-spans branch October 23, 2024 20:59
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.

3 participants