Skip to content

Comments

Refactor ANSI handling for better readability#7245

Merged
Evangelink merged 2 commits intomainfrom
dev/ygerges/refactor-terminal-options
Jan 16, 2026
Merged

Refactor ANSI handling for better readability#7245
Evangelink merged 2 commits intomainfrom
dev/ygerges/refactor-terminal-options

Conversation

@Youssef1313
Copy link
Member

Keeping three boolean properties for ANSI handling (UseAnsi, ForceAnsi, UseCIAnsi) is IMO unnecessarily complicated. It creates 8 possible combinations, while in reality we have just 4 possibilities (one of them is test-only).

This refactors the three properties to be a single property of a new enum type that can be NoAnsi, SimpleAnsi, AnsiIfPossible, ForceAnsi.

  • NoAnsi: disable both coloring and cursor movement. User requests it explicitly via --no-ansi
  • SimpleAnsi: enables only coloring. Used in CI, unless the user specifies --no-ansi.
  • AnsiIfPossible: Enables both coloring and cursor movement, but only if we detect that the terminal is capable.
  • ForceAnsi: Used only by unit tests. Force-enables coloring and cursor movement, even if terminal isn't capable.

@Youssef1313 Youssef1313 marked this pull request as ready for review January 16, 2026 17:52
@Youssef1313 Youssef1313 force-pushed the dev/ygerges/refactor-terminal-options branch from 7e73c65 to 7ef3e8b Compare January 16, 2026 17:53
@Evangelink Evangelink merged commit 978738e into main Jan 16, 2026
13 of 14 checks passed
@Evangelink Evangelink deleted the dev/ygerges/refactor-terminal-options branch January 16, 2026 20:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors ANSI handling from three boolean properties (UseAnsi, ForceAnsi, UseCIAnsi) to a single enum property (AnsiMode) with four clearly defined values, significantly improving code clarity and maintainability.

Changes:

  • Introduced AnsiMode enum with four values: NoAnsi, SimpleAnsi, AnsiIfPossible, and ForceAnsi
  • Refactored TerminalTestReporterOptions to use the new AnsiMode enum instead of three boolean properties
  • Updated TerminalTestReporter constructor logic to handle the new enum-based approach
  • Updated TerminalOutputDevice to set the appropriate AnsiMode based on command-line options and CI detection
  • Updated all test cases to use the new AnsiMode enum values

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporterOptions.cs Replaced three boolean properties with a single AnsiMode enum property; added the AnsiMode enum with four well-documented values
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs Refactored constructor logic to use AnsiMode enum with cleaner conditional flow
src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs Updated initialization logic to determine and set the appropriate AnsiMode based on command-line options and CI detection
test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs Updated all test cases to use the new AnsiMode enum values instead of multiple boolean properties

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants