Review Chisel/WpExtensions.php

[jakub300]

For example documentation for add_theme_support says that Must be called in the theme’s functions.php file to work. If attached to a hook, it must be ‘after_setup_theme’. The ‘init’ hook may be too late for some features. and we are actually calling it in init hook.

[luboskmetko]

In such case we could get rid off the whole extend method and just do:

class WpExtensions {
	public function __construct() {
		add_theme_support( 'post-formats' );
		add_theme_support( 'post-thumbnails' );
		add_theme_support( 'menus' );
		add_action( 'init', array( $this, 'registerPostTypes' ) );
		add_action( 'init', array( $this, 'registerTaxonomies' ) );
	}

	/**
	 * Use this method to register custom post types
	 */
	public function registerPostTypes() {
	}

	/**
	 * Use this method to register custom taxonomies
	 */
	public function registerTaxonomies() {
	}
}

What you think?

[JakubSzajna]

I'd like to make the methods as short as possible to extend readability of the code. What about methods like extend() called directly in constructor, and init() called via init hook defined in constructor? Then we can add a note about that to the documentation.

[luboskmetko]

Can you show an example how it would look like?

[JakubSzajna]

class WpExtensions {
	public function __construct() {
                $this->extend()
		add_action( 'init', array( $this, 'init' ) );
	}

        private function extend() {
		add_theme_support( 'post-formats' );
		add_theme_support( 'post-thumbnails' );
		add_theme_support( 'menus' );
        }

        public function init() {
                $this->registerPostTypes();
                $this->registerTaxonomies();
        }

	/**
	 * Use this method to register custom post types
	 */
	public function registerPostTypes() {
	}

	/**
	 * Use this method to register custom taxonomies
	 */
	public function registerTaxonomies() {
	}
}

Maybe something like that? There is not much code, but if you'd stack like 15 or more methods in the constructor it gets really messy there. Doing it like this you can leave a constructor untouched during development.

We could also split it more or describe how to split this file into subclasses, line inside namespace Chisel\Extensions, where we could add all extensions. I didn't do that when I was creating this structure because I thought that making this structure more complicated simply does not make sense for smaller projects. But Chisel gets bigger and bigger and eventually we will have to do that.

[jakub300]

According to Code Reference register_taxonomy and register_post_type should not be called before init so definitely they have to be separated from add_theme_support.

https://developer.wordpress.org/reference/functions/register_post_type/
https://developer.wordpress.org/reference/functions/register_taxonomy/

[luboskmetko]

@JakubSzajna I'm ok with this, looks good. Would you create a PR with that? Thanks