search

WordPress / create-block-theme

A WordPress plugin to create block themes
https://wordpress.org/plugins/create-block-theme/
GNU General Public License v2.0
324 stars 51 forks source link

Fix problem with zip file creation on Windows #606

Closed t-hamano closed 5 months ago

t-hamano commented 5 months ago

Fixes #602

This PR fixes an issue where files are not created correctly when using certain file archivers on Windows OS.

Where the problem occurs, the file is generated by ZipArchive::addFromString. The PHP documentation mentions the following:

Note: For maximum portability, it is recommended to always use forward slashes (/) as directory separator in ZIP filenames.

I don't know the exact cause, but I think backslashes are causing problems with certain file archivers.

Testing Instructions

Screenshot

Before After
image image
nicholasingham commented 5 months ago

Just a thought. The plugin code doesn't use the path_join function at the moment.

Syntax such as $a. '/' . $b or 'pattern/' . $c is used throughout.

For consistency, would it be better to continue to use the . '/' . syntax for this PR and decide whether to change to path_join everywhere later?

vcanales commented 5 months ago

Just a thought. The plugin code doesn't use the path_join function at the moment.

Syntax such as $a. '/' . $b or 'pattern/' . $c is used throughout.

For consistency, would it be better to continue to use the . '/' . syntax for this PR and decide whether to change to path_join everywhere later?

We're introducing it as a part of another PR as well https://github.com/WordPress/create-block-theme/pull/601.

There are also places where it might not be suitable to use it ie. where DIRECTORY_SEPARATOR is in use.

I don't see a drawback in introducing it here, and also gradually whenever we need to make changes to path handling logic.

nicholasingham commented 5 months ago

Makes sense. Many thanks.