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.

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

[Console] Fix color support for TTY output #53707

Merged
merged 1 commit into from Feb 3, 2024
Merged

[Console] Fix color support for TTY output #53707

merged 1 commit into from Feb 3, 2024

Conversation

theofidry
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #53693
License MIT

Prior to #53576, the output being a TTY was sufficient to consider the output as supporting colors. This is not enough but as a result, we are missing some additional checks.

This PR:

  • Adds a check for the COLORTERM environment variable. From what I've found it does not look this variable is ever set if there is no color support.
  • Adds a check for the screen-* TERM values. Similar to the check we already for xterm, you have screen-256color, screen-256color-bce, screen.xterm-256color or more.

@carsonbot carsonbot added this to the 5.4 milestone Jan 31, 2024
@theofidry theofidry changed the title [Console] Fix color support [Console] Fix color support for TTY output Jan 31, 2024
@chalasr chalasr mentioned this pull request Jan 31, 2024
Closed
@stof
Copy link
Member

stof commented Jan 31, 2024

The supports-color npm package has [2 different kind of checks] (https://github.com/chalk/supports-color/blob/d4f413efaf8da045c5ab440ed418ef02dbb28bf1/index.js#L153-L159) for TERM:

  • whether it ends with -256 or -256color (in which case it reports color support for what we call the Ansi8 mode)
  • whether it starts with a bunch of prefixes (more than just screen- and xterm-)

Should we take the full list there ?

@stof
Copy link
Member

stof commented Feb 2, 2024

@theofidry any chance to finish this PR ?

@theofidry
Copy link
Contributor Author

@stof done

stof
stof approved these changes Feb 2, 2024
View reviewed changes
Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply the fabbot coding standard patch (we use qualified function calls only for compiler-optimized functions in Symfony).

But other than that, this PR is good to me.

@theofidry
Copy link
Contributor Author

I thought I already did, I applied it again although no clue what the diff is 😅 looks like it's happy now at least

@stof
Copy link
Member

stof commented Feb 2, 2024

@theofidry for the diff, see the diff of your force-push: https://github.com/symfony/symfony/compare/76e89cb21a56a42a95c0dbaf1b75438441ee4410..50b0051c78f853d605cbc57209e065b9a7a3e357

This was about removing the \ qualifying the preg_match function name.

@fabpot
Copy link
Member

fabpot commented Feb 3, 2024

Thank you @theofidry.

@fabpot fabpot merged commit 84ae858 into symfony:5.4 Feb 3, 2024
10 of 11 checks passed
@nicolas-grekas nicolas-grekas mentioned this pull request Feb 5, 2024
Merged
nicolas-grekas added a commit that referenced this pull request Feb 7, 2024
@nicolas-grekas
minor #53794 [PhpUnitBridge][VarDumper] Fix color detection (nicolas-…
fa7531f 
…grekas)

This PR was merged into the 5.4 branch.

Discussion
----------

[PhpUnitBridge][VarDumper] Fix color detection

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Porting #53707 to VarDumper and tweaking it a bit.

/cc `@theofidry` FYI

Commits
-------

7ce86dc [VarDumper][PhpUnitBridge] Fix color detection
@melroy89
Copy link

Sorry this was a regression in 6.4.x as well right? Is this now merged into 5.4? I don't understand.

@wouterj
Copy link
Member

wouterj commented Feb 18, 2024

We up merge older branches into newer ones. Since the merge of this PR, 5.4 has been merged in 6.4 (and 7.0 + 7.1-dev). All supported versions will get a release at the end of the month.

@melroy89
Copy link

We up merge older branches into newer ones. Since the merge of this PR, 5.4 has been merged in 6.4 (and 7.0 + 7.1-dev). All supported versions will get a release at the end of the month.

ah OK. got it!

