Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the ANSI/color output logic by removing custom AI agent detection and aligning with upstream picocolors defaults. The changes simplify color handling by leveraging picocolors' built-in support for NO_COLOR and FORCE_COLOR environment variables, while implementing conditional FORCE_COLOR propagation to worker processes in TTY environments.
Changes:
- Removed AI agent detection logic and the
ynpackage dependency, simplifying color control to rely on standard environment variables - Implemented
getForceColorEnv()to conditionally setFORCE_COLORfor worker processes based on TTY status and color support - Added E2E tests to verify ANSI output behavior with
NO_COLORandFORCE_COLORenvironment variables
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Removed yn package entries |
| packages/core/src/utils/logger.ts | Converted ansiEnabled from function to constant; added getForceColorEnv(); simplified createColors() usage; removed rslog.greet override; updated clearScreen to use isTTY |
| packages/core/src/utils/agent/index.ts | Removed initAgentEnv and isAgentEnv functions, keeping only determineAgent export |
| packages/core/src/reporter/index.ts | Removed redundant ansiEnabled() check in ensureStatusRenderer() |
| packages/core/src/pool/index.ts | Replaced direct ansiEnabled() check with getForceColorEnv() call |
| packages/core/src/core/globalSetup.ts | Applied same getForceColorEnv() pattern for global setup worker pool |
| packages/core/src/core/restart.ts | Changed ansiEnabled() function call to ansiEnabled constant reference |
| packages/core/src/cli/prepare.ts | Removed initAgentEnv() call from CLI initialization |
| packages/core/package.json | Removed yn package from dependencies |
| e2e/snapshot/diff.test.ts | Added test assertions for ANSI escape sequences with FORCE_COLOR |
| e2e/scripts/index.ts | Added unsetEnv parameter support to test helper |
| e2e/reporter/ansi.test.ts | Updated test descriptions and added new test case for FORCE_COLOR behavior |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
human input:
picocolorsandtinyrainbowdefaultsFORCE_COLORpropagation to worker processes to ensure colored snapshot diffs in TTY environmentsisTTYutility to correctly handle CI environmentsmockRuntimeCode.jsand simplified mock type definitionsexamples/react-rsbuildandexamples/vueexamples/reactand synchronized its setup logicNO_COLORandFORCE_COLORenvironment variable priorityRelated Links
Checklist