-
Notifications
You must be signed in to change notification settings - Fork 69
fix: Use default text color for class names in log formatters #4139
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Previously class names were colored with Console.WHITE, which doesn't display well on terminals with light/white backgrounds. Now class names use the terminal's default text color, ensuring readability on both light and dark terminal themes. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary of ChangesHello @xerial, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a minor but impactful UI/UX issue in log output. By removing a hardcoded white color for logger class names, the change ensures that log messages are consistently readable regardless of the terminal's background color, improving the overall developer experience when viewing logs. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly removes the hardcoded Console.WHITE color for logger class names in several log formatters. This is a valuable improvement that enhances readability on terminals with light backgrounds by using the default text color. The changes are applied consistently and also improve code formatting by collapsing multi-line strings. I have one suggestion regarding code duplication that has become more apparent with these changes, which could be addressed to improve long-term maintainability.
| Console.WHITE, | ||
| r.leafLoggerName | ||
| )}] ${highlightLog(r.level, r.getMessage())} ${loc}" | ||
| f"${withColor(Console.BLUE, formatTimestamp(r.getMillis()))} ${logTag}%14s [${r.leafLoggerName}] ${highlightLog(r.level, r.getMessage())} ${loc}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is a good improvement. It also highlights the significant code duplication between this formatter and AppLogFormatter. The core log message format is now nearly identical, with this formatter just appending the source location (loc).
To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider extracting the common log string construction into a private helper method. SourceCodeLogFormatter could then call this helper and append the ${loc} information. This would make future changes to the log format easier to manage across different formatters.
Summary
Console.WHITEcoloring from class names (logger names) inAppLogFormatter,SourceCodeLogFormatter, andThreadLogFormatterTest plan
./sbt "logJVM/test"- all 55 tests pass🤖 Generated with Claude Code