Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use ANSI color codes and Unicode characters on Windows as well #662

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Use ANSI color codes and Unicode characters on Windows as well #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
880a25b
Common: prepareForOutput(): Use single color code per run of characters
MatmaRex Nov 17, 2024
41bc621
Common: prepareForOutput(): Use ANSI color codes on Windows as well
MatmaRex Nov 9, 2024
8e976bc
Common: prepareForOutput(): Use Unicode output on Windows as well on …
MatmaRex Nov 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Reports/Code.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false,
if (strpos($tokenContent, "\t") !== false) {
$token = $tokens[$i];
$token['content'] = $tokenContent;
if (stripos(PHP_OS, 'WIN') === 0) {
if (stripos(PHP_OS, 'WIN') === 0 && PHP_VERSION_ID < 70100) {
// Printing Unicode characters like '»' to Windows console only works since PHP 7.1.
$tab = "\000";
} else {
$tab = "\033[30;1m»\033[0m";
Expand Down
10 changes: 1 addition & 9 deletions src/Tokenizers/PHP.php
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the reasoning for this change ?

I mean, in contrast to the "Output now has fewer redundant ANSI color codes" changelog suggestion, this ends up creating more redundant ANSI color codes (at least on Windows).

Before:
image

After:
image

Screenshots generated using the following command on the previously posted code snippet on PHP 7.1 on Windows.

phpcs ./phpcs-662.php --tab-width=4 --standard=generic -vv

Also note (not necessarily to be addressed in this PR, probably should be a separate one): the --no-colors option can also be used on non-Windows OSes and the previous implementation already didn't respect that.
The PHP class does have access to the Config though via the Tokenizer::$config option, so maybe it should respect the colors setting ?

Original file line number Diff line number Diff line change
Expand Up @@ -516,10 +516,6 @@ protected function tokenize($string)
{
if (PHP_CODESNIFFER_VERBOSITY > 1) {
echo "\t*** START PHP TOKENIZING ***".PHP_EOL;
$isWin = false;
if (stripos(PHP_OS, 'WIN') === 0) {
$isWin = true;
}
}

$tokens = @token_get_all($string);
Expand Down Expand Up @@ -584,11 +580,7 @@ protected function tokenize($string)
) {
$token[1] .= "\n";
if (PHP_CODESNIFFER_VERBOSITY > 1) {
if ($isWin === true) {
echo '\n';
} else {
echo "\033[30;1m\\n\033[0m";
}
echo "\033[30;1m\\n\033[0m";
}

if ($tokens[($stackPtr + 1)][1] === "\n") {
Expand Down
45 changes: 17 additions & 28 deletions src/Util/Common.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,7 @@ public static function escapeshellcmd($cmd)
/**
* Prepares token content for output to screen.
*
* Replaces invisible characters so they are visible. On non-Windows
* operating systems it will also colour the invisible characters.
* Replaces invisible characters so they are visible, and colour them.
*
* @param string $content The content to prepare.
* @param string[] $exclude A list of characters to leave invisible.
Expand All @@ -278,35 +277,25 @@ public static function escapeshellcmd($cmd)
*/
public static function prepareForOutput($content, $exclude=[])
{
if (stripos(PHP_OS, 'WIN') === 0) {
if (in_array("\r", $exclude, true) === false) {
$content = str_replace("\r", '\r', $content);
}

if (in_array("\n", $exclude, true) === false) {
$content = str_replace("\n", '\n', $content);
}

if (in_array("\t", $exclude, true) === false) {
$content = str_replace("\t", '\t', $content);
}
} else {
if (in_array("\r", $exclude, true) === false) {
$content = str_replace("\r", "\033[30;1m\\r\033[0m", $content);
}
$replacements = [
"\r" => '\r',
"\n" => '\n',
"\t" => '\t',
" " => '·',
];
if (stripos(PHP_OS, 'WIN') === 0 && PHP_VERSION_ID < 70100) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this condition sufficient ?

If I read the https://www.php.net/manual/en/migration71.windows-support.php page, I get the impression an additional check may be needed against the value of the internal_encoding ini setting - and if that's empty, the default_charset ini setting - to verify it is set to UTF-8 ?

Also note that internal_encoding and default_charset were both only introduced in PHP 5.6: https://www.php.net/manual/en/migration56.deprecated.php#migration56.deprecated.iconv-mbstring-encoding

Same remark applies to the Code report change.

// Do not replace spaces on old PHP on Windows.
// Printing Unicode characters like '·' to Windows console only works since PHP 7.1.
unset($replacements[" "]);
}

if (in_array("\n", $exclude, true) === false) {
$content = str_replace("\n", "\033[30;1m\\n\033[0m", $content);
}
$replacements = array_diff_key($replacements, array_fill_keys($exclude, true));

if (in_array("\t", $exclude, true) === false) {
$content = str_replace("\t", "\033[30;1m\\t\033[0m", $content);
}
// Colour runs of invisible characters.
$match = implode('', array_keys($replacements));
$content = preg_replace("/([$match]+)/", "\033[30;1m$1\033[0m", $content);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ends up adding more redundant colour codes on Windows for PHP < 7.1, similar to what is happening in the PHP tokenizer.


if (in_array(' ', $exclude, true) === false) {
$content = str_replace(' ', "\033[30;1m·\033[0m", $content);
}
}//end if
$content = strtr($content, $replacements);

return $content;

Expand Down
59 changes: 44 additions & 15 deletions tests/Core/Util/Common/PrepareForOutputTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,45 +27,74 @@ final class PrepareForOutputTest extends TestCase
* @param string $content The content to prepare.
* @param string[] $exclude A list of characters to leave invisible.
* @param string $expected Expected function output.
* @param string $expectedWin Expected function output on Windows (unused in this test).
* @param string $expectedOld Expected function output on PHP<7.1 on Windows (unused in this test).
*
* @requires OS ^(?!WIN).*
* @dataProvider dataPrepareForOutput
*
* @return void
*/
public function testPrepareForOutput($content, $exclude, $expected, $expectedWin)
public function testPrepareForOutput($content, $exclude, $expected, $expectedOld)
{
$this->assertSame($expected, Common::prepareForOutput($content, $exclude));

}//end testPrepareForOutput()


/**
* Test formatting whitespace characters, on Windows.
* Test formatting whitespace characters, on modern PHP on Windows.
*
* @param string $content The content to prepare.
* @param string[] $exclude A list of characters to leave invisible.
* @param string $expected Expected function output (unused in this test).
* @param string $expectedWin Expected function output on Windows.
* @param string $expected Expected function output.
* @param string $expectedOld Expected function output on PHP<7.1 on Windows (unused in this test).
*
* @requires OS ^WIN.*.
* @requires PHP 7.1
* @dataProvider dataPrepareForOutput
*
* @return void
*/
public function testPrepareForOutputWindows($content, $exclude, $expected, $expectedWin)
public function testPrepareForOutputWindows($content, $exclude, $expected, $expectedOld)
{
$this->assertSame($expectedWin, Common::prepareForOutput($content, $exclude));
$this->assertSame($expected, Common::prepareForOutput($content, $exclude));

}//end testPrepareForOutputWindows()


/**
* Test formatting whitespace characters, on PHP<7.1 on Windows.
*
* @param string $content The content to prepare.
* @param string[] $exclude A list of characters to leave invisible.
* @param string $expected Expected function output (unused in this test).
* @param string $expectedOld Expected function output on PHP<7.1 on Windows.
*
* @requires OS ^WIN.*.
* @requires PHP < 7.1
* @dataProvider dataPrepareForOutput
*
* @return void
*/
public function testPrepareForOutputOldPHPWindows($content, $exclude, $expected, $expectedOld)
{
// PHPUnit 4.8 (used on PHP 5.4) does not support the `@requires PHP < 7.1` syntax,
// so double-check to avoid test failures.
if (PHP_VERSION_ID >= 70100) {
$this->markTestSkipped("Only for PHP < 7.1");
}

$this->assertSame($expectedOld, Common::prepareForOutput($content, $exclude));

}//end testPrepareForOutputOldPHPWindows()


/**
* Data provider.
*
* @see testPrepareForOutput()
* @see testPrepareForOutputWindows()
* @see testPrepareForOutputOldPHPWindows()
*
* @return array<string, array<string, mixed>>
*/
Expand All @@ -75,35 +104,35 @@ public static function dataPrepareForOutput()
'Special characters are replaced with their escapes' => [
'content' => "\r\n\t",
'exclude' => [],
'expected' => "\033[30;1m\\r\033[0m\033[30;1m\\n\033[0m\033[30;1m\\t\033[0m",
'expectedWin' => "\\r\\n\\t",
'expected' => "\033[30;1m\\r\\n\\t\033[0m",
'expectedOld' => "\033[30;1m\\r\\n\\t\033[0m",
],
'Spaces are replaced with a unique mark' => [
'content' => " ",
'exclude' => [],
'expected' => "\033[30;1m·\033[0m\033[30;1m·\033[0m\033[30;1m·\033[0m\033[30;1m·\033[0m",
'expectedWin' => " ",
'expected' => "\033[30;1m····\033[0m",
'expectedOld' => " ",
],
'Other characters are unaffected' => [
'content' => "{echo 1;}",
'exclude' => [],
'expected' => "{echo\033[30;1m·\033[0m1;}",
'expectedWin' => "{echo 1;}",
'expectedOld' => "{echo 1;}",
],
'No replacements' => [
'content' => 'nothing-should-be-replaced',
'exclude' => [],
'expected' => 'nothing-should-be-replaced',
'expectedWin' => 'nothing-should-be-replaced',
'expectedOld' => 'nothing-should-be-replaced',
],
'Excluded whitespace characters are unaffected' => [
'content' => "\r\n\t ",
'exclude' => [
"\r",
"\n",
],
'expected' => "\r\n\033[30;1m\\t\033[0m\033[30;1m·\033[0m",
'expectedWin' => "\r\n\\t ",
'expected' => "\r\n\033[30;1m\\t·\033[0m",
'expectedOld' => "\r\n\033[30;1m\\t\033[0m ",
],
];

Expand Down