Refactoring - wp-themes-inactive, wp-parent-theme & wp-active-theme by apermo · Pull Request #7507 · WordPress/wordpress-develop

apermo · 2024-10-05T16:14:40Z

'wp-active-theme' => self::get_wp_active_theme(),
'wp-parent-theme' => self::get_wp_parent_theme(),
'wp-themes-inactive' => self::get_wp_themes_inactive(),

Follow up to #7027 as discussed with @dmsnell and @costdev

Summary of the coming steps:

wp-filesystem
wp-constants
wp-database
wp-server
wp-media
wp-plugins-active & wp-plugins-inactive
wp-mu-plugins
& 9. & 10. wp-themes-inactive, wp-parent-theme & wp-active-theme (we are here)
wp-dropins
wp-paths-sizes
wp-core

Refactoring WP_Debug_Data::debug_data(); into smaller functions for

better maintainability
reduced complexity
preparation for future unit tests
improved extensibility (see trac for details)
added php7.2 style type hints

Trac ticket: https://core.trac.wordpress.org/ticket/61648#ticket

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.

'wp-active-theme' => self::get_wp_active_theme(), 'wp-parent-theme' => self::get_wp_parent_theme(), 'wp-themes-inactive' => self::get_wp_themes_inactive(),

github-actions · 2024-10-05T16:15:11Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

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

Props apermo, dmsnell.

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

github-actions · 2024-10-05T16:26:56Z

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

The Plugin and Theme Directories cannot be accessed within Playground.
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.

dmsnell · 2024-10-05T16:48:22Z

Github's diffing utility is having a hard time with this patch. For those wanting to review it, I encourage using an external diff tool, or change the algorithm with git diff --patience, for example.

dmsnell · 2024-10-05T17:52:40Z

@apermo I pushed a commit with a few changes. I would appreciate your review of two in particular:

I removed what appears to be unused variables in debug_data() - perhaps we overlooked removing those in an earlier commit.
In get_wp_parent_theme() I replaced the if ( $parent_theme ) with an early abort if ( ! $parent_theme ). this was not only to remove the level of indentation, but I believe it was previously returning an undefined variable $fields since that was set inside the if structure.

apermo · 2024-10-05T17:54:36Z

src/wp-admin/includes/class-wp-debug-data.php

+			return array(
+				'label'  => __( 'Parent Theme' ),
+				'fields' => array(),
+			);


are you sure adding this redundancy is beneficial?

the thought behind the early-abort is to avoid mistakes based on humans needing to continue parsing the function. before this was extracted it was pre-set to be an array() and the if was purely additive. now, with the logic localized, we have to do something affirmative in each branch.

I don't see any one solution purely dominant here. the redundancy doesn't bother me at all, and I tend to be more concerned about if complexity. either way, I've reverted this change for a different fix in 276de8b which pre-sets the $fields = array() var so it's not unset when returning

apermo · 2024-10-05T17:57:13Z

src/wp-admin/includes/class-wp-debug-data.php

