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: composer scripts doesn't work on Windows #233

Merged
merged 1 commit into from
Feb 21, 2023
Merged

Fix: composer scripts doesn't work on Windows #233

merged 1 commit into from
Feb 21, 2023

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Feb 18, 2023

Note: I added the --no-verify option on commit to prevent the large number of diffs created by lint-staged.

This PR fixes the composer's lint/format command to work on Windows.

PS D:\path\to\create-block-theme> npm run lint:php

> create-block-theme@1.5.1 lint:php D:\path\to\create-block-theme
> ./vendor/bin/phpcs

'.' is not recognized as an internal or external command,
operable program or batch file.

As seen in the issue in the npm repository, npm scripts starting with ./ do not work on Windows. Therefore, I used composer run-script instead, as is done in the Gutenberg repository.

In addition, .phpcs.xml.dist was renamed to phpcs.xml.dist because the ruleset file name specified by the composer script does not exist.


Confirm that the following commands work:

Should show errors.

npm run lint:php

Error should appear and the code should be formatted. To restore the automatically changed format, run git checkout ..

npm run lint:php:fix

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! Sorry, this was my bad. I didn't realise scripts starting with ./ don't work on Windows.

I can confirm npm run lint:php and npm run lint:php:fix still work the same for me.

🚢

@mikachan mikachan merged commit fa1e282 into WordPress:trunk Feb 21, 2023
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.

2 participants