Skip to content

Build/Test Tools: Format copied Gutenberg style registry - #12778

Open
benitoalba wants to merge 4 commits into
WordPress:trunkfrom
benitoalba:fix/65278-generated-php-formatting
Open

benitoalba wants to merge 4 commits into
WordPress:trunkfrom
benitoalba:fix/65278-generated-php-formatting

Conversation

@benitoalba

@benitoalba benitoalba commented Jul 30, 2026 •

Copy link
Copy Markdown

Formats the generated Gutenberg style registry while it is copied into Core.

Gutenberg currently outputs registry.php with unaligned array operators and missing spaces inside non-empty dependency arrays. Running build:dev followed by a Core PHPCS scan therefore reports coding standards violations. The copy step normalizes those known fields without requiring PHP or Composer in build-only jobs.

Processing is limited to registry.php with noProcess: [ '**/*', '!**/registry.php' ]. Other files are copied as buffers, preserving their bytes if Gutenberg adds binary assets. This also removes the need for the filename check inside the formatter.

Trac ticket: https://core.trac.wordpress.org/ticket/65278

Testing

Validated against trunk 0ce33127d5 with Node 24.18.0 and PHP 8.4.26.

  • npm ci --no-audit --no-fund
  • npm run build:dev and npm run build
  • npm run grunt -- jshint:grunt
  • PHP syntax, Core PHPCS, and PHPCompatibility checks on the copied registry
  • git diff --check

The unformatted registry reports 34 PHPCS errors and 40 warnings with the current Gutenberg build. The formatted output passes both Core rulesets.

A regression check runs the actual Grunt copy target with temporary PNG, raw binary, CSS with a byte-order mark, and nested registry fixtures. The original options alter all three non-registry fixtures. With this change, all 127 non-registry files, including the 124 existing style assets, copy byte for byte. The formatter runs only for the root and nested registries.

Both registry outputs match the previous formatter's output exactly and remain unchanged on a second pass. PHP confirms that all 20 entries retain their values and ordering. After both full builds, all 125 copied style files match the expected bytes. The temporary fixtures and validation scripts are kept outside the PR changes.

Use of AI Tools

AI assistance: Yes
Tool(s): OpenAI Codex
Model(s): GPT-5 (original work), GPT-6 (this follow-up)
Used for: Issue research, implementation, the review follow-up, and local validation.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Copilot AI review requested due to automatic review settings July 30, 2026 21:26
@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the Core build pipeline to normalize formatting in the copied Gutenberg style registry (registry.php) so that Core PHPCS scans pass in build-only environments without requiring PHP/Composer tooling.

Changes:

  • Adds a copy:gutenberg-styles options.process hook that only targets registry.php.
  • Normalizes alignment around => for the handle, path, and dependencies keys, and adjusts spacing in non-empty dependency arrays during the copy step.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Gruntfile.js Resolved
@benitoalba
benitoalba marked this pull request as ready for review July 30, 2026 23:37
@github-actions

github-actions Bot commented Jul 30, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props benitoalba, lancewillett, dhrupo.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@lancewillett lancewillett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to land. We can adjust from here with subsequent changes.

wporg-sync pushed a commit that referenced this pull request Sep 22, 2026
Align array assignment operators and normalize dependency array spacing when copying the generated style registry into Core. This allows PHPCS scans after build:dev to pass without requiring PHP tooling in build-only jobs.

Developed in: #12778

Props benitoalba, desrosj, jorbin, wildworks.
See #65278.


git-svn-id: https://develop.svn.wordpress.org/trunk@63883 602fd350-edb4-49c9-b593-d223f7449a82
@lancewillett

Copy link
Copy Markdown
Member

Landed in https://core.trac.wordpress.org/changeset/63883

wporg-sync pushed a commit to WordPress/WordPress that referenced this pull request Sep 22, 2026
Align array assignment operators and normalize dependency array spacing when copying the generated style registry into Core. This allows PHPCS scans after build:dev to pass without requiring PHP tooling in build-only jobs.

Developed in: WordPress/wordpress-develop#12778

Props benitoalba, desrosj, jorbin, wildworks.
See #65278.

Built from https://develop.svn.wordpress.org/trunk@63883


git-svn-id: http://core.svn.wordpress.org/trunk@63055 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@lancewillett

Copy link
Copy Markdown
Member

Reopening after the revert in https://core.trac.wordpress.org/changeset/63919.

Next step: settle the file-placement exceptions and upstream formatting approach before moving these forward.

Discussion: https://wordpress.slack.com/archives/C08D0Q6BHNY/p1790267518339219

@lancewillett lancewillett reopened this Sep 24, 2026
@lancewillett

Copy link
Copy Markdown
Member

@desrosj following up: this is waiting on your call on file placement and upstream formatting for the copied style registry. Once you decide, I can land or adjust it.

AI review · claude-opus-5-5

@dhrupo

dhrupo commented Nov 7, 2026

Copy link
Copy Markdown

Tested this on trunk b6091aa18d with the PR head 4265f7fb87 applied (macOS arm64, Node 24.18.1, PHP 7.4.33, PHPCS 3.13.5, Gutenberg c035c1a649).

Before: after npm run build:dev, vendor/bin/phpcs -n reports 34 ERRORS ... IN 1 FILE, and the file is src/wp-includes/css/dist/registry.php. grunt format:php:error aborts on the same file. npm run build produces the same 34 errors.

After: phpcs and phpcompat both exit 0 on registry.php, and the full phpcs -n scan is clean. The output is byte-identical to what phpcbf writes for the trunk file. Running the formatter on its own output changes nothing. The other 124 files in css/dist are unchanged. npm run build also works.

One problem: adding process to the gutenberg-styles target makes grunt read every file in it as UTF-8 text. Today there are only CSS files and registry.php, so nothing breaks. As a test, I put a PNG into gutenberg/build/styles/: with this PR it was copied as 5543 bytes instead of 3113 (corrupted). On trunk the copy was identical. Adding noProcess: [ '**/*', '!**/registry.php' ] to the options fixed that, and registry.php was still formatted.

A smaller point: the key list and the padding are hard-coded. A new key from the generator, or a []/multi-line dependency array, would pass through with no error. The alignment warnings would then come back.

Full report is on the Trac ticket.

@lancewillett

Copy link
Copy Markdown
Member

@benitoalba @dhrupo found one change worth making before this lands.

Setting process on the gutenberg-styles target makes grunt read and write every file in it as UTF-8 text, not just registry.php (node_modules/grunt/lib/grunt/file.js, lines 314–320). Today that's harmless: gutenberg/build/styles holds 768 CSS files and the one PHP file. But a binary file added upstream would be copied corrupted, with no error.

Please scope the processing to the registry:

options: {
	noProcess: [ '**/*', '!**/registry.php' ],
	process: function( content, srcpath ) {

With that, the path.basename() check inside process becomes redundant and can go.

The hard-coded key list is lower risk. A new key would pass through unformatted rather than break the build, so it's fine to leave.

My approval stands with that change. This still waits on @desrosj's file-placement call.

AI review · claude-opus-5-5

@benitoalba

Copy link
Copy Markdown
Author

Thanks @lancewillett and @dhrupo! Added noProcess and removed the filename check in 62922a2. Verified that binary files copy unchanged and the registry formatting stays the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

5 participants