Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Console][PhpUnitBridge][VarDumper] Add support forFORCE_COLOR environment variable#57777

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:7.2fromserious-angel:patch-1
Aug 13, 2024

Conversation

@serious-angel
Copy link
Contributor

@serious-angelserious-angel commentedJul 19, 2024•
edited
Loading

QA
Branch?7.2
Bug fix?no
New feature?yes
Deprecations?no
Issues
LicenseMIT

Copy link
ContributorAuthor

@serious-angelserious-angel left a comment•
edited
Loading

Choose a reason for hiding this comment

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

I am not sure aboutCHANGELOG file and how breaking it is.
Please suggest any details if possible.

Related:chalk/supports-color#105

@nicolas-grekas
Copy link
Member

Related also:https://force-color.org/

@nicolas-grekas
Copy link
Member

BTW,https://no-color.org/ tells that the NO_COLOR env var should not be the empty string, and we don't follow this spec currently. I think we should.

serious-angel reacted with thumbs up emoji

@carsonbotcarsonbot changed the titleAdded support for FORCE_COLOR environment variable[VarDumper] Added support for FORCE_COLOR environment variableJul 19, 2024
@OskarStarkOskarStark changed the title[VarDumper] Added support for FORCE_COLOR environment variable[VarDumper] Add support forFORCE_COLOR environment variableJul 19, 2024
nicolas-grekas added a commit that referenced this pull requestJul 26, 2024
…alue handling (alexandre-daubois)This PR was merged into the 5.4 branch.Discussion----------[Console][PhpUnitBridge][VarDumper] Fix `NO_COLOR` empty value handling| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        |#57777 (comment)| License       | MIT`NO_COLOR` must be non-empty in order to be considered enabled (https://no-color.org/):> when present and not an empty string (regardless of its value)Commits-------6a96ff9 [Console][PhpUnitBridge][VarDumper] Fix `NO_COLOR` empty value handling
symfony-splitter pushed a commit to symfony/var-dumper that referenced this pull requestJul 26, 2024
…alue handling (alexandre-daubois)This PR was merged into the 5.4 branch.Discussion----------[Console][PhpUnitBridge][VarDumper] Fix `NO_COLOR` empty value handling| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        |symfony/symfony#57777 (comment)| License       | MIT`NO_COLOR` must be non-empty in order to be considered enabled (https://no-color.org/):> when present and not an empty string (regardless of its value)Commits-------6a96ff9116 [Console][PhpUnitBridge][VarDumper] Fix `NO_COLOR` empty value handling
@alexandre-daubois
Copy link
Member

Hi @artshade, you may have a look at this fix to be sure to update everywhere it is relevant in Symfony:#57815

@serious-angel
Copy link
ContributorAuthor

...you may have a look at this fix to be sure to update everywhere it is relevant in Symfony:#57815

This is awesome! Thank you,@alexandre-daubois ! Have you considered the related forced color option?

@alexandre-daubois
Copy link
Member

You seemed to be well on your way to adding this feature, so I didn't want to impose a PR if you wanted to finish it :)

@serious-angel
Copy link
ContributorAuthor

serious-angel commentedJul 29, 2024•
edited
Loading

You seemed to be well on your way to adding this feature, so I didn't want to impose a PR if you wanted to finish it :)

Changed the priority ofNO_COLOR as in the snippet athttps://force-color.org, whereNO_COLOR comes first and only then -FORCE_COLOR:

if (no_color!=NULL&&no_color[0]!='\0')color= false;// ...if (force_color!=NULL&&force_color[0]!='\0')color= true;

Source

Regarding the PR, I am frankly not sure if the current state of the PR is correct considering all the branches and rules.

Copy link
Member

@derrabusderrabus left a comment

Choose a reason for hiding this comment

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

Thank you. Please add a test.

@carsonbotcarsonbot changed the title[VarDumper] Add support forFORCE_COLOR environment variable[Console][PhpUnitBridge][VarDumper] Add support forFORCE_COLOR environment variableAug 13, 2024
@nicolas-grekas
Copy link
Member

Thank you @artshade.

@nicolas-grekasnicolas-grekas merged commit8fb48a5 intosymfony:7.2Aug 13, 2024
@serious-angel
Copy link
ContributorAuthor

I am sorry for not implementing the test in-time...
Sincere gratitude,@nicolas-grekas, and everyone...

Let it be colors! 🌈 ✨

alexislefebvre reacted with hooray emoji

@serious-angelserious-angel deleted the patch-1 branchAugust 13, 2024 17:02
@fabpotfabpot mentioned this pull requestOct 27, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@derrabusderrabusderrabus requested changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

7.2

Development

Successfully merging this pull request may close these issues.

6 participants

@serious-angel@nicolas-grekas@alexandre-daubois@derrabus@OskarStark@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp