Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: get actual protocol for windows instead of protocol + drive #28

Merged
merged 3 commits into from
Jan 5, 2022

Conversation

sheremet-va
Copy link
Contributor

@sheremet-va sheremet-va commented Dec 31, 2021

On windows current regexp from this url gets incorrect protocol:

file:///C:/path
  • Incorrect: file:///C
  • Correct: file

This PR fixes it making {2,} ungreedy, so it will end after the first :. (You can test it in https://regex101.com/)

Related: vitest-dev/vitest#379

@antfu
Copy link
Member

antfu commented Dec 31, 2021

Can you add some tests?

@sheremet-va
Copy link
Contributor Author

sheremet-va commented Dec 31, 2021

Can you add some tests?

Your test setup doesn't allow me to import that function to properly test it, unless I can export it.

isValidNodeImport checks on the running machine, but tests run only on ubuntu and even then it's hard to replicate without Windows.

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@pi0 pi0 merged commit 15140cc into unjs:main Jan 5, 2022
@sheremet-va sheremet-va deleted the fix_proto_windows branch January 7, 2022 07:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants