Skip to content
Closed
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9ca0b27
WIP
jrfnl Nov 2, 2022
57f0dba
WIP step 2
jrfnl Nov 2, 2022
58b0ff1
WIP - step 3
jrfnl Nov 2, 2022
d3bbd20
WIP Step 3b
jrfnl Nov 2, 2022
c17341b
WIP - setp 4
jrfnl Nov 2, 2022
aa13245
WIP - simplify it a little more
jrfnl Nov 2, 2022
188ce0f
WIP - step 5
jrfnl Nov 2, 2022
194368e
WIP - side step 6
jrfnl Nov 2, 2022
8fed0c1
WIP - side step 7
jrfnl Nov 2, 2022
5fa9aae
CS & cleanup
aristath Nov 3, 2022
979bfef
replace array_key_exists with isset
aristath Nov 3, 2022
be50567
replace an array_merge with a foreach loop
aristath Nov 3, 2022
6aa5ede
Add an inline comment to avoid future refactors
aristath Nov 3, 2022
4434b1d
We can use isset() instead of array_key_exists() here (no null values)
aristath Nov 3, 2022
505a4f8
replace array_merge with a foreach loop
aristath Nov 3, 2022
0345837
replace array_merge with foreach loop in the flatten_tree method
aristath Nov 3, 2022
8df441c
replace array_key_exists() with isset()
aristath Nov 3, 2022
8c04874
Replace more array_merge with foreach loops where appropriate
aristath Nov 3, 2022
213e94b
Fix PHP 5.6 errors + added notes for the future
aristath Nov 3, 2022
8923cd8
Merge branch 'trunk' into feature/pp-ari-wp-theme-json
felixarntz Nov 10, 2022
5730ac1
Add comment references pointing to https://core.trac.wordpress.org/ti…
felixarntz Nov 10, 2022
4af0999
Merge branch 'trunk' into feature/pp-ari-wp-theme-json
felixarntz Nov 10, 2022
abf51b6
Merge branch 'trunk' into feature/pp-ari-wp-theme-json
felixarntz Nov 10, 2022
4889396
Merge branch 'trunk' into feature/pp-ari-wp-theme-json
felixarntz Nov 11, 2022
7ecd0a8
Clarify variable name.
felixarntz Nov 11, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Replace more array_merge with foreach loops where appropriate
  • Loading branch information
aristath committed Nov 8, 2022
commit 8c0487419066ec3634deb55ed924df016343af7a
38 changes: 29 additions & 9 deletions src/wp-includes/class-wp-theme-json.php
Original file line number Diff line number Diff line change
Expand Up @@ -1908,7 +1908,10 @@ protected static function get_style_nodes( $theme_json, $selectors = array() ) {
return $nodes;
}

$nodes = array_merge( $nodes, static::get_block_nodes( $theme_json ) );
$block_nodes = static::get_block_nodes( $theme_json );
foreach ( $block_nodes as $block_node ) {
$nodes[] = $block_node;
}

