spdlog-rs
spdlog-rs copied to clipboard
Pattern-based formatter
This PR introduces pattern-based formatters.
Related issue: #10
So here are the problems that need to be fixed and problems that are still unresolved after the first round of review:
Fixes WIP
- [x] Change the pattern template string of style ranges from
{^...$}to{^...}; - [x] Limit the names of user-defined custom patterns to match the restrictions of Rust identifiers;
- [x] Replace
-with_in pattern names; - [x] Put
TimeDate::tz_offset_strinto a lazy closure; - [x] Each user-defined pattern should only be associated with one name;
- [x] Remove single-character pattern names;
- [x] Missing a built-in pattern for
{year}-{month}-{day}(the ISO 8601 date format); - [x] Implement
Formatter::clone_boxforPatternFormatter; - [x] Ambiguous wording in the documentation for
Patternstruct - [x] Get local time from the global time cacher in built-in date time patterns;
- [x] Add a new cache value for the short year string;
- [x] Add a
pub(crate)method forLevelthat returns the level names; - [x] Do not include the column number in the output of the
Locpattern; - [x] ~Add
P: Patternbound toPatternFormatter<P>;~ UPDATE: Referred to this answer, bound trait only on allimplblocks,structkeeps unbounded. - [x] Rename
ColorRangetoStyleRange.
Unresolved Questions
- [x] Re-export
spdlog_macros::patterndirectly fromspdlograther than wrapping it behind a macro by example; - [x] ~Too-long pattern names can still make the pattern template string unreadable;~ So far so good.
- [x] Remove
pattern_formatter!.
Looks like all problems from the first round of review have been solved! 🎉
There should be only one final question left, and I've been thinking about it. I'd like to try to split the current trait Pattern into two concepts, struct Pattern + trait PatternEntity (or PatternFlag or other name?).
The motivations:
- Semantically, a
Patternis supposed to be a combination, but now a single placeholder is also considered aPattern. A formatter with only a single placeholder should not make much sense to users. - After splitting, macro
pattern!("template")will return astruct Pattern<(impl PatternEntity, impl PatternEntity, ...)>, and we need a wrapper like this to prevent users from accessing the tuple, as it is implementation details. - In the future we may support compiling patterns at runtime, and after splitting we can add function
fn Pattern::compile("template") -> Pattern<RuntimePattern>for this purpose. - In the current documentation, there is some confusion between the terms of
Pattern,TemplateandPlaceholder, which can be confusing for users to read. - This split change should not add any performance cost.
Simply put, this split just adds a wrapper to the existing trait Pattern combination.
@Lancern What is your opinion? :)
I'd like to try to split the current trait Pattern into two concepts, struct Pattern + trait PatternEntity (or PatternFlag or other name?).
I'd argue that this actually limits the ability to combine different patterns. An important usage is that, users can use built-in patterns as building blocks for their own patterns; and again, these user-crafted patterns can also be used as building blocks for even larger patterns. If pattern! returns a struct Pattern, it won't be able to be used for building larger patterns, unless we implement trait PatternEntity for struct Pattern. But then why we split trait Pattern in the first place?
Semantically, a Pattern is supposed to be a combination, but now a single placeholder is also considered a Pattern. A formatter with only a single placeholder should not make much sense to users.
Patterns can be freely combined does not mean patterns are combinations. A single placeholder as a pattern can also be useful; for instance, {full} or {payload}.
Yeah, you're right, I'll drop that idea.
I'll do some more checking and cleanup, and if there are no more issues this PR should be merged in the next few days.