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.

Sign up for GitHub

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

Jump to bottom

[CMSP-899] Remove our global for the upload_dir infinite loop #34

Merged
merged 1 commit into from Mar 4, 2024
Merged

[CMSP-899] Remove our global for the upload_dir infinite loop #34

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 4 additions & 13 deletions inc/fonts.php
View file
Open in desktop
Expand Up @@ -7,16 +7,6 @@

namespace Pantheon\Fonts;

/**
* Store the value of wp_get_upload_dir() in a global variable.
* This is to resolve an infinite loop when wp_get_upload_dir is used inside
* our filter of font_dir (because upload_dir is also being filtered).
*
* @var array $wp_upload_dir The value of wp_get_upload_dir().
* @see https://developer.wordpress.org/reference/functions/wp_get_upload_dir/
*/
$_pantheon_upload_dir = wp_get_upload_dir(); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable

/**
* Kick off our customizations to the WP_Font_Library.
*/
Expand All @@ -33,10 +23,11 @@ function bootstrap() {
* @param array $defaults The default settings for the font directory.
*/
function pantheon_font_dir( $defaults ) {
global $_pantheon_upload_dir;
$upload_dir = wp_get_upload_dir();

// Set our font directory.
$font_dir = $_pantheon_upload_dir['basedir'] . '/fonts';
$font_url = $_pantheon_upload_dir['baseurl'] . '/fonts';
$font_dir = $upload_dir['basedir'] . '/fonts';
$font_url = $upload_dir['baseurl'] . '/fonts';

Comment on lines +26 to 31
Copy link
Member

@pwtyler pwtyler Mar 4, 2024 •
edited

Choose a reason for hiding this comment

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

Can't make a suggestion on the whole function with the structure of the diff— I took a stab at simplifying but I'm swinging back and forth over here whether the following makes it less readable. Apply if you wish.

function pantheon_font_dir( $defaults ) {
	$upload_dir = wp_get_upload_dir();

	// Set our font directory.
	$defaults['basedir'] = $upload_dir['basedir'] . '/fonts';
	$defaults['baseurl'] = $upload_dir['baseurl'] . '/fonts';

	$defaults['path'] = $defaults['basedir'];
	$defaults['url'] = $defaults['baseurl'];
	return $defaults;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like defining $font_dir/$font_url and then saving those to the various $defaults values is more explicit about what's going on.

$defaults['path'] = $font_dir;
$defaults['url'] = $font_url;
Expand Down