🔥 feat: Add support for ForceColors in Logger middleware #3428
Conversation
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughA new boolean field, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FiberApp
participant LoggerMiddleware
participant Config
User->>FiberApp: Sends HTTP request
FiberApp->>LoggerMiddleware: Handles request
LoggerMiddleware->>Config: Checks ForceColors, DisableColors, terminal/env
alt ForceColors is true
LoggerMiddleware->>LoggerMiddleware: Enable colors in output
else ForceColors is false
LoggerMiddleware->>LoggerMiddleware: Use standard color detection logic
end
LoggerMiddleware->>FiberApp: Continues processing
FiberApp->>User: Returns response
Assessment against linked issues
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@yorickdewid Please update the documentation in the Also add an entry in |
gaby
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3428 +/- ##
==========================================
+ Coverage 83.87% 83.97% +0.10%
==========================================
Files 119 119
Lines 11892 11892
==========================================
+ Hits 9974 9986 +12
+ Misses 1488 1479 -9
+ Partials 430 427 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for forcing colors in the Logger middleware even when the terminal might not normally support them. Key changes include the introduction of a new unit test for ForceColors, modifications in the logger's color handling logic in the default logger file, and updates to the configuration defaults to enable color forcing.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| middleware/logger/logger_test.go | Added a unit test to validate ForceColors behavior |
| middleware/logger/default_logger.go | Adjusted the color handling logic to account for the ForceColors flag |
| middleware/logger/config.go | Updated configuration to include the ForceColors option and update enableColors logic |
Comments suppressed due to low confidence (1)
middleware/logger/default_logger.go:150
- Overwriting the provided Stream unconditionally may cause unexpected behavior, particularly when a custom writer (such as a buffer for testing) is supplied. Consider checking if a custom Stream is already provided before overriding it.
cfg.Stream = colorable.NewColorableStdout()
|
@gaby Where do you want me to add this 'ForceColors' in the Also, Am I seeing this correctly that the use of colors in the output is never actually tested? From what I gather the only testcase doing something with color is |
|
@yorickdewid thanks for this feature, can you check the review hints and add it to our "what´s new" overview |
|
@yorickdewid |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@yorickdewid thanks for the customization |
Description
Test_Logger_ForceColorsto test this behavior.Fixes #3400