Implement use slog.Leveler and slog.LevelVar. And adding custom log level#180
Implement use slog.Leveler and slog.LevelVar. And adding custom log level#180kondr1 wants to merge 1 commit intocharmbracelet:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for slog.Leveler and slog.LevelVar interfaces while adding custom log level functionality. The changes enable more flexible level management by allowing dynamic level variables and custom level types that implement the slog.Leveler interface.
Key changes:
- Replace direct
Levelusage withslog.Levelerinterface throughout the codebase - Change level storage from
int64to*slog.LevelVarfor dynamic level management - Update styling system to use
intkeys instead ofLevelkeys for better extensibility - Add support for custom levels with proper styling integration
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| logger.go | Core changes to use slog.Leveler interface and *slog.LevelVar for level storage |
| pkg.go | Updated package-level functions and NewWithOptions to handle slog.Leveler types |
| options.go | Changed Options.Level field type from Level to slog.Leveler |
| styles.go | Modified Levels map to use int keys instead of Level keys |
| level.go | Added Level() slog.Level method to implement slog.Leveler interface |
| text.go | Updated text formatter to work with slog.Leveler interface |
| json.go | Enhanced JSON formatter to handle custom levels through the Levels map |
| logger_121.go | Updated slog handler implementation for Go 1.21+ |
| logger_no121.go | Updated slog handler implementation for Go <1.21 |
| *_test.go | Updated all tests to work with new level system and added custom level tests |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
|
||
| // GetLevel returns the level for the default logger. | ||
| func GetLevel() Level { | ||
| func GetLevel() slog.Leveler { |
There was a problem hiding this comment.
The return type change from Level to slog.Leveler is a breaking API change. Consider adding a deprecation notice or maintaining backward compatibility by providing both functions.
| func GetLevel() slog.Leveler { | |
| // | |
| // Deprecated: Use GetLeveler() instead. This will be removed in a future release. | |
| func GetLevel() Level { | |
| return Default().GetLevel().Level() | |
| } | |
| // GetLeveler returns the slog.Leveler for the default logger. | |
| func GetLeveler() slog.Leveler { |
| l.mu.RLock() | ||
| defer l.mu.RUnlock() | ||
| return Level(l.level) | ||
| return l.level.Level() |
There was a problem hiding this comment.
The return type change from Level to slog.Leveler is a breaking API change. The method now returns l.level.Level() instead of the *slog.LevelVar itself, which may not be what callers expect when they need to modify the level dynamically.
| func (l *Logger) SetLevel(level Level) { | ||
| func (l *Logger) SetLevel(level slog.Leveler) { | ||
| l.mu.Lock() | ||
| defer l.mu.Unlock() |
There was a problem hiding this comment.
The parameter type change from Level to slog.Leveler is a breaking API change. Consider maintaining backward compatibility or providing clear migration guidance.
| } else { | ||
| lvl.Set(o.Level.Level()) | ||
| } | ||
| l.level = lvl |
There was a problem hiding this comment.
Using a type switch on an interface without handling the nil case explicitly could lead to unexpected behavior. Consider adding a nil check before the type switch.
| l.level = lvl | |
| if o.Level == nil { | |
| lvl := new(slog.LevelVar) | |
| lvl.Set(slog.LevelInfo) | |
| l.level = lvl | |
| } else { | |
| switch lvlVar := o.Level.(type) { | |
| case *slog.LevelVar: | |
| l.level = lvlVar | |
| default: | |
| lvl := new(slog.LevelVar) | |
| lvl.Set(o.Level.Level()) | |
| l.level = lvl | |
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #180 +/- ##
==========================================
+ Coverage 80.92% 83.09% +2.17%
==========================================
Files 11 11
Lines 739 834 +95
==========================================
+ Hits 598 693 +95
+ Misses 126 125 -1
- Partials 15 16 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
#98
#91