Skip to content

[Console] Restoring the ability to output unicode text to the Win10 console #49987

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
Merged
merged 1 commit into from
Apr 12, 2023
Merged

[Console] Restoring the ability to output unicode text to the Win10 console #49987

merged 1 commit into from
Apr 12, 2023

Conversation

aleksandr-shevchenko
Copy link
Contributor

@aleksandr-shevchenko aleksandr-shevchenko commented Apr 10, 2023 •
edited by nicolas-grekas
Loading

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Restoring the ability to output unicode text to the Win10 console after corrupting the console on line 224 in symfony/console/Terminal.php


require_once __DIR__ . '/../vendor/autoload.php';

use App\Commands\ClearcacheCommand;
use Symfony\Component\Console\Application;

echo "abc йфяЙФЯёЁ üÜiİöÖğĞıIəƏçÇşŞ" . PHP_EOL;

$app = new Application();
$app->setName("abc йфяЙФЯёЁ üÜiİöÖğĞıIəƏçÇşŞ");
$app->setVersion("0.0.001");

$app->run();

before
ss_2023_04_10__13_58_30
after
ss_2023_04_10__14_00_31

@carsonbot carsonbot added this to the 6.3 milestone Apr 10, 2023
@nicolas-grekas
Copy link
Member

Why do you say "restore"? Does the call change some state? If yes, can't we restore the previous cp instead? Aka use sapi_windows_cp_get() before and restore after, instead of hardcoding 65001?

@aleksandr-shevchenko
Copy link
Contributor Author

aleksandr-shevchenko commented Apr 10, 2023 •
edited
Loading

Why do you say "restore"? Does the call change some state? If yes, can't we restore the previous cp instead? Aka use sapi_windows_cp_get() before and restore after, instead of hardcoding 65001?

    private static function readFromProcess(string|array $command): ?string
    {
        if (!\function_exists('proc_open')) {
            return null;
        }

        $descriptorspec = [
            1 => ['pipe', 'w'],
            2 => ['pipe', 'w'],
        ];

        $process = proc_open($command, $descriptorspec, $pipes, null, null, ['suppress_errors' => true]);
        if (!\is_resource($process)) {
            return null;
        }

        $info = stream_get_contents($pipes[1]);
        fclose($pipes[1]);
        fclose($pipes[2]);
        proc_close($process);

        if (\function_exists('sapi_windows_cp_set')) {
            sapi_windows_cp_set(65001);
        }

        return $info;
    }

In this function, before line $info = stream_get_contents($pipes[1]); echo can print unicode text to Win10 console, but after that line - not.
I don't understand why the console gets corrupted, but my solution works. What's wrong with hardcoding UTF-8(65001)?

        if (\function_exists('sapi_windows_cp_get')) {
            $cp1 = sapi_windows_cp_get();
            echo $cp1 . " abc йфяЙФЯёЁ üÜiİöÖğĞıIəƏçÇşŞ" . PHP_EOL;
        }

        $info = stream_get_contents($pipes[1]);

        if (\function_exists('sapi_windows_cp_get')) {
            $cp2 = sapi_windows_cp_get();
            echo $cp2 . " abc йфяЙФЯёЁ üÜiİöÖğĞıIəƏçÇşŞ" . PHP_EOL;
        }
        echo $info;

ss_2023_04_11__08_42_41

@nicolas-grekas
Copy link
Member

Strange :)

What's wrong with hardcoding UTF-8(65001)?

hardcoding is always an issue, especially when there's a way not to hardcode. Here, this change would break consoles that rely on another CP. Wouldn't my previous suggestion work?

@aleksandr-shevchenko
Copy link
Contributor Author

aleksandr-shevchenko commented Apr 11, 2023 •
edited
Loading

Wouldn't my previous suggestion work?

Also works well (and without hardcoding 65001).

        if (\function_exists('sapi_windows_cp_get')) {
            $cp = sapi_windows_cp_get();
            echo $cp . " abc йфяЙФЯёЁ üÜiİöÖğĞıIəƏçÇşŞ" . PHP_EOL;
        }

        $info = stream_get_contents($pipes[1]);

        if (\function_exists('sapi_windows_cp_get') && \function_exists('sapi_windows_cp_set')) {
            sapi_windows_cp_set($cp);
            echo $cp . " abc йфяЙФЯёЁ üÜiİöÖğĞıIəƏçÇşŞ" . PHP_EOL;
        }
        echo $info;

ss_2023_04_11__14_23_25

@chalasr
Copy link
Member

chalasr commented Apr 11, 2023

We should probably do the same as #41113 (or even reuse if possible).
Also this qualifies as a bugfix, isn't it? Please fill in the PR template you deleted when opening (you can find it in other PRs).

@carsonbot carsonbot changed the title Restoring the ability to output unicode text to the Win10 console [Console] Restoring the ability to output unicode text to the Win10 console Apr 12, 2023
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 5.4 Apr 12, 2023
@aleksandr-shevchenko @nicolas-grekas
[Console] Restoring the ability to output unicode text to the Win10 c…
91a364c 
…onsole

Restoring the ability to output unicode text to the Win10 console after corrupting the console on line 224
@nicolas-grekas nicolas-grekas changed the base branch from 6.3 to 5.4 April 12, 2023 06:44
@nicolas-grekas
Copy link
Member

Thank you @aleksandr-shevchenko.

@nicolas-grekas nicolas-grekas merged commit 6ae4ac9 into symfony:5.4 Apr 12, 2023
@aleksandr-shevchenko aleksandr-shevchenko deleted the patch-3 branch April 13, 2023 20:50
@aleksandr-shevchenko aleksandr-shevchenko mentioned this pull request Apr 19, 2023
Closed
This was referenced Apr 28, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalasr chalasr Awaiting requested review from chalasr chalasr is a code owner

@lyrixx lyrixx Awaiting requested review from lyrixx

@yceruto yceruto Awaiting requested review from yceruto

@wouterj wouterj Awaiting requested review from wouterj

@dunglas dunglas Awaiting requested review from dunglas

@OskarStark OskarStark Awaiting requested review from OskarStark

@xabbuh xabbuh Awaiting requested review from xabbuh

Assignees
No one assigned
Labels
Console Status: Needs Review
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
4 participants
@aleksandr-shevchenko @nicolas-grekas @chalasr @carsonbot