Skip to content

Commit 93bc65e

Browse files
kenjiskenjis
authored
Merge pull request #5893 from kenjis/fix-cli-color
fix: using multiple CLI::color() in CLI::write() outputs strings with wrong color
2 parents 491fe59 + c0ca99f commit 93bc65e

File tree

4 files changed

+141
-50
lines changed

4 files changed

+141
-50
lines changed

‎system/CLI/CLI.php‎

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ public static function clearScreen()
469469
*/
470470
public static function color(string $text, string $foreground, ?string $background = null, ?string $format = null): string
471471
{
472-
if (! static::$isColored) {
472+
if (! static::$isColored || $text === '') {
473473
return $text;
474474
}
475475

@@ -481,6 +481,48 @@ public static function color(string $text, string $foreground, ?string $backgrou
481481
throw CLIException::forInvalidColor('background', $background);
482482
}
483483

484+
$newText = '';
485+
486+
// Detect if color method was already in use with this text
487+
if (strpos($text, "\033[0m") !== false) {
488+
$pattern = '/\\033\\[0;.+?\\033\\[0m/u';
489+
490+
preg_match_all($pattern, $text, $matches);
491+
$coloredStrings = $matches[0];
492+
493+
// No colored string found. Invalid strings with no `\033[0;??`.
494+
if ($coloredStrings === []) {
495+
return $newText . self::getColoredText($text, $foreground, $background, $format);
496+
}
497+
498+
$nonColoredText = preg_replace(
499+
$pattern,
500+
'<<__colored_string__>>',
501+
$text
502+
);
503+
$nonColoredChunks = preg_split(
504+
'/<<__colored_string__>>/u',
505+
$nonColoredText
506+
);
507+
508+
foreach ($nonColoredChunks as $i => $chunk) {
509+
if ($chunk !== '') {
510+
$newText .= self::getColoredText($chunk, $foreground, $background, $format);
511+
}
512+
513+
if (isset($coloredStrings[$i])) {
514+
$newText .= $coloredStrings[$i];
515+
}
516+
}
517+
} else {
518+
$newText .= self::getColoredText($text, $foreground, $background, $format);
519+
}
520+
521+
return $newText;
522+
}
523+
524+
private static function getColoredText(string $text, string $foreground, ?string $background, ?string $format): string
525+
{
484526
$string = "\033[" . static::$foreground_colors[$foreground] . 'm';
485527

486528
if ($background !== null) {
@@ -491,30 +533,6 @@ public static function color(string $text, string $foreground, ?string $backgrou
491533
$string .= "\033[4m";
492534
}
493535

494-
// Detect if color method was already in use with this text
495-
if (strpos($text, "\033[0m") !== false) {
496-
// Split the text into parts so that we can see
497-
// if any part missing the color definition
498-
$chunks = mb_split('\\033\\[0m', $text);
499-
// Reset text
500-
$text = '';
501-
502-
foreach ($chunks as $chunk) {
503-
if ($chunk === '') {
504-
continue;
505-
}
506-
507-
// If chunk doesn't have colors defined we need to add them
508-
if (strpos($chunk, "\033[") === false) {
509-
$chunk = static::color($chunk, $foreground, $background, $format);
510-
// Add color reset before chunk and clear end of the string
511-
$text .= rtrim("\033[0m" . $chunk, "\033[0m");
512-
} else {
513-
$text .= $chunk;
514-
}
515-
}
516-
}
517-
518536
return $string . $text . "\033[0m";
519537
}
520538

‎tests/system/CLI/CLITest.php‎

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,21 @@ protected function tearDown(): void
3939
public function testNew()
4040
{
4141
$actual = new CLI();
42+
4243
$this->assertInstanceOf(CLI::class, $actual);
4344
}
4445

4546
public function testBeep()
4647
{
4748
$this->expectOutputString("\x07");
49+
4850
CLI::beep();
4951
}
5052

5153
public function testBeep4()
5254
{
5355
$this->expectOutputString("\x07\x07\x07\x07");
56+
5457
CLI::beep(4);
5558
}
5659

@@ -96,6 +99,7 @@ public function testIsWindows()
9699
public function testNewLine()
97100
{
98101
$this->expectOutputString('');
102+
99103
CLI::newLine();
100104
}
101105

@@ -119,19 +123,21 @@ public function testColorSupportOnNoColor()
119123
{
120124
$nocolor = getenv('NO_COLOR');
121125
putenv('NO_COLOR=1');
122-
123126
CLI::init(); // force re-check on env
127+
124128
$this->assertSame('test', CLI::color('test', 'white', 'green'));
129+
125130
putenv($nocolor ? "NO_COLOR={$nocolor}" : 'NO_COLOR');
126131
}
127132

128133
public function testColorSupportOnHyperTerminals()
129134
{
130135
$termProgram = getenv('TERM_PROGRAM');
131136
putenv('TERM_PROGRAM=Hyper');
132-
133137
CLI::init(); // force re-check on env
138+
134139
$this->assertSame("\033[1;37m\033[42m\033[4mtest\033[0m", CLI::color('test', 'white', 'green', 'underline'));
140+
135141
putenv($termProgram ? "TERM_PROGRAM={$termProgram}" : 'TERM_PROGRAM');
136142
}
137143

@@ -146,72 +152,105 @@ public function testColor()
146152
// After the tests on NO_COLOR and TERM_PROGRAM above,
147153
// the $isColored variable is rigged. So we reset this.
148154
CLI::init();
149-
$this->assertSame("\033[1;37m\033[42m\033[4mtest\033[0m", CLI::color('test', 'white', 'green', 'underline'));
155+
156+
$this->assertSame(
157+
"\033[1;37m\033[42m\033[4mtest\033[0m",
158+
CLI::color('test', 'white', 'green', 'underline')
159+
);
160+
}
161+
162+
public function testColorEmtpyString()
163+
{
164+
$this->assertSame(
165+
'',
166+
CLI::color('', 'white', 'green', 'underline')
167+
);
150168
}
151169

152170
public function testPrint()
153171
{
154172
CLI::print('test');
155-
$expected = 'test';
156173

174+
$expected = 'test';
157175
$this->assertSame($expected, CITestStreamFilter::$buffer);
158176
}
159177

160178
public function testPrintForeground()
161179
{
162180
CLI::print('test', 'red');
163-
$expected = "\033[0;31mtest\033[0m";
164181

182+
$expected = "\033[0;31mtest\033[0m";
165183
$this->assertSame($expected, CITestStreamFilter::$buffer);
166184
}
167185

168186
public function testPrintBackground()
169187
{
170188
CLI::print('test', 'red', 'green');
171-
$expected = "\033[0;31m\033[42mtest\033[0m";
172189

190+
$expected = "\033[0;31m\033[42mtest\033[0m";
173191
$this->assertSame($expected, CITestStreamFilter::$buffer);
174192
}
175193

176194
public function testWrite()
177195
{
178196
CLI::write('test');
197+
179198
$expected = PHP_EOL . 'test' . PHP_EOL;
180199
$this->assertSame($expected, CITestStreamFilter::$buffer);
181200
}
182201

183202
public function testWriteForeground()
184203
{
185204
CLI::write('test', 'red');
205+
186206
$expected = "\033[0;31mtest\033[0m" . PHP_EOL;
187207
$this->assertSame($expected, CITestStreamFilter::$buffer);
188208
}
189209

190210
public function testWriteForegroundWithColorBefore()
191211
{
192212
CLI::write(CLI::color('green', 'green') . ' red', 'red');
193-
$expected = "\033[0;31m\033[0;32mgreen\033[0m\033[0;31m red\033[0m" . PHP_EOL;
213+
214+
$expected = "\033[0;32mgreen\033[0m\033[0;31m red\033[0m" . PHP_EOL;
194215
$this->assertSame($expected, CITestStreamFilter::$buffer);
195216
}
196217

197218
public function testWriteForegroundWithColorAfter()
198219
{
199220
CLI::write('red ' . CLI::color('green', 'green'), 'red');
200-
$expected = "\033[0;31mred \033[0;32mgreen\033[0m" . PHP_EOL;
221+
222+
$expected = "\033[0;31mred \033[0m\033[0;32mgreen\033[0m" . PHP_EOL;
223+
$this->assertSame($expected, CITestStreamFilter::$buffer);
224+
}
225+
226+
/**
227+
* @see https://github.com/codeigniter4/CodeIgniter4/issues/5892
228+
*/
229+
public function testWriteForegroundWithColorTwice()
230+
{
231+
CLI::write(
232+
CLI::color('green', 'green') . ' red ' . CLI::color('green', 'green'),
233+
'red'
234+
);
235+
236+
$expected = "\033[0;32mgreen\033[0m\033[0;31m red \033[0m\033[0;32mgreen\033[0m" . PHP_EOL;
201237
$this->assertSame($expected, CITestStreamFilter::$buffer);
202238
}
203239

204240
public function testWriteBackground()
205241
{
206242
CLI::write('test', 'red', 'green');
243+
207244
$expected = "\033[0;31m\033[42mtest\033[0m" . PHP_EOL;
208245
$this->assertSame($expected, CITestStreamFilter::$buffer);
209246
}
210247

211248
public function testError()
212249
{
213250
$this->stream_filter = stream_filter_append(STDERR, 'CITestStreamFilter');
251+
214252
CLI::error('test');
253+
215254
// red expected cuz stderr
216255
$expected = "\033[1;31mtest\033[0m" . PHP_EOL;
217256
$this->assertSame($expected, CITestStreamFilter::$buffer);
@@ -220,15 +259,19 @@ public function testError()
220259
public function testErrorForeground()
221260
{
222261
$this->stream_filter = stream_filter_append(STDERR, 'CITestStreamFilter');
262+
223263
CLI::error('test', 'purple');
264+
224265
$expected = "\033[0;35mtest\033[0m" . PHP_EOL;
225266
$this->assertSame($expected, CITestStreamFilter::$buffer);
226267
}
227268

228269
public function testErrorBackground()
229270
{
230271
$this->stream_filter = stream_filter_append(STDERR, 'CITestStreamFilter');
272+
231273
CLI::error('test', 'purple', 'green');
274+
232275
$expected = "\033[0;35m\033[42mtest\033[0m" . PHP_EOL;
233276
$this->assertSame($expected, CITestStreamFilter::$buffer);
234277
}
@@ -273,9 +316,18 @@ public function testShowProgressWithoutBar()
273316
public function testWrap()
274317
{
275318
$this->assertSame('', CLI::wrap(''));
276-
$this->assertSame('1234' . PHP_EOL . ' 5678' . PHP_EOL . ' 90' . PHP_EOL . ' abc' . PHP_EOL . ' de' . PHP_EOL . ' fghij' . PHP_EOL . ' 0987654321', CLI::wrap('1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321', 5, 1));
277-
$this->assertSame('1234 5678 90' . PHP_EOL . ' abc de fghij' . PHP_EOL . ' 0987654321', CLI::wrap('1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321', 999, 2));
278-
$this->assertSame('1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321', CLI::wrap('1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321'));
319+
$this->assertSame(
320+
'1234' . PHP_EOL . ' 5678' . PHP_EOL . ' 90' . PHP_EOL . ' abc' . PHP_EOL . ' de' . PHP_EOL . ' fghij' . PHP_EOL . ' 0987654321',
321+
CLI::wrap('1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321', 5, 1)
322+
);
323+
$this->assertSame(
324+
'1234 5678 90' . PHP_EOL . ' abc de fghij' . PHP_EOL . ' 0987654321',
325+
CLI::wrap('1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321', 999, 2)
326+
);
327+
$this->assertSame(
328+
'1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321',
329+
CLI::wrap('1234 5678 90' . PHP_EOL . 'abc de fghij' . PHP_EOL . '0987654321')
330+
);
279331
}
280332

281333
public function testParseCommand()
@@ -287,6 +339,7 @@ public function testParseCommand()
287339
];
288340
$_SERVER['argc'] = 3;
289341
CLI::init();
342+
290343
$this->assertNull(CLI::getSegment(3));
291344
$this->assertSame('b', CLI::getSegment(1));
292345
$this->assertSame('c', CLI::getSegment(2));
@@ -312,6 +365,7 @@ public function testParseCommandMixed()
312365
'sure',
313366
];
314367
CLI::init();
368+
315369
$this->assertNull(CLI::getSegment(7));
316370
$this->assertSame('b', CLI::getSegment(1));
317371
$this->assertSame('c', CLI::getSegment(2));
@@ -335,6 +389,7 @@ public function testParseCommandOption()
335389
'd',
336390
];
337391
CLI::init();
392+
338393
$this->assertSame(['parm' => 'pvalue'], CLI::getOptions());
339394
$this->assertSame('pvalue', CLI::getOption('parm'));
340395
$this->assertSame('-parm pvalue ', CLI::getOptionString());
@@ -359,6 +414,7 @@ public function testParseCommandMultipleOptions()
359414
'value 3',
360415
];
361416
CLI::init();
417+
362418
$this->assertSame(['parm' => 'pvalue', 'p2' => null, 'p3' => 'value 3'], CLI::getOptions());
363419
$this->assertSame('pvalue', CLI::getOption('parm'));
364420
$this->assertSame('-parm pvalue -p2 -p3 "value 3" ', CLI::getOptionString());
@@ -373,11 +429,13 @@ public function testWindow()
373429
$height = new ReflectionProperty(CLI::class, 'height');
374430
$height->setAccessible(true);
375431
$height->setValue(null);
432+
376433
$this->assertIsInt(CLI::getHeight());
377434

378435
$width = new ReflectionProperty(CLI::class, 'width');
379436
$width->setAccessible(true);
380437
$width->setValue(null);
438+
381439
$this->assertIsInt(CLI::getWidth());
382440
}
383441

@@ -391,6 +449,7 @@ public function testWindow()
391449
public function testTable($tbody, $thead, $expected)
392450
{
393451
CLI::table($tbody, $thead);
452+
394453
$this->assertSame(CITestStreamFilter::$buffer, $expected);
395454
}
396455

‎user_guide_src/source/changelogs/v4.2.0.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ BREAKING
1818
- ``spark``
1919
- The method signature of ``CodeIgniter\CLI\CommandRunner::_remap()`` has been changed to fix a bug.
2020
- The ``CodeIgniter\Autoloader\Autoloader::initialize()`` has changed the behavior to fix a bug. It used to use Composer classmap only when ``$modules->discoverInComposer`` is true. Now it always uses the Composer classmap if Composer is available.
21+
- The color code output by :ref:`CLI::color() <cli-library-color>` has been changed to fix a bug.
2122

2223
Enhancements
2324
************

0 commit comments

Comments
 (0)