/**
* Filters the list of style nodes with metadata.
Expand Down Expand Up @@ -2038,7 +2041,9 @@ public function get_styles_for_block( $block_metadata ) {
// the feature selector. This may occur when multiple block
// support features use the same custom selector.
if ( isset( $feature_declarations[ $feature_selector ] ) ) {
$feature_declarations[ $feature_selector ] = array_merge( $feature_declarations[ $feature_selector ], $new_feature_declarations );
foreach ( $new_feature_declarations as $new_feature_declaration ) {
$feature_declarations[ $feature_selector ][] = $feature_declaration;
Copy link
Member

@oandregal oandregal Dec 15, 2022

Choose a reason for hiding this comment

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

@aristath @desrosj @felixarntz @spacedmonkey @jrfnl @hellofromtonya

Shouldn't the variable assigned be $new_feature_declaration instead of $feature_declaration? This has been reported as problematic by the Gutenberg lint jobs (see conversation).

I haven't looked at what regression this introduced but it'd be good to create a follow-up PR fixing it and adding a test case.

Copy link
Member

Choose a reason for hiding this comment

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

@oandregal I concur. This is a bug which should be fixed.

Suggested change
$feature_declarations[ $feature_selector ][] = $feature_declaration;
$feature_declarations[ $feature_selector ][] = $new_feature_declaration;

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦 yeah, looks like this was a typo.. Good catch 👍

Copy link
Member

Choose a reason for hiding this comment

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

Happens to the best of us ;-)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with this code path, so asked folks to provide details for testing at WordPress/gutenberg#46579 (comment) so we can add a new unit test and gauge how severe this is.

Copy link
Member

@jrfnl jrfnl Dec 15, 2022

Choose a reason for hiding this comment

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

AFAICS, this should go in the next 6.1 patch release.

Edit to clarify: I'm basing this assessment on the following:

  • It's very clearly a bug with an obvious fix.
  • It's likely a regression compared to previous behaviour.

}
} else {
$feature_declarations[ $feature_selector ] = $new_feature_declarations;
}
Expand Down Expand Up @@ -2611,8 +2616,9 @@ protected static function remove_insecure_settings( $input ) {
$output = array();
foreach ( static::PRESETS_METADATA as $preset_metadata ) {
foreach ( static::VALID_ORIGINS as $origin ) {
$path_with_origin = array_merge( $preset_metadata['path'], array( $origin ) );
$presets = _wp_array_get( $input, $path_with_origin, null );
$path_with_origin = $preset_metadata['path'];
$path_with_origin[] = $origin;
$presets = _wp_array_get( $input, $path_with_origin, null );
if ( null === $presets ) {
continue;
}
Expand Down Expand Up @@ -2857,7 +2863,10 @@ public function get_data() {
*/
foreach ( $nodes as $node ) {
foreach ( static::PRESETS_METADATA as $preset_metadata ) {
$path = array_merge( $node['path'], $preset_metadata['path'] );
$path = $node['path'];
foreach ( $preset_metadata['path'] as $preset_metadata_path ) {
$path[] = $preset_metadata_path;
}
$preset = _wp_array_get( $output, $path, null );
if ( null === $preset ) {
continue;
Expand Down Expand Up @@ -2891,7 +2900,10 @@ public function get_data() {
foreach ( $nodes as $node ) {
$all_opt_ins_are_set = true;
foreach ( static::APPEARANCE_TOOLS_OPT_INS as $opt_in_path ) {
$full_path = array_merge( $node['path'], $opt_in_path );
$full_path = $node['path'];
foreach ( $opt_in_path as $opt_in_path_item ) {
$full_path[] = $opt_in_path_item;
}
// Use "unset prop" as a marker instead of "null" because
// "null" can be a valid value for some props (e.g. blockGap).
$opt_in_value = _wp_array_get( $output, $full_path, 'unset prop' );
Expand All @@ -2902,9 +2914,14 @@ public function get_data() {
}

if ( $all_opt_ins_are_set ) {
_wp_array_set( $output, array_merge( $node['path'], array( 'appearanceTools' ) ), true );
$node_path_with_appearance_tools = $node['path'];
$node_path_with_appearance_tools[] = 'appearanceTools';
_wp_array_set( $output, $node_path_with_appearance_tools, true );
foreach ( static::APPEARANCE_TOOLS_OPT_INS as $opt_in_path ) {
$full_path = array_merge( $node['path'], $opt_in_path );
$full_path = $node['path'];
foreach ( $opt_in_path as $opt_in_path_item ) {
$full_path[] = $opt_in_path_item;
}
// Use "unset prop" as a marker instead of "null" because
// "null" can be a valid value for some props (e.g. blockGap).
$opt_in_value = _wp_array_get( $output, $full_path, 'unset prop' );
Expand Down Expand Up @@ -3054,7 +3071,10 @@ public function set_spacing_sizes() {
$slug += 10;
}

$spacing_sizes = array_merge( $below_sizes, $above_sizes );
$spacing_sizes = $below_sizes;
foreach ( $above_sizes as $above_sizes_item ) {
$spacing_sizes[] = $above_sizes_item;
}

// If there are 7 or less steps in the scale revert to numbers for labels instead of t-shirt sizes.
if ( $spacing_scale['steps'] <= 7 ) {
Expand Down