infinum / eightshift-libs

Library that is meant to be used inside Eightshift Boilerplate and Eightshift Boilerplate Plugin libs via composer in order to be able to easily set up a modern development process.
https://eightshift.com
MIT License
62 stars 11 forks source link

Paths not resolved correctly on Windows #443

Open ashthomas92 opened 1 week ago

ashthomas92 commented 1 week ago

On Windows, no paths are resolved from the manifest, leading to a InvalidManifest exception.

Description

The Helpers::joinPaths() method prepends the directory separator onto the generated base file path. On Windows this causes the paths not to be resolved, leading to it being unable to read any files from manifest.json. This fix checks to see if the base path has a disk indicator prefixing the path, and only prepends the separator when it doesn't.

mbmjertan commented 1 week ago

Glad to see Eightshift usage and contributions.

I think this method is unreliable, especially for extending Eightshift, and should be refactored entirely. I am willing to do this myself.

This isn't at all a critique of your PR, @ashtomas92, and I am very grateful to see contributions.

why couldn't I just do this:

file_get_contents(Helpers::joinPaths(['Blocks', 'components', 'heading', 'manifest.json']

Once again, very happy to see contributions. I am hoping a fix will get merged soon.

hexydec commented 2 days ago

If the original path is captured from a known location using __DIR__, the base path should be easy to work out without de/reconstructing it, the slash at the start can then just remain if it was already present, same for the drive indicator as per Windows.