Skip to content

feat(framework): Use rich to render colors, fixing color-rendering issues on Windows#6626

Draft
panh99 wants to merge 9 commits intomainfrom
fix-log-colors
Draft

feat(framework): Use rich to render colors, fixing color-rendering issues on Windows#6626
panh99 wants to merge 9 commits intomainfrom
fix-log-colors

Conversation

@panh99
Copy link
Member

@panh99 panh99 commented Feb 25, 2026

No description provided.

@panh99 panh99 requested a review from tanertopal as a code owner February 25, 2026 11:22
Copilot AI review requested due to automatic review settings February 25, 2026 11:22
@panh99 panh99 requested a review from danieljanes as a code owner February 25, 2026 11:22
@panh99 panh99 marked this pull request as draft February 25, 2026 11:24
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 updates Flower’s Python console logging to use rich for color rendering, aiming to fix color output issues on Windows terminals.

Changes:

  • Replace ANSI escape-code coloring with rich styles for console log level rendering.
  • Rework ConsoleHandler to render and print formatted log output via rich.Console.
  • Simplify update_console_handler to update the module’s global console_handler directly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +82 to +93
message = MESSAGE_FORMATTER.format(record)
formatted_time = TIME_FORMATTER.formatTime(record)
if self.json:
record.message = record.getMessage().replace("\t", "").strip()

# Check if the message is empty
if not record.message:
message = message.replace("\t", "").strip()
if not message:
return

super().emit(record)

def format(self, record: LogRecord) -> str:
"""Format function that adds colors to log level."""
seperator = " " * (8 - len(record.levelname))
if self.json:
log_fmt = "{lvl='%(levelname)s', time='%(asctime)s', msg='%(message)s'}"
renderable: str | Text = f"{{lvl='{record.levelname}', "
renderable += f"time='{formatted_time}', msg='{message}'}}"
else:
log_fmt = (
f"{LOG_COLORS[record.levelname] if self.colored else ''}"
f"%(levelname)s {'%(asctime)s' if self.timestamps else ''}"
f"{LOG_COLORS['RESET'] if self.colored else ''}"
f": {seperator} %(message)s"
)
formatter = logging.Formatter(log_fmt)
return formatter.format(record)
separator = " " * (8 - len(record.levelname))
timestamp = f" {formatted_time}" if self.timestamps else ""
head = f"{record.levelname}{timestamp}"
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

formatted_time = TIME_FORMATTER.formatTime(record) is computed for every record even when self.json is false and self.timestamps is false, where the value is unused. Moving time formatting inside the branches that need it would avoid unnecessary strftime/time conversion work in the hot path.

Copilot uses AI. Check for mistakes.
markup=False,
no_color=not colored,
)

Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

Console is initialized with file=self.stream, but elsewhere in this module console_handler.stream is reassigned (e.g., mirror_output_to_queue, redirect_output, restore_output) to capture/redirect output. rich.Console keeps its own file reference, so log output will continue going to the original stream instead of the updated handler.stream, which can break CLI JSON output capture/redirection. Consider overriding setStream/adding a setter that updates self.console.file whenever self.stream changes, or re-create/update the Console in emit() based on the current stream.

Suggested change
def setStream(self, stream: TextIO | None) -> None: # type: ignore[override]
"""Set the stream and keep the Console's file in sync."""
super().setStream(stream)
# Ensure the rich Console writes to the same stream as the handler.
# logging.StreamHandler.setStream sets self.stream, which may be None.
# rich.Console expects a file-like object; if self.stream is None,
# Console will continue using its existing file.
if hasattr(self, "console") and self.stream is not None:
self.console.file = self.stream

Copilot uses AI. Check for mistakes.
@github-actions github-actions bot added the Maintainer Used to determine what PRs (mainly) come from Flower maintainers. label Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintainer Used to determine what PRs (mainly) come from Flower maintainers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants