Skip to content
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

Pattern-based formatter #12

Merged
merged 66 commits into from
Sep 21, 2022
Merged

Pattern-based formatter #12

merged 66 commits into from
Sep 21, 2022

Conversation

Lancern
Copy link
Collaborator

@Lancern Lancern commented Jun 27, 2022

This PR introduces pattern-based formatters.

Related issue: #10

Lancern and others added 30 commits February 25, 2022 14:34
This commit implements the following built-in patterns:
- Full pattern;
- Log level pattern and short log level pattern;
- Source location pattern, source path pattern, source basename pattern, source
  line pattern, and source column pattern;
- Logger name pattern;
- Payload pattern;
- Process ID pattern;
- Thread ID pattern.

Future commits will contain patterns relative to the formatting of date and
time.
This commit seperates the old LocalTimeCacher originally implemented in the full
formatter to a new Cacher util that is more general and reusable.
This commit adds the following built-in patterns:
- Abbreviated month name pattern;
- Month name pattern;
- Abbreviated weekday name pattern;
- Weekday name pattern.
This commit contains the following new built-in patterns:
- Milliseconds pattern;
- Microseconds pattern;
- Nanoseconds pattern.
This commit addes the formatted strings of the following date time parts as new
fields into the local time cacher: year, month, day, hour, minute, and second.
This commit contains the following new built-in pattern implementations:
- Full date time;
- Short year;
- Year;
- Month;
- Day;
- Hour;
- Minute;
- Second.

This commit also contains some refactors.
This commit contains implementations for the following two built-in patterns:
- Time;
- Short time.
This commit contains the following implementations of three new built-in
patterns:
- Hour in 12-hour format;
- Time in 12-hour format;
- UTC timezone offset.
The previous merge (`39d45fc`) from 'main' branch discarded some
changes, so we need to restore them.

We created a new branch 'pattern-formatter-fix-merge' from the last
commit (`c8ecffb`) of the previous merge. In this commit we merge 'main'
branch into the new branch to restore the discarded changes, then in the
next commit we will merge the changes after the previous merge into the
new branch, and finally, we merge the new branch into the current branch
to complete the fix.

In this commit, `winapi` dependency is no longer optional.
The overall workflow of the pattern! procedural macro:
1. Parsing the macro input token stream into:
  - An AST representing the internal structure of the pattern template string.
    The spdlog_macros::parse::PatternTemplate struct represents a node within
    the AST tree;
  - A mapping that associates custom pattern formatters with their names.
    The spdlog_macros::parse::CustomPatternMapping struct holds the mapping.
  The spdlog_macros::parse::Pattern struct represents the parsed artifacts.
2. Synthesising the parsed artifacts into an expression that evaluates to a
   pattern formatter. The expression will be a tuple expression.

The synthesising step is almost done. I'm still working on parsing the pattern
template string into the AST representation. Other parts of the parsing step
are almost done.
@SpriteOvO SpriteOvO force-pushed the pattern-formatter branch 2 times, most recently from 1f7fa92 to 0cafb39 Compare September 18, 2022 08:06
@SpriteOvO
Copy link
Owner

SpriteOvO commented Sep 18, 2022

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 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.
  • After splitting, macro pattern!("template") will return a struct 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, Template and Placeholder, 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? :)

@Lancern
Copy link
Collaborator Author

Lancern commented Sep 18, 2022

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}.

@SpriteOvO
Copy link
Owner

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.

Copy link
Owner

@SpriteOvO SpriteOvO left a comment

Choose a reason for hiding this comment

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

LGTM! Much appreciated!

@SpriteOvO SpriteOvO merged commit da3733d into main Sep 21, 2022
@SpriteOvO SpriteOvO deleted the pattern-formatter branch September 23, 2022 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue for pattern formatters
2 participants