wecodemore / wpstarter

Easily bootstrap whole site Composer packages for WordPress.
https://wecodemore.github.io/wpstarter/
MIT License
244 stars 34 forks source link

Register additional theme directory instead of moving content #2

Closed gmazzap closed 9 years ago

gmazzap commented 9 years ago

Current version of WP Starter asks to move default wp-content directory from WordPress package folder to project content folder.

Reason is that way WordPress is ready-to use just immediately after is WP Starter finished its work, without having to require default themes in composer.json that are already shipped with WordPress package.

The downside of this is that composer update will update themes inside WP package folder, but not the moved themes.

Move again theme on update is surely possible, but there is a simpler and probably better solution.

WordPress allows the presence of more directories where to look for themes, in fact, theme directories can be added via register_theme_directory() function.

If we add that function to the generated wp-config.php we don't need to move themes and in case of update default themes will stay updated as well.

As a bonus, without moving themes we can remove a question from installation routine (less complexity) and speed of entire process increase as well.

We probably should raise main version of the package if this feature is implemented, because backward compatibility breaks.

Opinions?

franz-josef-kaiser commented 9 years ago

My personal opinion is this: I don't care having the wp-content directory twice. To me moving it is a lot of effort for something that shouldn't be cared in this repo. I see the place for a possible solution (if there even is one needed) on the opposite site: The place from where we fetch the repo.

Maybe we should ask on Twitter and other channels for more opinions and only care about that if enough people think that this is an absolute must.

gmazzap commented 9 years ago

To me, this package aim can be summarized in:

Make the process that goes from a composer.json file to a fully working Composer-managed WordPress site in less steps as possibile, and with less efforts as possibile.

Once WordPress needs a theme to work, the reason why, at the moment, WP Starter moves content folder is not to avoid the content folder exists twice (I don't care it as well), but to make possible to just visit the site and see it working immediately after setup finished.

Surely this can be done requiring default themes from wpackagist, but it means

  1. to add wpackagist to repositories settings
  2. to add a theme to require settings from wpackagist
  3. to set WP_DEFAULT_THEME in .env file to match the theme in require. This is necessary because default theme change form a WP version to another and if WP version constraint (hopefully) is something like ">=4.1" is possible that the theme set as default is different from the one required, if we don't use this setting

(Actually step 1 can be avoided if the theme in require is a custom theme available on packagist).

None of these 3 steps is hard to do, but to be consistent with the "less steps as possibile, and with less efforts as possibile" I think we can save users from these 3 steps considering that themes are already there.

E.g. let's assume I want to use WP Starter to quickly create a vanilla WP install to test a plugin (something I'll delete in few hours or maybe minutes). I don't care about the theme (but probably I want a default one to test the plugin), why have I to apply the 3 steps?

This is why I'm not agree that this is something "shouldn't be cared in this repo".

On the countrary, I'm agree that move things and ask a question during install is too much effort for it and this is why I opened this issue.

Actually, the solution proposed solve the problem, does not require any additional step, does not move any file nor ask question during install. I don't see any other downsides as well... Do you? (is not a rhetorical question).

Note that for more advanced installations where a custom theme is required, and probably that theme is added to WP_DEFAULT_THEME setting, the proposed solution doesn't require any step to be disabled, nor interfere in any way...

franz-josef-kaiser commented 9 years ago

Currently I use a mu-plugin (written by @toscho ) to register an additional theme directory:

<?php
/* Plugin Name: Local Theme Roots */

/**
 * Create a custom theme directory on a custom domain.
 * Best used as mu plugin in a local development stack.
 * @param  string $root URL or path
 * @return string
 */

add_filter( 'theme_root_uri', 'switch_theme_root_local' );
add_filter( 'theme_root',     'switch_theme_root_local' );
function switch_theme_root_local( $root )
{
    if ( 0 !== stripos( strrev( $_SERVER['HTTP_HOST'] ), 'ved.' ) )
        return $root;

    // URl/URI handling
    if ( 'theme_root_uri' === current_filter() )
        return 'http://'.$_SERVER['HTTP_HOST'].'/wp-content/themes';

    // Path handling
    global $wp_theme_directories;

    // If we made it so far we are in the 'theme_root' filter.
    $new_root = dirname( __DIR__ ).'/themes';

    if ( ! isset( $wp_theme_directories ) )
        $wp_theme_directories = [ dirname( __DIR__ ).'/../wp/wp-content/themes/', ];

    // Too early to use register_theme_directory()
    if (
        isset( $wp_theme_directories )
        AND ! in_array( $new_root, $wp_theme_directories )
    )
        $wp_theme_directories[] = $new_root;

    return $new_root;
}

This allows me to simply ignore most of the problems you mentioned.

What I find tempting is the word "advanced" that you used above. Maybe switching between a "default" no-brainer and no-human mode and an "advanced" mode would be a viable solution.

If we add that function to the generated wp-config.php we don't need to move themes and in case of update default themes will stay updated as well.

If I would only understand how this should work... Do you have some snippet to discuss?

gmazzap commented 9 years ago

A Preface

As you know, WP Starter currently as a MU plugins loader. It allows to load MU plugins in subfolders. Reason is that if you use Composer to load MU plugins, they are placed inside a subfolder and so WP doesn't recognize them. Traditionally a MU plugin is used to load other MU plugins in subfolders, because anything else is too late. But using Composer we can't load that MU plugin... it's like an infinite loop. I solved the problem adding a loader to wp-config.php.

To do that, I added a WordPress hook before add_action is available. That is done via the Helper::addHook() method.

Register additional theme folder

Do you have some snippet to discuss?

Yes. Immagine to add following code after the line that loads MU plugins in our wp-config.php:

WCM\WPStarter\Helpers::addHook(
    'plugins_loaded',
    function() {
        register_theme_directory (__DIR__.'/{{{WP_INSTALL_PATH}}}/wp-content/themes');
    }
);

where, of course, {{{WP_INSTALL_PATH}}} is replaced on installation with the WordPress subfolder.

With this one liner we are able to make WordPress properly recognize themes that are placed inside WP package folder. Fully core compatible, no effort, zero problems.

franz-josef-kaiser commented 9 years ago

As you know, WP Starter currently as a MU plugins loader.

Yes, I saw that at the end of wp-config.php.

With this one liner we are able to make WordPress properly recognize themes that are placed inside WP package folder. Fully core compatible, no effort, zero problems.

and a :+1: from me.

gmazzap commented 9 years ago

Ok, I lied. Everytime I say "no problem" WordPress seems to find a way to contradict me.

Due to this the theme folder registration may be vanished if the subfolder /themes does not exist in project wp-content folder.

Should we create the folder /themes if it does not exist to overcome this issue?

franz-josef-kaiser commented 9 years ago

Hm. I haven't actually seen that before as I really always use wpackagist to add default "Twenty*" theme at least. So :+1: for creating it.

gmazzap commented 9 years ago

Discussed workflow has been implemented in v2.0