Add colours to run-test

[Girgias]

Based from: xdebug/xdebug@296f31d

Not sure if "BORK" actually deserves it's special category/colour

[cmb69]

I like that very much, but at least for Windows it would make sense to check sapi_windows_vt100_support(), and to not print ANSI escape codes in that case. Or perhaps even better let the user control that with a command line option.

[Girgias]

I like that very much, but at least for Windows it would make sense to check sapi_windows_vt100_support(), and to not print ANSI escape codes in that case. Or perhaps even better let the user control that with a command line option.

The command line option is indeed something which I should add.

Just to be clear in regards to Windows, as I'm not familiar with it's console env, I should not print the ANSI escape code in case VT100 is not support and default back to the previous behaviour?

[twose]

I did the same in my own extension, hope this help :)

// namespace XXX;

const COLOR_NONE = 0;
const COLOR_RED = 1;
const COLOR_GREEN = 2;
const COLOR_YELLOW = 3;
const COLOR_BLUE = 4;
const COLOR_MAGENTA = 5;
const COLOR_CYAN = 6;
const COLOR_WHITE = 7;

function dye(string $string, int $color): string
{
    // check if it is available here?
    return ($color !== COLOR_NONE ? "\033[3{$color}m{$string}\033[0m" : $string);
}

BTW, isn't LEAK worth having color? 😂

[cmb69]

[…] I should not print the ANSI escape code in case VT100 is not support and default back to the previous behaviour?

Yes. :)

[Girgias]

Added checks for Windows and added a Dark Blue colouring for LEAK.

[theodorejb]

Should the American spelling of color be used? That seems to be the typical spelling elsewhere in the source code: https://github.com/php/php-src/search?q=color

[KalleZ]

Should the American spelling of color be used? That seems to be the typical spelling elsewhere in the source code: https://github.com/php/php-src/search?q=color

I agree, we tend to use more Americanized naming, would prefer to stick to that for consistency

[cmb69]

The Windows specific changes look good to me. I also would prefer the American English spelling.

[Girgias]

Will use the Americanized version the.
Should I enable the --no-color flag in CI or not?

[KalleZ]

@Girgias If our CIs support it and it looks good, then I do not see why not

[Girgias]

Just looks like AppVeyor doesn't enable VT100, and Travis and Azure only show skipped/XFAIL/FAIL so seems reasonable to keep it on CI

[theodorejb]

colorise should be colorize (here and elsewhere)

[carusogabriel]

Can I suggest to add colors to the summary table as well? Something like:

or

[carusogabriel]

Missing

Suggested change

	$color = "\e[0;37m{$result}"; break;
	$color = "\e[0;37m{$result}\e[0m"; break;

?

[Girgias]

Can I suggest to add colors to the summary table as well? Something like:

or

The get_summary() function is quite a mess so adding this is pretty annoying as it would need a bunch of conditionals and more splitting it up. I'd rather leave that for later after a possible refactor of this function.

[cmb69]

FWIW, the AppVeyor CI failure is unrelated to this PR.

[nikic]

This ends up being pretty much the same as the normal text color :/ Maybe use yellow or orange?

[nikic]

Basically, let this fall through the the default case that also handles XFAIL. XFAIL and SKIP seems pretty similar.

[nikic]

I'd just make all of these red, they're all effectively a failure.

[nikic]

Why is there an \e[0m here?

[nikic]

Suggested change

	switch ( $result ) {
	switch ($result) {