-		if ( $parent_theme ) {
-			$parent_theme_version       = $parent_theme->version;
-			$parent_theme_version_debug = $parent_theme_version;
+		if ( ! $parent_theme ) {


If you want to keep this early return, I'd vote for a stronger condition. ->parent() will return false

So unless you opt to remove it. How about:

if ( $parent_theme === false ) {

I've replaced the early-abort based on our Slack discussion. here though the flip in logic matches the existing behavior, whereas a strict check for false would change it.

This is the last part in a larger modularization of the data in `WP_Debug_Data`. Previously this was a single massive method drawing in debug data from various groups of related data, where the groups were independent from each other. This patch separates the findal set of twelve groups, the `wp-active-theme`, `wp-parent-theme`, and `wp-themes-inactive` info, into a separate methods focused on those data. This work precedes changes to make the `WP_Debug_Data` class more extensible for better use by plugin and theme code. Developed in #7507 Discussed in https://core.trac.wordpress.org/ticket/61648 Props apermo, dmsnell. Fixes #61648. git-svn-id: https://develop.svn.wordpress.org/trunk@59176 602fd350-edb4-49c9-b593-d223f7449a82

dmsnell · 2024-10-05T19:22:54Z

Merged in [59176]
dc6f059

This is the last part in a larger modularization of the data in `WP_Debug_Data`. Previously this was a single massive method drawing in debug data from various groups of related data, where the groups were independent from each other. This patch separates the findal set of twelve groups, the `wp-active-theme`, `wp-parent-theme`, and `wp-themes-inactive` info, into a separate methods focused on those data. This work precedes changes to make the `WP_Debug_Data` class more extensible for better use by plugin and theme code. Developed in WordPress/wordpress-develop#7507 Discussed in https://core.trac.wordpress.org/ticket/61648 Props apermo, dmsnell. Fixes #61648. Built from https://develop.svn.wordpress.org/trunk@59176 git-svn-id: http://core.svn.wordpress.org/trunk@58571 1a063a9b-81f0-0310-95a4-ce76da25c4cd

This is the last part in a larger modularization of the data in `WP_Debug_Data`. Previously this was a single massive method drawing in debug data from various groups of related data, where the groups were independent from each other. This patch separates the findal set of twelve groups, the `wp-active-theme`, `wp-parent-theme`, and `wp-themes-inactive` info, into a separate methods focused on those data. This work precedes changes to make the `WP_Debug_Data` class more extensible for better use by plugin and theme code. Developed in WordPress/wordpress-develop#7507 Discussed in https://core.trac.wordpress.org/ticket/61648 Props apermo, dmsnell. Fixes #61648. Built from https://develop.svn.wordpress.org/trunk@59176 git-svn-id: https://core.svn.wordpress.org/trunk@58571 1a063a9b-81f0-0310-95a4-ce76da25c4cd

This is the last part in a larger modularization of the data in `WP_Debug_Data`. Previously this was a single massive method drawing in debug data from various groups of related data, where the groups were independent from each other. This patch separates the findal set of twelve groups, the `wp-active-theme`, `wp-parent-theme`, and `wp-themes-inactive` info, into a separate methods focused on those data. This work precedes changes to make the `WP_Debug_Data` class more extensible for better use by plugin and theme code. Developed in WordPress#7507 Discussed in https://core.trac.wordpress.org/ticket/61648 Props apermo, dmsnell. Fixes #61648. git-svn-id: https://develop.svn.wordpress.org/trunk@59176 602fd350-edb4-49c9-b593-d223f7449a82

Refactored

b246788

'wp-active-theme' => self::get_wp_active_theme(), 'wp-parent-theme' => self::get_wp_parent_theme(), 'wp-themes-inactive' => self::get_wp_themes_inactive(),

Merge branch 'trunk' into refactor-wp-debug-data/themes

907bbd3

Indentation alignment, early-abort instead of returning undefined array.

139cd01

apermo commented Oct 5, 2024

View reviewed changes

Re-indent

276de8b

dmsnell approved these changes Oct 5, 2024

View reviewed changes

dmsnell closed this Oct 5, 2024

dmsnell deleted the refactor-wp-debug-data/themes branch October 5, 2024 19:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring - wp-themes-inactive, wp-parent-theme & wp-active-theme#7507

Refactoring - wp-themes-inactive, wp-parent-theme & wp-active-theme#7507
apermo wants to merge 4 commits intoWordPress:trunkfrom
apermo:refactor-wp-debug-data/themes

apermo commented Oct 5, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Oct 5, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Oct 5, 2024

Uh oh!

dmsnell commented Oct 5, 2024

Uh oh!

dmsnell commented Oct 5, 2024

Uh oh!

apermo Oct 5, 2024

Uh oh!

dmsnell Oct 5, 2024

Uh oh!

apermo Oct 5, 2024

Uh oh!

dmsnell Oct 5, 2024

Uh oh!

dmsnell commented Oct 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

apermo commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 5, 2024

Test using WordPress Playground

Some things to be aware of

Uh oh!

dmsnell commented Oct 5, 2024

Uh oh!

dmsnell commented Oct 5, 2024

Uh oh!

apermo Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

dmsnell Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

apermo Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

dmsnell Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

dmsnell commented Oct 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

apermo commented Oct 5, 2024 •

edited

Loading

github-actions bot commented Oct 5, 2024 •

edited

Loading