From c26f6bb906d6f65e632e67ff60d6d104f1e8c944 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 May 2018 16:39:33 +0200 Subject: [PATCH 1/3] ColourUtil tests and small refactors --- src/CliMenuBuilder.php | 4 +- src/MenuStyle.php | 19 ++--- src/Util/ColourUtil.php | 40 ++++++----- test/MenuStyleTest.php | 19 +---- test/Util/ColourUtilTest.php | 136 +++++++++++++++++++++++++++++++++++ 5 files changed, 174 insertions(+), 44 deletions(-) create mode 100644 test/Util/ColourUtilTest.php diff --git a/src/CliMenuBuilder.php b/src/CliMenuBuilder.php index 07ef41be..f099c4fd 100644 --- a/src/CliMenuBuilder.php +++ b/src/CliMenuBuilder.php @@ -199,7 +199,7 @@ public function setExitButtonText(string $exitButtonText) : self return $this; } - public function setBackgroundColour($colour, string $fallback = null) : self + public function setBackgroundColour(string $colour, string $fallback = null) : self { $this->style['bg'] = ColourUtil::validateColour( $this->terminal, @@ -210,7 +210,7 @@ public function setBackgroundColour($colour, string $fallback = null) : self return $this; } - public function setForegroundColour($colour, string $fallback = null) : self + public function setForegroundColour(string $colour, string $fallback = null) : self { $this->style['fg'] = ColourUtil::validateColour( $this->terminal, diff --git a/src/MenuStyle.php b/src/MenuStyle.php index b59f4db0..5b28da2e 100644 --- a/src/MenuStyle.php +++ b/src/MenuStyle.php @@ -20,12 +20,12 @@ class MenuStyle protected $terminal; /** - * @var int|string + * @var string */ protected $fg; /** - * @var int|string + * @var string */ protected $bg; @@ -172,8 +172,11 @@ public function __construct(Terminal $terminal = null) { $this->terminal = $terminal ?: TerminalFactory::fromSystem(); - $this->setFg(static::$defaultStyleValues['fg']); - $this->setBg(static::$defaultStyleValues['bg']); + $this->fg = static::$defaultStyleValues['fg']; + $this->bg = static::$defaultStyleValues['bg']; + + $this->generateColoursSetCode(); + $this->setWidth(static::$defaultStyleValues['width']); $this->setPadding(static::$defaultStyleValues['padding']); $this->setMargin(static::$defaultStyleValues['margin']); @@ -199,13 +202,13 @@ public function getDisabledItemText(string $text) : string */ private function generateColoursSetCode() : void { - if (is_string($this->fg)) { + if (!is_numeric($this->fg)) { $fgCode = self::$availableForegroundColors[$this->fg]; } else { $fgCode = sprintf("38;5;%s", $this->fg); } - if (is_string($this->bg)) { + if (!is_numeric($this->bg)) { $bgCode = self::$availableBackgroundColors[$this->bg]; } else { $bgCode = sprintf("48;5;%s", $this->bg); @@ -259,7 +262,7 @@ public function getFg() return $this->fg; } - public function setFg($fg, string $fallback = null) : self + public function setFg(string $fg, string $fallback = null) : self { $this->fg = ColourUtil::validateColour( $this->terminal, @@ -276,7 +279,7 @@ public function getBg() return $this->bg; } - public function setBg($bg, string $fallback = null) : self + public function setBg(string $bg, string $fallback = null) : self { $this->bg = ColourUtil::validateColour( $this->terminal, diff --git a/src/Util/ColourUtil.php b/src/Util/ColourUtil.php index 67a38b45..efb6fb5e 100644 --- a/src/Util/ColourUtil.php +++ b/src/Util/ColourUtil.php @@ -284,7 +284,7 @@ class ColourUtil 255 => 'white', ]; - public static function getDefaultColoursNames() : array + public static function getDefaultColourNames() : array { return static::$defaultColoursNames; } @@ -296,7 +296,7 @@ public static function getDefaultColoursNames() : array public static function map256To8(int $colourCode) : string { if (!isset(static::$coloursMap[$colourCode])) { - throw new \InvalidArgumentException("Invalid colour code"); + throw new \InvalidArgumentException('Invalid colour code'); } return static::$coloursMap[$colourCode]; @@ -306,22 +306,28 @@ public static function map256To8(int $colourCode) : string * Check if $colour exists * If it's a 256-colours code and $terminal doesn't support it, returns a fallback value */ - public static function validateColour(Terminal $terminal, $colour, string $fallback = null) + public static function validateColour(Terminal $terminal, string $colour, string $fallback = null) : string { - if (is_int($colour)) { - if ($colour < 0 || $colour > 255) { - throw new \InvalidArgumentException("Invalid colour code"); - } - if ($terminal->getColourSupport() < 256) { - if ($fallback !== null) { - Assertion::inArray($fallback, static::getDefaultColoursNames()); - return $fallback; - } - return static::map256To8($colour); - } - } else { - Assertion::inArray($colour, static::getDefaultColoursNames()); + if (!is_numeric($colour)) { + return static::validateColourName($colour); } - return $colour; + + Assertion::between($colour, 0, 255, 'Invalid colour code'); + + if ($terminal->getColourSupport() >= 256) { + return $colour; + } + + if ($fallback !== null) { + return static::validateColourName($fallback); + } + + return static::map256To8($colour); + } + + private static function validateColourName(string $colourName) : string + { + Assertion::inArray($colourName, static::getDefaultColourNames()); + return $colourName; } } diff --git a/test/MenuStyleTest.php b/test/MenuStyleTest.php index ca861b12..ef533140 100644 --- a/test/MenuStyleTest.php +++ b/test/MenuStyleTest.php @@ -74,21 +74,6 @@ public function testMenuStyleCanBeInstantiatedByCliMenuBuilder() : void static::assertSame(MenuStyle::class, get_class($style)); } - public function testAvailableColours() : void - { - static::assertSame([ - 'black', - 'red', - 'green', - 'yellow', - 'blue', - 'magenta', - 'cyan', - 'white', - 'default' - ], ColourUtil::getDefaultColoursNames()); - } - public function testGetColoursSetCode() : void { static::assertSame("\e[37;44m", $this->getMenuStyle()->getColoursSetCode()); @@ -152,8 +137,8 @@ public function test256ColoursCodes() : void $style = $this->getMenuStyle(256); $style->setBg(16, 'white'); $style->setFg(206, 'red'); - static::assertSame(16, $style->getBg()); - static::assertSame(206, $style->getFg()); + static::assertSame('16', $style->getBg()); + static::assertSame('206', $style->getFg()); static::assertSame("\033[38;5;206;48;5;16m", $style->getColoursSetCode()); $style = $this->getMenuStyle(8); diff --git a/test/Util/ColourUtilTest.php b/test/Util/ColourUtilTest.php new file mode 100644 index 00000000..dbe2afa8 --- /dev/null +++ b/test/Util/ColourUtilTest.php @@ -0,0 +1,136 @@ + + */ +class ColourUtilTest extends TestCase +{ + public function testAvailableColours() : void + { + self::assertSame( + [ + 'black', + 'red', + 'green', + 'yellow', + 'blue', + 'magenta', + 'cyan', + 'white', + 'default' + ], + ColourUtil::getDefaultColourNames() + ); + } + + public function testMap256To8ThrowsExceptionIfCodeNotValid() : void + { + self::expectException(\InvalidArgumentException::class); + self::expectExceptionMessage('Invalid colour code'); + + ColourUtil::map256To8(512); + } + + public function testMap256To8() : void + { + self::assertEquals('white', ColourUtil::map256To8(255)); + self::assertEquals('magenta', ColourUtil::map256To8(213)); + self::assertEquals('yellow', ColourUtil::map256To8(143)); + self::assertEquals('blue', ColourUtil::map256To8(103)); + self::assertEquals('green', ColourUtil::map256To8(64)); + } + + public function testValidateColourThrowsExceptionIfColourNot256AndNot8() : void + { + self::expectException(\Assert\InvalidArgumentException::class); + + $terminal = $this->createMock(Terminal::class); + + ColourUtil::validateColour($terminal, 'teal'); + } + + public function testValidateColourThrowsExceptionIfFallbackNotValidWhenTerminalDoesNotSupport256Colours() : void + { + self::expectException(\Assert\InvalidArgumentException::class); + + $terminal = $this->createMock(Terminal::class); + $terminal->expects($this->once()) + ->method('getColourSupport') + ->willReturn(8); + + ColourUtil::validateColour($terminal, 255, 'teal'); + } + + public function testValidateColourWithFallbackWhenTerminalDoesNotSupport256Colours() : void + { + $terminal = $this->createMock(Terminal::class); + $terminal->expects($this->once()) + ->method('getColourSupport') + ->willReturn(8); + + self::assertEquals('red', ColourUtil::validateColour($terminal, 255, 'red')); + } + + public function testValidateColourPicksFallbackFromPreComputedListWhenTerminalDoesNotSupport256Colours() : void + { + $terminal = $this->createMock(Terminal::class); + $terminal->expects($this->once()) + ->method('getColourSupport') + ->willReturn(8); + + self::assertEquals('yellow', ColourUtil::validateColour($terminal, 148)); + } + + /** + * @dataProvider invalidColourCodeProvider + */ + public function testValidateColourThrowsExceptionIfInvalid256ColourCodeUsed(int $colourCode) : void + { + self::expectException(\Assert\InvalidArgumentException::class); + + ColourUtil::validateColour($this->createMock(Terminal::class), $colourCode); + } + + public function invalidColourCodeProvider() : array + { + return [ + [-1], + [256], + [1000], + ]; + } + + /** + * @dataProvider validColourCodeProvider + */ + public function testValidateColourWith256ColoursWhenTerminalSupports256Colours(int $colourCode) : void + { + $terminal = $this->createMock(Terminal::class); + $terminal->expects($this->once()) + ->method('getColourSupport') + ->willReturn(256); + + self::assertEquals($colourCode, ColourUtil::validateColour($terminal, $colourCode)); + } + + public function validColourCodeProvider() : array + { + return [ + [0], + [255], + [1], + [100], + ]; + } + + public function testValidateColourWithValid8ColourName() : void + { + self::assertEquals('red', ColourUtil::validateColour($this->createMock(Terminal::class), 'red')); + } +} From 4049d03bb8d2f6a50dba342c0ba35ba8f3cae9b8 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 May 2018 16:45:36 +0200 Subject: [PATCH 2/3] CS --- src/Util/ColourUtil.php | 2 +- test/Util/ColourUtilTest.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Util/ColourUtil.php b/src/Util/ColourUtil.php index efb6fb5e..dd2fc21d 100644 --- a/src/Util/ColourUtil.php +++ b/src/Util/ColourUtil.php @@ -322,7 +322,7 @@ public static function validateColour(Terminal $terminal, string $colour, string return static::validateColourName($fallback); } - return static::map256To8($colour); + return static::map256To8((int) $colour); } private static function validateColourName(string $colourName) : string diff --git a/test/Util/ColourUtilTest.php b/test/Util/ColourUtilTest.php index dbe2afa8..74511c04 100644 --- a/test/Util/ColourUtilTest.php +++ b/test/Util/ColourUtilTest.php @@ -100,9 +100,9 @@ public function testValidateColourThrowsExceptionIfInvalid256ColourCodeUsed(int public function invalidColourCodeProvider() : array { return [ - [-1], - [256], - [1000], + [-1], + [256], + [1000], ]; } From 5a07d01c50252c22155276f1f4448a7c9dd73ee2 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 May 2018 16:59:01 +0200 Subject: [PATCH 3/3] Use ctype_digit instead of is_numeric --- src/MenuStyle.php | 4 ++-- src/Util/ColourUtil.php | 2 +- test/CliMenuBuilderTest.php | 2 +- test/MenuStyleTest.php | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/MenuStyle.php b/src/MenuStyle.php index 5b28da2e..da67741c 100644 --- a/src/MenuStyle.php +++ b/src/MenuStyle.php @@ -202,13 +202,13 @@ public function getDisabledItemText(string $text) : string */ private function generateColoursSetCode() : void { - if (!is_numeric($this->fg)) { + if (!ctype_digit($this->fg)) { $fgCode = self::$availableForegroundColors[$this->fg]; } else { $fgCode = sprintf("38;5;%s", $this->fg); } - if (!is_numeric($this->bg)) { + if (!ctype_digit($this->bg)) { $bgCode = self::$availableBackgroundColors[$this->bg]; } else { $bgCode = sprintf("48;5;%s", $this->bg); diff --git a/src/Util/ColourUtil.php b/src/Util/ColourUtil.php index dd2fc21d..3ad11e20 100644 --- a/src/Util/ColourUtil.php +++ b/src/Util/ColourUtil.php @@ -308,7 +308,7 @@ public static function map256To8(int $colourCode) : string */ public static function validateColour(Terminal $terminal, string $colour, string $fallback = null) : string { - if (!is_numeric($colour)) { + if (!ctype_digit($colour)) { return static::validateColourName($colour); } diff --git a/test/CliMenuBuilderTest.php b/test/CliMenuBuilderTest.php index 72f05dbd..15b15440 100644 --- a/test/CliMenuBuilderTest.php +++ b/test/CliMenuBuilderTest.php @@ -145,7 +145,7 @@ public function testSetBgThrowsExceptionWhenColourCodeIsNotInRange() : void $builder = new CliMenuBuilder; $builder->setTerminal($terminal); - $builder->setBackgroundColour(-5, 'white'); + $builder->setBackgroundColour(257, 'white'); } public function testDisableDefaultItems() : void diff --git a/test/MenuStyleTest.php b/test/MenuStyleTest.php index ef533140..005b673a 100644 --- a/test/MenuStyleTest.php +++ b/test/MenuStyleTest.php @@ -164,7 +164,7 @@ public function testSetBgThrowsExceptionWhenColourCodeIsNotInRange() : void $this->expectExceptionMessage('Invalid colour code'); $style = $this->getMenuStyle(256); - $style->setBg(-5, 'white'); + $style->setBg(257, 'white'); } public function testGetMarkerReturnsTheCorrectMarkers() : void