@theofidry theofidry deleted the fix/color-support branch February 20, 2024 08:43
This was referenced Feb 27, 2024
Merged
Merged
Merged
github-merge-queue bot pushed a commit to Lendable/composer-license-checker that referenced this pull request Feb 29, 2024
@renovate
Update Composer runtime dependencies to v6.4.4 (#272)
36a334d 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [symfony/console](https://symfony.com)
([source](https://togithub.com/symfony/console)) | `6.4.3` -> `6.4.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fconsole/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fconsole/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fconsole/6.4.3/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fconsole/6.4.3/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [symfony/process](https://symfony.com)
([source](https://togithub.com/symfony/process)) | `6.4.3` -> `6.4.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fprocess/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fprocess/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fprocess/6.4.3/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fprocess/6.4.3/6.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>symfony/console (symfony/console)</summary>

### [`v6.4.4`](https://togithub.com/symfony/console/releases/tag/v6.4.4)

[Compare
Source](https://togithub.com/symfony/console/compare/v6.4.3...v6.4.4)

**Changelog**
(symfony/console@v6.4.3...v6.4.4)

- bug
[symfony/symfony#54009](https://togithub.com/symfony/symfony/issues/54009)
\[Console] Fix display of vertical Table on Windows OS
([@&#8203;VincentLanglet](https://togithub.com/VincentLanglet))
- bug
[symfony/symfony#54001](https://togithub.com/symfony/symfony/issues/54001)
\[Console] Fix display of Table on Windows OS
([@&#8203;VincentLanglet](https://togithub.com/VincentLanglet))
- bug
[symfony/symfony#53707](https://togithub.com/symfony/symfony/issues/53707)
\[Console] Fix color support for TTY output
([@&#8203;theofidry](https://togithub.com/theofidry))
- bug
[symfony/symfony#53711](https://togithub.com/symfony/symfony/issues/53711)
\[Console] Allow false as a $shortcut in InputOption
([@&#8203;jayminsilicon](https://togithub.com/jayminsilicon))

</details>

<details>
<summary>symfony/process (symfony/process)</summary>

###
[`v6.4.4`](https://togithub.com/symfony/process/compare/v6.4.3...v6.4.4)

[Compare
Source](https://togithub.com/symfony/process/compare/v6.4.3...v6.4.4)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Lendable/composer-license-checker).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMTIuMCIsInVwZGF0ZWRJblZlciI6IjM3LjIxMi4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
github-merge-queue bot pushed a commit to Lendable/composer-license-checker that referenced this pull request Mar 2, 2024
@renovate
Update Composer runtime dependencies to v7 (major) (#282)
dcf106e 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [symfony/console](https://symfony.com)
([source](https://togithub.com/symfony/console)) | `6.4.4` -> `7.0.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fconsole/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fconsole/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fconsole/6.4.4/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fconsole/6.4.4/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [symfony/process](https://symfony.com)
([source](https://togithub.com/symfony/process)) | `6.4.4` -> `7.0.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fprocess/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fprocess/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fprocess/6.4.4/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fprocess/6.4.4/7.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>symfony/console (symfony/console)</summary>

### [`v7.0.4`](https://togithub.com/symfony/console/releases/tag/v7.0.4)

[Compare
Source](https://togithub.com/symfony/console/compare/v7.0.3...v7.0.4)

**Changelog**
(symfony/console@v7.0.3...v7.0.4)

- bug
[symfony/symfony#54009](https://togithub.com/symfony/symfony/issues/54009)
\[Console] Fix display of vertical Table on Windows OS
([@&#8203;VincentLanglet](https://togithub.com/VincentLanglet))
- bug
[symfony/symfony#54001](https://togithub.com/symfony/symfony/issues/54001)
\[Console] Fix display of Table on Windows OS
([@&#8203;VincentLanglet](https://togithub.com/VincentLanglet))
- bug
[symfony/symfony#53707](https://togithub.com/symfony/symfony/issues/53707)
\[Console] Fix color support for TTY output
([@&#8203;theofidry](https://togithub.com/theofidry))
- bug
[symfony/symfony#53711](https://togithub.com/symfony/symfony/issues/53711)
\[Console] Allow false as a $shortcut in InputOption
([@&#8203;jayminsilicon](https://togithub.com/jayminsilicon))

### [`v7.0.3`](https://togithub.com/symfony/console/releases/tag/v7.0.3)

[Compare
Source](https://togithub.com/symfony/console/compare/v7.0.2...v7.0.3)

**Changelog**
(symfony/console@v7.0.2...v7.0.3)

- bug
[symfony/symfony#53516](https://togithub.com/symfony/symfony/issues/53516)
\[Console] Allow '0' as a $shortcut in InputOption.php
([@&#8203;lawsonjl-ornl](https://togithub.com/lawsonjl-ornl))
- bug
[symfony/symfony#53576](https://togithub.com/symfony/symfony/issues/53576)
\[Console] Only execute additional checks for color support if the
output ([@&#8203;theofidry](https://togithub.com/theofidry))

### [`v7.0.2`](https://togithub.com/symfony/console/releases/tag/v7.0.2)

[Compare
Source](https://togithub.com/symfony/console/compare/v7.0.1...v7.0.2)

**Changelog**
(symfony/console@v7.0.1...v7.0.2)

- bug
[symfony/symfony#52940](https://togithub.com/symfony/symfony/issues/52940)
\[Console] Fix color support check on non-Windows platforms
([@&#8203;theofidry](https://togithub.com/theofidry))
- bug
[symfony/symfony#52941](https://togithub.com/symfony/symfony/issues/52941)
\[Console] Fix xterm detection
([@&#8203;theofidry](https://togithub.com/theofidry))

### [`v7.0.1`](https://togithub.com/symfony/console/releases/tag/v7.0.1)

[Compare
Source](https://togithub.com/symfony/console/compare/v7.0.0...v7.0.1)

**Changelog**
(symfony/console@v7.0.0...v7.0.1)

-   no significant changes

### [`v7.0.0`](https://togithub.com/symfony/console/releases/tag/v7.0.0)

[Compare
Source](https://togithub.com/symfony/console/compare/v6.4.4...v7.0.0)

**Changelog**
(symfony/console@v7.0.0-RC2...v7.0.0)

-   no significant changes

</details>

<details>
<summary>symfony/process (symfony/process)</summary>

### [`v7.0.4`](https://togithub.com/symfony/process/releases/tag/v7.0.4)

[Compare
Source](https://togithub.com/symfony/process/compare/v7.0.3...v7.0.4)

**Changelog**
(symfony/process@v7.0.3...v7.0.4)

- bug
[symfony/symfony#54006](https://togithub.com/symfony/symfony/issues/54006)
\[Process] Fix the `command -v` exception (@&#8203;kayw-geek)
- bug
[symfony/symfony#53821](https://togithub.com/symfony/symfony/issues/53821)
\[Process] Fix Inconsistent Exit Status in proc_get_status for PHP
Versions Below 8.3 ([@&#8203;Luc45](https://togithub.com/Luc45))

### [`v7.0.3`](https://togithub.com/symfony/process/releases/tag/v7.0.3)

[Compare
Source](https://togithub.com/symfony/process/compare/v7.0.2...v7.0.3)

**Changelog**
(symfony/process@v7.0.2...v7.0.3)

- bug
[symfony/symfony#53481](https://togithub.com/symfony/symfony/issues/53481)
\[Process] Fix executable finder when the command starts with a dash
([@&#8203;kayw-geek](https://togithub.com/kayw-geek))

### [`v7.0.2`](https://togithub.com/symfony/process/releases/tag/v7.0.2)

[Compare
Source](https://togithub.com/symfony/process/compare/v7.0.0...v7.0.2)

**Changelog**
(symfony/process@v7.0.1...v7.0.2)

- bug
[symfony/symfony#52864](https://togithub.com/symfony/symfony/issues/52864)
\[HttpClient]\[Mailer]\[Process] always pass microseconds to usleep as
integers ([@&#8203;xabbuh](https://togithub.com/xabbuh))

### [`v7.0.0`](https://togithub.com/symfony/process/releases/tag/v7.0.0)

[Compare
Source](https://togithub.com/symfony/process/compare/v6.4.4...v7.0.0)

**Changelog**
(symfony/process@v7.0.0-RC2...v7.0.0)

-   no significant changes

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Lendable/composer-license-checker).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMjAuMiIsInVwZGF0ZWRJblZlciI6IjM3LjIyMC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof approved these changes

@fabpot fabpot fabpot approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Bug Console Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@theofidry @stof @fabpot @melroy89 @wouterj @chalasr @carsonbot