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 directory separation problems on Windows machine #14

Merged
merged 2 commits into from
Sep 28, 2023
Merged

Fix directory separation problems on Windows machine #14

merged 2 commits into from
Sep 28, 2023

Conversation

BoGnY
Copy link
Contributor

@BoGnY BoGnY commented Sep 27, 2023

Fork and rebase of #9 for fix #4 and fix #10

@TomasVotruba
Copy link
Member

Thanks 👍

Could you extract this to a method, so we can re-use it have it clearly seprated? Something like "normalizePath"

@BoGnY
Copy link
Contributor Author

BoGnY commented Sep 27, 2023

@TomasVotruba commented on Sep 27, 2023, 1:03 PM GMT+2:

Thanks 👍

Could you extract this to a method, so we can re-use it have it clearly seprated? Something like "normalizePath"

I have found that upgrading nette/utils from 3.2 to 4.0 they have added the method FileSystem::unixSlashes($path) which is exactly what we need 😄
I've already checked that there are not breaking changes with your package if we upgrade nette/utils to 4.0.

Do you prefer upgrade or only extraction method??

If only extraction, do you prefer normalizePath($path) method on both PatchDiffer and PathResolver class or I create a new class FileSystem, maybe under Symplify\VendorPatches\Utils namespace, and put method inside it??

@TomasVotruba
Copy link
Member

Extraction method is good enough. FileSystemHelper with static method is fine with me 👍

@BoGnY
Copy link
Contributor Author

BoGnY commented Sep 28, 2023

Done. I've also fixed test so it not fails on Windows machine.

@TomasVotruba TomasVotruba merged commit f63c6f4 into symplify:main Sep 28, 2023
@TomasVotruba
Copy link
Member

Thank you

@BoGnY
Copy link
Contributor Author

BoGnY commented Oct 12, 2023

@TomasVotruba can you tag a new release??

@TomasVotruba
Copy link
Member

Sure, 11.3.0 is out now 👍

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.

Issues with patching on Local Windows Machine Incorrect directory separators under windows.
2 participants