-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Implement stream_vt100_support user function #2103
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.
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
Conversation
The first parameter should be a stream (php://stdout or php://stderr). If no second parameter is specified: the function checks if the stream supports VT100 control codes: - Windows: returns TRUE if Windows is 10.0.10586 or greater, the console has the ENABLE_VIRTUAL_TERMINAL_PROCESSING flag and the stream is not redirected. - POSIX: returns TRUE if the stream is not redirected. - Others cases: returns FALSE If the second parameter is specified: the function tries to enable or disable the VT100 control codes. - Windows: returns TRUE if Windows is 10.0.10586 or greater, the stream is not redirected and the ENABLE_VIRTUAL_TERMINAL_PROCESSING flag has been set/unset - POSIX: returns TRUE is enabling and the stream is not redirected - POSIX: returns TRUE is disabling and the stream is redirected - Others cases: returns FALSE
Any hint about how to retrieve the standard Windows handles ( |
Done in bd93781 😉 |
Done in d9b1c99 😉 |
Hi Wouldn't it be easier if this was added as a context option instead? |
What exactly do you mean? An example? |
Sample code: <?php
$sampleText = "\033[101;93m Yellow text on red background \033[0m\n";
echo "Enabled: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo $sampleText;
echo "Enable: ", stream_vt100_support(STDOUT, true) ? "ok" : "failed", "\n";
echo "Enabled: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo $sampleText;
echo "Disable: ", stream_vt100_support(STDOUT, false) ? "ok" : "failed", "\n";
echo "Enabled: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo $sampleText; |
Remark 1: since the <?php
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo "Enable on STDERR : ", stream_vt100_support(STDERR, true) ? "ok" : "failed", "\n";
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo "Disable on STDERR: ", stream_vt100_support(STDERR, false) ? "ok" : "failed", "\n";
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n"; ouputs
Remark 2: this function checks if the specified stream is redirected. <?php
echo "Enable on STDOUT: ", stream_vt100_support(STDOUT, true) ? "ok" : "failed", "\n";
echo "Enable on STDERR: ", stream_vt100_support(STDERR, true) ? "ok" : "failed", "\n";
echo "\n";
echo "Enabled on STDOUT: ", stream_vt100_support(STDOUT) ? "yes" : "no", "\n";
echo "Enabled on STDERR: ", stream_vt100_support(STDERR) ? "yes" : "no", "\n";
|
RETURN_FALSE; | ||
} | ||
if (php_stream_can_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT) == SUCCESS) { | ||
php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT, (void*)&fileno, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The void* cast will cause the stack corruption on 64-bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The void* cast will cause the stack corruption on 64-bit.
Fixed in 694e207
#endif | ||
|
||
/* Check if the current Windows version supports VT100 control codes */ | ||
BOOL php_win32_console_os_supports_vt100(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions should be exported. Later it can be integrated with phpdbg, but can be useful for non core as well. Easy done like in win32/winutil.h.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 2f05643
@@ -1062,7 +1062,7 @@ function error_report($testname, $logname, $tested) | |||
} | |||
} | |||
|
|||
function system_with_timeout($commandline, $env = null, $stdin = null) | |||
function system_with_timeout($commandline, $env = null, $stdin = null, $captureStdOut = true, $captureStdErr = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a nice approach, but i'd rather avoid adding the new section to run-tests.php. It is not a general case and probably only useful for this concrete case, so IMO a new section is not justified.
What could be done instead, were doing an extra PHP run inside the test. Fe we do this for the CLI server, you can cehck sapi/cli/tests/php_cli_server.inc for a simple example framework. It is also better to be close to the real word, when testing redirection. Like
php.exe -r "if (stream_vt100_support(STDOUT, true)) echo \"\033[101;93msupported\033[0m\n\"; else echo 'not supported'; " > tmp
should should out "not supported" into a file on both linux and windows. Or i don't get this part right?
Thanks.
@mlocati for the stream option, it could be similar like I've mentioned in the ticket, like
It were same as using the function. As for me, the stream option is not absolutely necessary in the current implementation. It would become necessary, if we'd go for an automation to strip the ANSI controls. Thanks. |
@mlocati Great work in trying to get this incorporated (I have been following this via Symfony and Composer issues). However, your implementation is incorrect in testing if the handle is redirected, rather than testing if it is a terminal (which is what Microsoft have updated their vt100 documentation and state that:
so there is no need for any version checking, which will make the code simpler. Likewise, you can use GetConsoleMode as the equivalent of isatty, as mentioned above (which is what Docker does). |
Did you mean |
…lPathNameByHandleW to check if a handle is a valid console stream
@johnstevenson Thanks! I'd keep a "is this stream a valid console" function, so that we can update it in case the current implementation needs future adjustments (btw, I replaced |
@weltling So, instead of #ifdef _WIN64
(zend_long or long long) fileno;
#else
int fileno;
#endif I should write intptr_t fileno; ? |
@mlocati yeah, that's fine. The (void *) would be exact same size platform dependent. Thanks. |
@weltling I used About $ctx1 = stream_context_create();
stream_context_set_option($ctx1, 'stdio', 'enable_vt100', true);
$ctx2 = stream_context_create();
stream_context_set_option($ctx2, 'stdio', 'enable_vt100', false); The problem here is that one may think that the two contexts are isolated, but they are not... |
@mlocati you're right. I was just testing and looking yet more over the patch. Please consider a snippet like
In the current implementation it won't work, as the checks are focused on the original stdio handles. The Any php stream may point to a valid console, not just STDIO. I guess that set/get console mode will anyway apply to the given console object disregarding the fd used. Full isolation seems not to be possible therefore, as that's the way how the low level API works. But on the PHP side it should be possible to operate on Thanks |
We need to restore previous console mode on failing SetConsole calls only for STDIN
@johnstevenson I did some tests, and it always returned -1 ( |
# Conflicts: # win32/build/config.w32
@mlocati If you debug php-win.exe and step through @weltling Blame me for suggesting the |
@mlocati ok, in that case it might be interesting with appveyor. You rule actually :) There is a couple other things going on yet. Like the work on the new SDK tools, which interferes with related areas. All in all, IMO it's worth it to create a PR for the matter and discuss there. @johnstevenson yeah, that's a similar case, as SetConsoleCP and co would fail as well. Thanks. |
Great!! |
PS: what are the plans for these fubctions? I mean, for which php versions will them be available? |
@mlocati, it's merged into master, which is the future 7.2 nowadays. 7.1 was already freezed by the discussion time, anyway. And otherwise, it is fine to go through a good QA for the patches of such extent. The 7.2 cycle will start spring 2017, so there's enough time for possible fix ups and doc. I've already positive feedback from yet another tester, the PR is therefore fine for master. One thing standing out yet is the newly released Nano Server, as it has quite minimalist console capabilities. More or less, that's the approx plan for this feature. Thanks. |
@weltling / @johnstevenson It seems that the $ php -r "var_export(stream_isatty(STDOUT));"
false I'm not very familiar with the |
@mlocati so far i get with the git shell
it's based on some parts of Cygwin. Otherwise, no chance for me with pure Cygwin, as it's principally a workspace breaker when it comes to the native VS environment. Is that a Cygwin build of PHP? The result above is with the native build. FYI, we don't officially support Cygwin and other similar build flavors. Thanks. |
Nope: it's the php.exe version from https://windows.php.net/download/#php-7.2-ts-VC15-x86 |
$ uname -a
CYGWIN_NT-10.0 MYPC 2.10.0(0.325/5/3) 2018-02-02 15:16 x86_64 Cygwin
$ which php
/cygdrive/c/Dev/PHP/php
$ php -v
PHP 7.2.4 (cli) (built: Mar 28 2018 04:46:46) ( ZTS MSVC15 (Visual C++ 2017) x86 )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
with Xdebug v2.6.0, Copyright (c) 2002-2018, by Derick Rethans
with Zend OPcache v7.2.4, Copyright (c) 1999-2018, by Zend Technologies
$ php -r "var_export(stream_isatty(STDOUT));"
false |
Cygwin, Msys2 etc emulate pseudo terminals via named pipes, so this is correct (if unhelpful). |
I've noticed the same problem in WLS. By the way, quite interestingly, with Git Bash this command php.exe -r "var_export([stream_isatty(STDOUT), sapi_windows_vt100_support(STDOUT)]);" outputs
|
This function (
from Dockerfile:
I receive errors:
When I'm running using
I receive no errors:
Is somebody knows what is the cause of such strange situation? :) P.S.:
and the same when it passed:
|
This is a followup of https://bugs.php.net/bug.php?id=72768
TL;DR Console apps of newer Windows 10 versions support colors, but we need to manually set the
ENABLE_VIRTUAL_TERMINAL_PROCESSING
console flag.Here's my proposal of a new userland function:
bool stream_vt100_support(resource stream[, bool enable])
Where:
stream
should beSTDOUT
orSTDERR
enable
missing: the function returns true if the specified stream supports colorsenable
is true: the function tries to enable color support for the stream (return true on success)enable
is false: the function tries to disable color support for the stream (return true on success)To be done:
php://stdout
): it should instead accept a stream resource