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 slashes in paths on Windows #201

Merged
merged 2 commits into from
Jan 30, 2020
Merged

Fix slashes in paths on Windows #201

merged 2 commits into from
Jan 30, 2020

Conversation

michaelmior
Copy link
Contributor

Currently things break in bash on Windows because the cwd and the current path use different types of slashes and eventually the globby test shows that they are in different directories.
This just normalizes the slashes for both paths so it won't cause issues later.

@tclindner
Copy link
Owner

Hey @michaelmior! Thank you for opening a PR! I’ll get a release cut tomorrow. Before I merge I would like to review some recent changes that were done for globby. I recall that we hit something like this before.

Is it possible to solve this without introducing a module?

@michaelmior
Copy link
Contributor Author

I think that's a good question :) I generally like to avoid introducing new modules as well. In this case, globby itself already uses slash so it will be installed anyway. That said, slash is basically one function which is relatively short so I'm sure that relevant logic could be extracted.

What would be even nicer is if we could guarantee that both paths were generated in the same format in the first place.

@tclindner
Copy link
Owner

Thank you! I’ll get the release cut tomorrow. Thank you again for taking the time to open a PR. I appreciate it!

@michaelmior
Copy link
Contributor Author

Thanks!

@tclindner tclindner added the bug 🪲 Something isn't working label Jan 30, 2020
@tclindner tclindner merged commit 7034270 into tclindner:master Jan 30, 2020
@tclindner
Copy link
Owner

@michaelmior v4.5.1 has been released. Thank you again for opening a PR 🎉 🙌

@michaelmior
Copy link
Contributor Author

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants