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.

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

feat: support space contained in the filename for asset #8752

Merged
merged 5 commits into from
Oct 20, 2023
Merged

feat: support space contained in the filename for asset #8752

merged 5 commits into from
Oct 20, 2023

Conversation

Jeff-Tian
Copy link
Contributor

See vuejs/vitepress#2596.

When including an image into a markdown file, most tools must encode the space to render the image in preview mode.

But the encoded space will fail the vitepress building process due to the import statement containing the encoded path. So we need to decode it back when it's an absolute path.

Using space in the filename is a bad idea, but the operating system allows it, and many automatic processes will generate a filename with space, so I hope we can support it!

More information

The transformAssetUrl will use parseUrl to the asset path, which encodes the path.

@patak-dev patak-dev mentioned this pull request Aug 1, 2023
Open
@sodatea sodatea added the 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. label Aug 4, 2023
@sxzz sxzz changed the title feat(https://github.com/vuejs/vitepress/issues/2596): support space contained in the filename for asset feat: support space contained in the filename for asset Aug 13, 2023
@Jeff-Tian Jeff-Tian mentioned this pull request Aug 31, 2023
Closed
3 tasks
sxzz
sxzz approved these changes Aug 31, 2023
View reviewed changes
Copy link
Member

@sxzz sxzz left a comment

Choose a reason for hiding this comment

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

LGTM

@sxzz sxzz added ready for review This PR requires more reviews and removed wait changes labels Aug 31, 2023
@github-actions
Copy link

github-actions bot commented Aug 31, 2023 •
edited

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 85.9 kB 32.6 kB 29.5 kB
vue.global.prod.js 132 kB 49.3 kB 44.4 kB

Usages

Name Size Gzip Brotli
createApp 47.9 kB 18.8 kB 17.2 kB
createSSRApp 50.6 kB 19.9 kB 18.2 kB
defineCustomElement 50.3 kB 19.6 kB 17.9 kB
overall 61.2 kB 23.7 kB 21.6 kB

@Jeff-Tian
Copy link
Contributor Author

I don't understand the lint-and-test-dts yet, but I will study it tomorrow. 😂

@sodatea sodatea added ready to merge The PR is ready to be merged. and removed ready for review This PR requires more reviews labels Oct 10, 2023
@yyx990803 yyx990803 merged commit 36c99a9 into vuejs:main Oct 20, 2023
11 checks passed
yyx990803 pushed a commit that referenced this pull request Oct 21, 2023
@Jeff-Tian @yyx990803
fix(compiler-sfc): support asset paths containing spaces (#8752)
1844f21 
By decoding them before generating them as JavaScript import paths

fix vuejs/vitepress#2596
fix vuejs/vitepress#573
lumozx pushed a commit to lumozx/core that referenced this pull request Oct 21, 2023
@Jeff-Tian @lumozx
fix(compiler-sfc): support asset paths containing spaces (vuejs#8752)
b7fb0ce 
By decoding them before generating them as JavaScript import paths

fix vuejs/vitepress#2596
fix vuejs/vitepress#573
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sodatea sodatea sodatea approved these changes

@sxzz sxzz sxzz approved these changes

Assignees

@sodatea sodatea

Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. need discussion ready to merge The PR is ready to be merged.
Projects
Triage Session
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Jeff-Tian @sodatea @sxzz @yyx990803