Add size ratio to the output of wp media image-size#58
Conversation
schlessera
left a comment
There was a problem hiding this comment.
I provided two suggestions to improve maintainability in comments.
Also, the spacing and alignment is completely off, please make sure the formatting adheres to the WordPress Coding Standards.
src/Media_Command.php
Outdated
| $height_ratio = $height / $gcd; | ||
| $image_ratio = "{$width_ratio}:{$height_ratio}"; | ||
| } | ||
| return $image_ratio; |
There was a problem hiding this comment.
You should prefer early returns instead of else clauses.
Something like this:
if ( $height == 0 ) {
return "0:{$width}";
}
if ( $width == 0 ) {
return "{$height}:0";
}
$gcd = $this->gcd( $width, $height );
$width_ratio = $width / $gcd;
$height_ratio = $height / $gcd;
return "{$width_ratio}:{$height_ratio}";
There was a problem hiding this comment.
Thanks @schlessera for the quick review! I'll take a look tomorrow and adjust :)
src/Media_Command.php
Outdated
| 'width' => intval( get_option( 'large_size_w' ) ), | ||
| 'height' => intval( get_option( 'large_size_h' ) ), | ||
| 'crop' => false !== get_option( 'large_crop' ) ? 'hard' : 'soft', | ||
| 'ratio' => false !== get_option( 'large_crop' ) ? $this->get_ratio( intval( get_option( 'large_size_w' ) ), intval( get_option( 'large_size_h' ) ) ) : 'N/A', |
There was a problem hiding this comment.
As we're using the string 'N/A' multiple times here, you should put it into a constant and refer to the constant instead.
This way, if we want to change the value to display for these cases, we can do it consistently and without running the risk of missing one or more instances.
There was a problem hiding this comment.
you should put it into a constant and refer to the constant instead.
Variable — constants are globally scoped.
|
@schlessera @danielbachhuber comments addressed, and formatting fixed as well. |
|
Thanks @runofthemill ! |
…size-output Add size ratio to the output of wp media image-size
Fixes #56