work with Wordpress UPLOADS constant

[xavierpriour]

Wordpress lets you define a custom uploads directory by setting the UPLOADS constant (http://codex.wordpress.org/Editing_wp-config.php#Moving_uploads_folder).
The existing implementation of "resize" did not take that case into account and generated incorrect URL for the files, resulting in broken img links.
To fix it, I rewrote the function, then also updated the other image manipulation filters (retina, img_to_jpg and letterbox) to handle that case as well.

In the end, I significantly refactored TimberImageHelper to focus on comprehensive directory handling, and moved image manipulation to dedicated class hierarchy (under TimberImageOperation).

[jarednova]

Hi @xavierpriour, thanks for the PR! There's a lot to go through, so it might take me a few days. To start us off can you take a look at resolving the errors in PHP 5.3?:

https://travis-ci.org/jarednova/timber/builds/48849954

... PHP 5.3 doesn't support accessing array properties from a method so...

$path = wp_uploads_dir()['basedir'];

Should become:

$wp_uploads_dir = wp_uploads_dir();
$path = $wp_uploads_dir['basedir'];

The other thing that would be very helpful is to provide a docblock for each new class you've added so I can make sure I understand the intent as I go through it. (This is something I want to do with existing classes too, but good to start somewhere). The WordPress docs have a little more background here.

And a few others like that. Once that's clear I'll dive more

[xavierpriour]

Hi @jarednova,

Thanks for the follow-up, I'll work on the class docs.

On the PHP 5.3, any chance I could convince you to switch to requiring PHP
5.5 only, as 5.3 is now officially completely out of support (
http://php.net/supported-versions.php), and even 5.4 is now security fix
only?
This would be in line with Long term support considered harmful.

On Fri, Feb 6, 2015 at 5:01 PM, Jared Novack notifications@github.com
wrote:

Hi @xavierpriour https://github.com/xavierpriour, thanks for the PR!
There's a lot to go through, so it might take me a few days. To start us
off can you take a look at resolving the errors in PHP 5.3?:

https://travis-ci.org/jarednova/timber/builds/48849954

... PHP 5.3 doesn't support accessing array properties from a method so...

$path = wp_uploads_dir()['basedir'];

Should become:

$wp_uploads_dir = wp_uploads_dir();$path = $wp_uploads_dir['basedir'];

The other thing that would be very helpful is to provide a docblock for
each new class you've added so I can make sure I understand the intent as I
go through it. (This is something I want to do with existing classes too,
but good to start somewhere). The WordPress docs
https://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/
have a little more background here.

And a few others like that. Once that's clear I'll dive more

—
Reply to this email directly or view it on GitHub
#457 (comment).

Xavier Priour

[jnweaver]

Just to jump in on the PHP version support, we're in a position of having to run RedHat Enterprise Linux 6 on our VMs. RHEL 6 packages PHP 5.3 (with security patch back ports as needed).

While I really wish we were not tied to 5.3, it's the reality in our organization. I'm sure most people developing for Wordpress are not doing so under RHEL or other enterprise Linux (where packages are locked in for the long haul), but it's important to keep in mind that there are some of us.

That and Wordpress still officially supports 5.2.4 or later (though they recommend 5.4 or later). https://wordpress.org/about/requirements/

[xavierpriour]

Okay @jnweaver @jarednova - guess it has to be 5.3 then...

Would you know of an (easy) way to setup a PHP 5.3 Timber test env?
I'm on 5.5, and the suggested Timber test env (https://github.com/jarednova/timber/wiki/Unit-testing) also installs 5.5.

[jarednova]

@xavierpriour there's some discussion here about switching PHP versions in VVV (conclusion: not a part of the software). So the best bet seems to be using Travis to report issues and fixing (and then pushing back to your branch to re-test). You can see the builds you submit listed here:

https://travis-ci.org/jarednova/timber/pull_requests

... I've found that you can solve 99% of the problems with this:

Arrays from methods

PHP 5.4

$foo = get_my_bar()['foo'];

PHP 5.3

$bar = get_my_bar();
$foo = $bar['foo'];

Declaring arrays

PHP 5.4

$foo = [];

PHP 5.3

$foo = array();

$this in Closures

PHP 5.4

add_filter('my_foo', function() {
    $this->bar = 'quack';
});

PHP 5.3

add_filter('my_foo', function() use ($this) {
    $this->bar = 'quack';
});

[xavierpriour]

All done, build passing, thanks a lot for the 5.3 tips @jarednova

[jarednova]

@xavierpriour: The TimberImage functions feel SO MUCH BETTER. I did make some changes, mainly just separating each class into its own file. Once that was complete, I added some further docs and merged into master. I'm going to push this to WP.org next week. Thanks again!

[xavierpriour]

Glad I could help.

I see you moved is_absolute to URLHelper, which is a great thing - you might want to move TimberImageHelper::is_external as well, while you're at it.

Also, I realize the delete code is still in TimberImageHelper, while I believe it belongs in the TimberImageOperation classes (case in point, I think retina images are never deleted).

Want me to submit a PR in that sense?