Skip to content

Conversation

@ramonjd
Copy link
Member

@ramonjd ramonjd commented Sep 19, 2022

What?

Update to #44248

Let's make sure we prioritize the incoming prettify option over the SCRIPT_DEBUG setting.

Context: WordPress/wordpress-develop#3273

Why?

If $options['prettify'] is set we want to use it, even if false. The || operand did not permit this.

How?

Check for the option using isset and use it if found.

Testing Instructions

Run npm run test:unit:php /var/www/html/wp-content/plugins/gutenberg/phpunit/script-loader.php

Let's make sure we prioritize the incoming prettify option over the SCRIPT_DEBUG setting.
@ramonjd ramonjd self-assigned this Sep 19, 2022
@ramonjd ramonjd added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Sep 19, 2022
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Great catch, thanks for the follow-up @ramonjd! I hadn't tested both of these settings together in the previous PR 😅

✅ Tested that the default is still not to prettify
✅ Tested that setting SCRIPT_DEBUG to true switches on prettify
✅ Tested that with SCRIPT_DEBUG set to true, passing options['prettify'] set to false overrides the SCRIPT_DEBUG setting and forces the prettification to be switched off.

LGTM! 🎉

@ramonjd ramonjd merged commit 394644e into trunk Sep 19, 2022
@ramonjd ramonjd deleted the update/script-loader-block-supports-prettify branch September 19, 2022 22:03
@github-actions github-actions bot added this to the Gutenberg 14.2 milestone Sep 19, 2022
@ramonjd ramonjd added the Backport to WP 6.9 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 19, 2022
@ndiego ndiego removed the Backport to WP 6.9 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 26, 2022
@ockham
Copy link
Contributor

ockham commented Sep 26, 2022

We've removed the label since this change is already in Core.

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

Labels

Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

5 participants