Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Use filepath package throughout for Windows support#34

Merged
FooSoft merged 1 commit intoFooSoft:masterfrom
albertdev:bugfix/use-filepath-package
Dec 14, 2021
Merged

Use filepath package throughout for Windows support#34
FooSoft merged 1 commit intoFooSoft:masterfrom
albertdev:bugfix/use-filepath-package

Conversation

@albertdev
Copy link
Contributor

The "path" package supports only forward slashes in paths, like those found in *nix operating systems or URLs.

Meanwhile Windows with its CP/M and QDOS ancestry requires drive letters and backslashes, and thus the "filepath" package needs to be used. This packages handles all the gnarly edge cases on different platforms and will make sure that all paths are properly normalized.

I've been able to use homemaker on Windows without this fix, but the experience has been bad.

@albertdev
Copy link
Contributor Author

albertdev commented Sep 13, 2021

Please note that this PR contains all of my recent fixes. However, after moving some functions around I couldn't really make this one work on its own, so please have a look at my other PRs first and see if they can be merged.

If those would be completed then I can rebase this PRs branch to be clean, at which point the history is straightforward again.

This is the main reason why I marked this as a "Draft PR".

@FooSoft
Copy link
Owner

FooSoft commented Dec 14, 2021

The other PRs look fine and have been merged!

The "path" package supports only forward slashes in paths, like those
found in *nix operating systems or URLs.

Meanwhile Windows with its CP/M and QDOS ancestry requires drive letters
and backslashes, and thus the "filepath" package needs to be used.
@albertdev albertdev force-pushed the bugfix/use-filepath-package branch from e376da9 to edb42a0 Compare December 14, 2021 22:41
@albertdev
Copy link
Contributor Author

@FooSoft Thank you. I've rebased my changes so that this PR looks cleaner.

@albertdev albertdev marked this pull request as ready for review December 14, 2021 22:44
@FooSoft
Copy link
Owner

FooSoft commented Dec 14, 2021

Yep, this looks good -- thanks again!

@FooSoft FooSoft merged commit 0a90c2e into FooSoft:master Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants