[Console] Constant STDOUT might be undefined

[derrabus]

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #34341
License	MIT
Doc PR	N/A

[nicolas-grekas]

fopen('php://stdout', 'w')?

[derrabus]

fopen('php://stdout', 'w')?

Would be overkill here. The constant is defined by the CLI SAPI. Thus, if the constant is not there, we're not on CLI, thus we're not on a terminal with VT100 support.

We already have a similar check in PhpUnitBridge:

symfony/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php

Lines 355 to 357 in fb4065a

	if (!\defined('STDOUT')) {
	return false;
	}

[nicolas-grekas]

I'm not sure I agree. If the check targets the sapi, let's make it crystal clear. Checks for color support all use php://stdout. STDOUT is not used anywhere else in the codebase as far as I checked. I think we should check what we want to check in a portable way, using php://stdout Sorry my previous comment was too laconic.

[derrabus]

STDOUT is not used anywhere else in the codebase as far as I checked.

Well yes, the method I've linked in my previous comment uses the constant to check for color support as well.

symfony/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php

Lines 353 to 383 in fb4065a

	private static function hasColorSupport()
	{
	if (!\defined('STDOUT')) {
	return false;
	}
	if ('Hyper' === getenv('TERM_PROGRAM')) {
	return true;
	}
	if (\DIRECTORY_SEPARATOR === '\\') {
	return (\function_exists('sapi_windows_vt100_support')
	&& sapi_windows_vt100_support(STDOUT))
	|| false !== getenv('ANSICON')
	|| 'ON' === getenv('ConEmuANSI')
	|| 'xterm' === getenv('TERM');
	}
	if (\function_exists('stream_isatty')) {
	return stream_isatty(STDOUT);
	}
	if (\function_exists('posix_isatty')) {
	return posix_isatty(STDOUT);
	}
	$stat = fstat(STDOUT);
	// Check if formatted mode is S_IFCHR
	return $stat ? 0020000 === ($stat['mode'] & 0170000) : false;
	}

Also, I wouldn't want to change the way stdout is accessed without a Windows machine to test on. 😕

[nicolas-grekas]

That's the bridge, it's not a piece of reusability, it's a tool :)
No worries about php://stdout, it does the very same job.

[derrabus]

All right, here you go.

@lazosweb Can you please test again? Sorry for the hassle. 😃

[lazosweb]

@derrabus . All good. No issue. That will work too.

[chalasr]

Should be merged after #34346 and rebased on 3.4.

[derrabus]

Should be merged after #34346 and rebased on 3.4.

@chalasr Feel free to squash my change into your PR. I don't think, we need to backport a buggy solution first. 😉

[nicolas-grekas]

Thank you @derrabus