Skip to content

Checks all possible theme directories for is_writable#3245

Open
thijsoo wants to merge 1 commit intoWordPress:trunkfrom
thijsoo:50018-check-if-all-theme-directories-are-writable
Open

Checks all possible theme directories for is_writable#3245
thijsoo wants to merge 1 commit intoWordPress:trunkfrom
thijsoo:50018-check-if-all-theme-directories-are-writable

Conversation

@thijsoo
Copy link
Copy Markdown

@thijsoo thijsoo commented Sep 14, 2022

I made sure all themes in the wp_theme_directories global are checked for wp_is_writable
Trac ticket: https://core.trac.wordpress.org/ticket/50018


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.

Comment on lines +344 to +345
foreach ( $wp_theme_directories as $wp_directory_listing ) {
$is_writable_template_directory = wp_is_writable( $wp_directory_listing );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
foreach ( $wp_theme_directories as $wp_directory_listing ) {
$is_writable_template_directory = wp_is_writable( $wp_directory_listing );
foreach ( $wp_theme_directories as $wp_theme_directory ) {
$is_writable_template_directory = wp_is_writable( $wp_theme_directory );

@Clorith
Copy link
Copy Markdown
Member

Clorith commented Sep 18, 2022

Thank you for the patch @thijsoo !

In addition to the suggested variable naming from @costdev, there's a few things we need to keep track of, the current proposed solution would set the value of $is_writable_template_directory to the status of the very last directory being checked, so instead it may be an idea to instead show each available directory, and if they are writable or not, as the value (this can be passed as an array, some pseudo-code for you below to demonstrate)

$is_writable_value = [];
$is_writable_debug = [];
foreach ( $theme_directories as $theme_directory ) {
    $is_writable_value[] = sprintf(
        '%s: %s',
        $theme_directory,
        ( wp_is_writable( $theme_directory ) ? __( 'Writable' ) : __( 'Not writable' ) )
    );

    $is_writable_debug[] = sprintf(
        '%s: %s',
        $theme_directory,
        ( wp_is_writable( $theme_directory ) ? 'writable' : 'not writable' )
    );
}

(Note that it needs two variables, as the debug value is never translated, for consistency between languages in debug data.)

The label for "The themes directory" should also be updated to use the _n() translation function, so that it can account for if there is only one, or multiple theme directories available.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants