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

fix: update regex to allow whitespace & + in Windows path #74

Merged
merged 4 commits into from
Aug 5, 2024
Merged

fix: update regex to allow whitespace & + in Windows path #74

merged 4 commits into from
Aug 5, 2024

Conversation

MauriceChocoSwiss
Copy link
Contributor

@MauriceChocoSwiss MauriceChocoSwiss commented Jul 24, 2024

Updated regex to authorize whitespace in file path.

Many projects has a whitespace in folder name (like me). Many time it doesn't affect anything but here it prevent file to open.

Let me know !

@dominikg
Copy link
Contributor

Hey,

would you be interested in adding + as well? Thats used by sveltekit ( see #50 (comment) ) and sveltejs/vite-plugin-svelte#943

cc @sodatea this would be the "expand allow list" i mentioned, might be a stop-gap solution until you find the time to implement propper escaping.

@MauriceChocoSwiss
Copy link
Contributor Author

Why not, I didn't use sveltkit but I think it can be good for sveltkit user.

@dominikg
Copy link
Contributor

i wonder if you want \s or an actual space char in that group

MauriceChocoSwiss and others added 2 commits July 30, 2024 14:37
Co-authored-by: Dominik G. <dominik.goepel@gmx.de>
Co-authored-by: Dominik G. <dominik.goepel@gmx.de>
@MauriceChocoSwiss
Copy link
Contributor Author

MauriceChocoSwiss commented Jul 30, 2024

I prefer to use the \s, it cover more cases like tabs, line breaks etc... In case of ^^

@dominikg
Copy link
Contributor

dominikg commented Aug 1, 2024

is it possible to have line breaks or tabs in a file path on windows? (that file system never fails to impress, just not sure if in a positive way)

@MauriceChocoSwiss
Copy link
Contributor Author

I really don't know... You prefer to use only spaces ?

@dominikg
Copy link
Contributor

dominikg commented Aug 1, 2024

unless they are possible/allowed, i would prefer just space, but maybe @sodatea has a different opinion.

It would be good to get this merged soonish so we can include it in vite 5.5.

@haoqunjiang haoqunjiang merged commit 9779c46 into yyx990803:master Aug 5, 2024
@haoqunjiang haoqunjiang changed the title Update regex to authorize whitespace in path fix: update regex to allow whitespace & + in Windows path Aug 5, 2024
@haoqunjiang
Copy link
Collaborator

According to https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file CR, LF and TABs are not allowed in a file path. So I think a single space character should suffice.

@haoqunjiang
Copy link
Collaborator

As for other special characters, there seem to be many more to add for compatibility with all the popular filesystem-based routing solutions, but I'll leave that to a later PR.

@oneezy
Copy link

oneezy commented Aug 24, 2024

As for other special characters, there seem to be many more to add for compatibility with all the popular filesystem-based routing solutions, but I'll leave that to a later PR.

yeahh @sodatea it's still an issue on Win11

Reference

issue: sveltejs/vite-plugin-svelte#943 (comment)

@Loizzus
Copy link

Loizzus commented Aug 26, 2024

I don't think banning characters is the solution to this problem. Even if you did find all the characters that are currently used by every system that depends on Vite why prevent some creative new use of valid special characters in the future? This entire feature seems very short sighted.

haoqunjiang added a commit to haoqunjiang/launch-editor that referenced this pull request Sep 4, 2024
haoqunjiang added a commit to haoqunjiang/launch-editor that referenced this pull request Sep 4, 2024
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.

5 participants