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.

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

[Console] Add Ansi8 (256 color) support, improve true color (Ansi24) support detection #46944

Merged
merged 1 commit into from Jul 22, 2022

Conversation

julien-boudry
Copy link
Contributor

Q A
Branch? 6.2 for features
Bug fix? no
New feature? yes
Deprecations? no
License MIT
  • Refactor the detection code of the colorimetric capacities of the terminal (much easier to extend next)
  • Improve true color terminal detection according to https://github.com/termstandard/colors/
  • Add detection for 256 colors terminal (Ansi8 support) and add the conversion from true color (Ansi24) to it. This is especially useful for the Apple Terminal which is still deployed by default in 2022, but not only.

@carsonbot carsonbot added this to the 6.2 milestone Jul 14, 2022
@julien-boudry julien-boudry changed the title Path arcenciel [Console] Ansi8 (256 color) support, improve true color (Ansi24) support detection Jul 14, 2022
julien-boudry added a commit to julien-boudry/symfony that referenced this pull request Jul 14, 2022
@julien-boudry julien-boudry changed the title [Console] Ansi8 (256 color) support, improve true color (Ansi24) support detection [Console] Add Ansi8 (256 color) support, improve true color (Ansi24) support detection Jul 15, 2022
@carsonbot
Copy link

Hey!

I think @CupOfTea696 has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

src/Symfony/Component/Console/Color.php Outdated Show resolved Hide resolved
src/Symfony/Component/Console/Color.php Outdated Show resolved Hide resolved
src/Symfony/Component/Console/Tests/ColorTest.php Outdated Show resolved Hide resolved
src/Symfony/Component/Console/Color.php Outdated Show resolved Hide resolved
@julien-boudry
Copy link
Contributor Author

julien-boudry commented Jul 15, 2022

In pratice:

  • Many terminals support Ansi24 (truecolor), but can not be identified (Gnome, Windows Terminal...) as true color. Before this PR, Sf rollback to Ansi4 (16 colors) for them. After this PR, is still often the case yet, but few of them can be added to the Ansi24 detection, and some others can be added at least at the Ansi8 (256 colors) detection (because of env TERM name like xterm-256). But is not possible for Gnome Terminal and Windows Terminal (for example), still unchanged.

  • Almost all terminals support 256 colors (Ansi8). Before this PR, Sf rollback to Ansi4 (16 colors) for them. After this PR, almost all (including Apple Terminal) use conversion from true Ansi 24 (truecolor) to Ansi8 (256 colors). It's better. (but also not Gnome, not Windows)

  • Future scope?

    • Use Ansi24 (truecolor) by default, and degrade only if 256 colors is explicit? (today, it's the opposite) Could work in a large majority of cases (including Gnome or Windows Terminal for many years), has no impact on Apple Terminal, but can lead to some problems with very old or rare terminals. (but Gnome developer says about that: We don't care 😄)
      In fact, conversion to Ansi4 (16 colors) will never happen (because no one declares it explicitly) without using a database like Termcap maybe (but not perfect also).

    • Or go ahead with speculative detection based on other ENV variables (like env OS variable on windows).

@julien-boudry julien-boudry force-pushed the path-arcenciel branch 2 times, most recently from 4d7092c to e0ea32e Compare July 15, 2022 22:24
@julien-boudry julien-boudry requested a review from stof July 15, 2022 22:39
src/Symfony/Component/Console/Tests/TerminalTest.php Outdated Show resolved Hide resolved
src/Symfony/Component/Console/Color.php Outdated Show resolved Hide resolved
src/Symfony/Component/Console/Color.php Outdated Show resolved Hide resolved
src/Symfony/Component/Console/Terminal.php Outdated Show resolved Hide resolved
@julien-boudry julien-boudry force-pushed the path-arcenciel branch 3 times, most recently from 7f8f760 to aaa3f37 Compare July 18, 2022 23:54
@julien-boudry
Copy link
Contributor Author

Note: @fabpot fabbot.io doesn't like Enum with methods. And not also empty line before doc. comments on CASE. (I have removed it here).

@julien-boudry julien-boudry requested a review from stof July 19, 2022 00:02
@julien-boudry
Copy link
Contributor Author

I will change the enum name soon as discussed. Add one more test. And Squash all these commits at the end. And possibly, we will be perfect.

@julien-boudry julien-boudry marked this pull request as draft July 19, 2022 12:12
@julien-boudry julien-boudry force-pushed the path-arcenciel branch 2 times, most recently from 922741b to cfaffa0 Compare July 21, 2022 18:25
julien-boudry added a commit to julien-boudry/symfony that referenced this pull request Jul 21, 2022
…improves terminal support detection (Ansi4, Ansi8, Ansi24)

* The detection of terminal capabilities is always conservatively estimated, without speculation that may result in display errors. But significantly improved, bringing in particular 256 colors compatibility with Apple Terminal (instead of 8).
* Ansi8 is used by converting the RGB hexadecimal to the nearest color.
* The whole corresponding code is refactored and many tests are added for both existing and new features.
@julien-boudry julien-boudry marked this pull request as ready for review July 21, 2022 18:26
@julien-boudry julien-boudry requested a review from stof July 21, 2022 18:28
…improves terminal support detection (Ansi4, Ansi8, Ansi24)

* The detection of terminal capabilities is always conservatively estimated, without speculation that may result in display errors. But significantly improved, bringing in particular 256 colors compatibility with Apple Terminal (instead of 8).
* Ansi8 is used by converting the RGB hexadecimal to the nearest color.
* The whole corresponding code is refactored and many tests are added for both existing and new features.
@fabpot
Copy link
Member

fabpot commented Jul 22, 2022

Thank you @julien-boudry.

@fabpot fabpot merged commit 6c8c93d into symfony:6.2 Jul 22, 2022
fabpot added a commit that referenced this pull request Sep 7, 2022
…lor Mode (julien-boudry)

This PR was merged into the 6.2 branch.

Discussion
----------

[Console] Terminal Color Mode refactoring and force Color Mode

| Q             | A
| ------------- | ---
| Branch?       | 6.2 for features
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| License       | MIT

Continue #46944

Terminal Color Mode refactoring: Adding a way to force color mode by the dev. user (with a new method), fewer `getenv()` calls (cache value), simpler tests (use the a new method and a new constant).

For example, it can be useful in an environment where none of the expected environment variables are available (Docker container...) , but where the support of a specific mode is imperative.

A future evolution could be to add an optional default option to all commands to force a particular mode by the final user.

Commits
-------

4cb8384 [Console] Terminal Color Mode refactoring: Adding a way to force color mode by user, fewer getenv() calls, simpler tests.
@fabpot fabpot mentioned this pull request Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants