refactor: acta#21
Open
Sn0wo2 wants to merge 13 commits into
Open
Conversation
📝 Walkthrough走查该 PR 对 acta 的颜色处理、配置系统和初始化流程进行了系统性重构。核心变化包括引入 Styled 结构体取代函数式颜色转换、将 Filter 配置改为紧凑字符串形式、简化 LayerBuilder 和 init 流程，以及清理异步写入器的计数机制。同时更新了文档、演示程序和相关测试。 变更概览核心系统重构
文档与演示
🎯 4 (复杂) | ⏱️ ~75 分钟
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's Guide重构 acta 的 tracing 设置以使用 boxed layers 和基于向量的分层 subscriber，引入带有 serde 支持的基于 Filter 的配置模型，简化围绕 init/TracingGuard 的公共 API，集中管理颜色/样式处理，使路径宽度计算可通过 acta-build 在构建时配置，并相应更新文档和测试。 通过 TracingGuard 在运行时重新加载过滤器的时序图sequenceDiagram
actor User
participant App
participant TracingGuard
participant ReloadHandle
participant EnvFilter
User->>App: request filter update
App->>TracingGuard: set_level(level) / set_filter(filter)
TracingGuard->>EnvFilter: EnvFilter::try_new(filter.as_directive())
EnvFilter-->>TracingGuard: EnvFilter
TracingGuard->>ReloadHandle: modify(closure_set_envfilter)
ReloadHandle-->>TracingGuard: Result
TracingGuard-->>App: Result
App-->>User: filtering updated
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideRefactors acta’s tracing setup to use boxed layers and a vector-based layered subscriber, introduces a new Filter-based configuration model with serde support, simplifies the public API around init/TracingGuard, centralizes color/style handling, makes path-width computation build-time configurable via acta-build, and updates docs/tests accordingly. Sequence diagram for runtime filter reload via TracingGuardsequenceDiagram
actor User
participant App
participant TracingGuard
participant ReloadHandle
participant EnvFilter
User->>App: request filter update
App->>TracingGuard: set_level(level) / set_filter(filter)
TracingGuard->>EnvFilter: EnvFilter::try_new(filter.as_directive())
EnvFilter-->>TracingGuard: EnvFilter
TracingGuard->>ReloadHandle: modify(closure_set_envfilter)
ReloadHandle-->>TracingGuard: Result
TracingGuard-->>App: Result
App-->>User: filtering updated
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the subscriber initialization by replacing a complex layered type system with a vector of boxed layers and simplifies the configuration and filtering logic. It also removes several dependencies, including arrayvec, smart-default, and derive_more. Feedback identifies critical bugs in the path formatting logic, including incorrect string manipulation that reverses directory names and potential panics due to unsafe string slicing on non-character boundaries. Additionally, the reviewer raised concerns regarding performance regressions caused by introducing heap allocations in the logging hot path.
Owner
Author
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 5 个问题，并提供了一些高层次的反馈：
- 新的
Formatter::format_path实现比较难以理解（多次分配、先反转再切分的逻辑等）；建议把截断逻辑抽取到一个有文档的小辅助函数中，并简化目录尾部计算逻辑，这样预期行为会更清晰、更容易推理。 - 在
color::style::Styled::from中，ANSI16 映射每次转换都会新建一个 16 元素数组并调用一次Palette::default()；你可以通过复用一个静态的调色板和颜色表（或者一个小的辅助函数），避免每次调用都重新构建它们，从而降低开销。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `Formatter::format_path` implementation is quite hard to follow (multiple allocations, reverse-then-split logic, etc.); consider extracting the truncation logic into a small, documented helper and simplifying the directory tail calculation so the intended behavior is clearer and easier to reason about.
- In `color::style::Styled::from`, the ANSI16 mapping allocates a fresh 16-element array and calls `Palette::default()` on every conversion; you could reduce overhead by reusing a static palette and color table (or a small helper function) instead of rebuilding them per call.
## Individual Comments
### Comment 1
<location path="src/builder.rs" line_range="18-27" />
<code_context>
- }
-}
+pub(crate) type BoxedLayer = Box<dyn tracing_subscriber::Layer<Registry> + Send + Sync>;
+pub(crate) type InnerSubscriber = Layered<Vec<BoxedLayer>, Registry>;
+pub(crate) type ReloadHandle =
+ tracing_subscriber::reload::Handle<tracing_subscriber::EnvFilter, InnerSubscriber>;
</code_context>
<issue_to_address>
**issue (bug_risk):** 使用 `Vec<BoxedLayer>` 作为 `Registry` 的 `Layer` 看起来并不安全，而且很可能会因为孤儿规则（orphan rules）而无法通过编译。
`InnerSubscriber` 假设 `Vec<BoxedLayer>` 实现了 `Layer<Registry>`，从而让 `Registry::default().with(layers)` 能通过类型检查，但你不能在这里添加 `impl Layer<Registry> for Vec<BoxedLayer>`，因为根据孤儿规则，`Layer` 和 `Vec` 都是外部类型。这意味着在当前 crate 中不可能让 `Vec<BoxedLayer>` 满足 `Layer<S>` 约束。
如果现在代码是可以编译的，很可能说明 `InnerSubscriber` 已经不再匹配实际的订阅者类型，因此 `reload::Handle<EnvFilter, InnerSubscriber>` 也是不正确的。相反，建议使用一个本地的新类型包装，例如 `struct Layers(Vec<BoxedLayer>); impl<S> Layer<S> for Layers where S: Subscriber + LookupSpan<'_> { … }`，然后在 `.with(...)` 和 `InnerSubscriber` 类型别名中都使用 `Layers`。
</issue_to_address>
### Comment 2
<location path="src/fmt/mod.rs" line_range="135-144" />
<code_context>
+ pub(crate) fn format_path(&self, file: &str, line: u32) -> String {
</code_context>
<issue_to_address>
**issue (bug_risk):** 新的路径压缩逻辑比较难以推理，而且很可能没有产生你期望的目录前缀。
新的实现会对 `[..last_slash]` 做反转，然后在反转后的字符串上按 `'/'` 切分。由于目录分隔符也跟着字符一起被反转，得到的片段不再和原始路径组件一一对应，很难真正表示“仍然能容纳的尾部目录名”。它还在一个热点路径上创建了若干中间 `String`，并做了额外的逐 `char` 处理。
更简单、更安全的方法是在概念上保持旧行为：在正向路径字节上操作，从左侧裁剪直到 `len <= max_width`，如果发生了裁剪就前置 `…`。如果你想保留文件名加部分目录上下文，可以：
1. 构造 `file_with_line = format!("{filename}:{line}")`。
2. 如果长度合适，就像现在一样右对齐。
3. 如果不合适，就计算在保证 `format!("{tail}:{line}")` 能放得下的前提下，可以保留 `path_str` 末尾的多少内容，或者退回到 `…` + 后缀的逻辑——整个过程不需要反转字符串。
这样逻辑更容易理解，也可以避免被破坏的目录名称。
</issue_to_address>
### Comment 3
<location path="src/fmt/test.rs" line_range="9" />
<code_context>
fn formatter_defaults() {
let fmt = Formatter::new();
assert_eq!(fmt.time_format, "%H:%M:%S");
- assert_eq!(fmt.path_width, BUILD_PATH_WIDTH);
+ assert_eq!(fmt.path_width, DEFAULT_PATH_WIDTH);
assert!(fmt.show_path);
assert!(fmt.show_spans);
</code_context>
<issue_to_address>
**suggestion (testing):** 为新的 `Formatter::format_path` 截断和规范化逻辑添加有针对性的测试。
当前新的 `Formatter::format_path` 逻辑（基于字符串的截断、`src/` 与 `src\` 的处理、省略号以及目录尾部启发式规则）除了这个默认宽度检查外，没有被覆盖到。
请添加一些聚焦的测试，用来：
- 覆盖短路径（无需截断，右对齐），
- 覆盖会变成相对路径并被截断的长 `src/` 路径，
- 验证 Windows 风格路径被规范为 `/`，
- 覆盖 `file_with_line.len() + 2 <= max_width` 这个分支，
- 覆盖最终带前导省略号的分支。
你可以通过将 `fmt.path_width` 设置为较小值并断言精确的格式化字符串，让这些测试具有确定性。
Suggested implementation:
```rust
fn formatter_defaults() {
let fmt = Formatter::new();
assert_eq!(fmt.time_format, "%H:%M:%S");
assert_eq!(fmt.path_width, DEFAULT_PATH_WIDTH);
assert!(fmt.show_path);
assert!(fmt.show_spans);
}
#[test]
fn format_path_short_no_truncation_right_aligned() {
let mut fmt = Formatter::new();
// Wide enough that no truncation happens
fmt.path_width = 40;
let formatted = fmt.format_path("src/lib.rs", 10);
// No truncation markers
assert!(!formatted.contains('…'), "expected no ellipsis: {formatted}");
assert!(!formatted.starts_with("..."), "expected no ascii ellipsis: {formatted}");
// Should end with the full file-with-line tail and be right-aligned within the field
assert!(formatted.ends_with("src/lib.rs:10"), "expected to end with full path: {formatted}");
assert!(
formatted.len() <= fmt.path_width,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
#[test]
fn format_path_long_src_path_truncated_relative() {
let mut fmt = Formatter::new();
// Force truncation even after making it relative to src/
fmt.path_width = 24;
let formatted = fmt.format_path(
"/home/user/project/src/module/submodule/deeply_nested_file_name.rs",
42,
);
// Should be made relative to src/ and then truncated
assert!(
!formatted.contains("/home/user/project/"),
"expected path to be made relative to src/: {formatted}",
);
assert!(
formatted.contains("deeply_nested_file_name.rs:42") || formatted.contains("file_name.rs:42"),
"expected file name with line to be present in truncated output: {formatted}",
);
// Truncation marker and width bound
assert!(
formatted.contains('…') || formatted.starts_with("..."),
"expected some form of ellipsis/truncation marker: {formatted}",
);
assert!(
formatted.len() <= fmt.path_width,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
#[test]
fn format_path_windows_style_paths_normalized() {
let mut fmt = Formatter::new();
fmt.path_width = 40;
let formatted = fmt.format_path(r"C:\project\src\module\file.rs", 7);
// Windows separators should be normalized to '/'
assert!(
!formatted.contains('\\'),
"expected backslashes to be normalized out: {formatted}",
);
assert!(
formatted.contains("src/module/file.rs:7"),
"expected normalized src path with line: {formatted}",
);
}
#[test]
fn format_path_hits_file_with_line_branch() {
let mut fmt = Formatter::new();
// Choose a width such that `file_with_line.len() + 2 <= max_width`
// so that branch is exercised without needing truncation.
let file_with_line = "src/main.rs:1";
let max_width = file_with_line.len() + 2;
fmt.path_width = max_width as u16;
let formatted = fmt.format_path("src/main.rs", 1);
// We expect the full file-with-line to appear without truncation markers
assert!(
formatted.contains(file_with_line),
"expected full file_with_line in output: {formatted}",
);
assert!(
!formatted.contains('…') && !formatted.starts_with("..."),
"expected no ellipsis for file_with_line branch: {formatted}",
);
assert!(
formatted.len() <= fmt.path_width as usize,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
#[test]
fn format_path_leading_ellipsis_for_very_long_paths() {
let mut fmt = Formatter::new();
// Very narrow width to ensure we hit the final leading-ellipsis truncation branch
fmt.path_width = 16;
let formatted = fmt.format_path(
"/very/long/prefix/that/is/not/src/and/should/be/trimmed/file.rs",
99,
);
// Final branch should produce a leading ellipsis and keep just the tail
assert!(
formatted.starts_with('…') || formatted.starts_with("..."),
"expected leading ellipsis for aggressively truncated path: {formatted}",
);
assert!(
formatted.contains("file.rs:99"),
"expected file name with line to be preserved in final-branch truncation: {formatted}",
);
assert!(
formatted.len() <= fmt.path_width,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
```
These tests assume:
1. `Formatter::format_path(&self, path: &str, line: u32) -> String` exists and is accessible from this module.
2. `Formatter::path_width` is a public or otherwise settable numeric field (here treated as `u16`; adjust the cast or type if it differs).
3. The implementation normalizes `\` to `/`, makes absolute `src/`-containing paths relative to `src/`, and uses either the Unicode ellipsis `…` or a `"..."` prefix for truncation.
If the actual behavior differs (e.g., different ellipsis marker, no right-alignment, different relativization rule), update the assertions to match the concrete formatting rules while keeping the test structure (short/no truncation, long `src/` + truncation, Windows normalization, `file_with_line.len() + 2 <= max_width` branch, and final leading-ellipsis branch) the same.
</issue_to_address>
### Comment 4
<location path="src/test.rs" line_range="18-27" />
<code_context>
+fn build_test_guard(level: Level, style: Style) -> (TracingGuard, TestSubscriber) {
</code_context>
<issue_to_address>
**suggestion (testing):** 为新的 `TracingGuard::set_filter` 以及带原始指令（raw directive）的 level/target API 添加测试覆盖。
现有测试覆盖了 `with_style`、`set_target_level` 和 `remove_target_level`，但新的 `TracingGuard::set_filter` API 和更严格的 `Level`/`Filter` 语义目前还没有测试。
请添加一些测试，例如：
- 通过 `build_test_guard(Level::Info, ...)` 构建一个 guard，调用 `set_filter(Filter::from_directive("info,my_crate=debug"))`，并断言 `guard.filter.as_directive()` 匹配该指令且调用返回 `Ok(())`。
- 调用 `set_level(Level::Warn)` 并断言 `guard.filter.as_directive()` 变为 `"warn"` 且没有错误。
- 在基于原始指令的 `set_filter` 之后调用 `set_target_level("demo", Level::Trace)`，并断言每个 target 的覆盖设置被正确反映且调用成功。
这些测试将有助于锁定新的公共 API 行为，而无需依赖 `EnvFilter` 的内部细节。
Suggested implementation:
```rust
fn build_test_guard(level: Level, style: Style) -> (TracingGuard, TestSubscriber) {
let filter = Filter::new(level);
let env_filter =
tracing_subscriber::EnvFilter::try_new(filter.as_directive()).unwrap_or_default();
let (env_layer, raw): (_, ReloadHandle) = tracing_subscriber::reload::Layer::new(env_filter);
let layers: Vec<BoxedLayer> = vec![
tracing_subscriber::fmt::Layer::default()
.with_writer(io::sink)
.boxed(),
];
// NOTE: The rest of `build_test_guard` is assumed to remain unchanged.
}
#[cfg(test)]
mod tracing_guard_filter_tests {
use super::*;
#[test]
fn set_filter_with_raw_directive_updates_guard() {
// Start from an initial guard with a simple level-based filter.
let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default());
// Apply a raw directive that includes a per-target override.
let filter = Filter::from_directive("info,my_crate=debug");
guard
.set_filter(filter)
.expect("set_filter with raw directive should succeed");
// The guard's filter should now match the directive we provided.
assert_eq!(
guard.filter.as_directive(),
"info,my_crate=debug",
"guard.filter.as_directive() should reflect the raw directive applied via set_filter"
);
}
#[test]
fn set_level_after_filter_replaces_with_simple_level_directive() {
let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default());
// First, set a more complex filter.
guard
.set_filter(Filter::from_directive("info,my_crate=debug"))
.expect("set_filter should succeed");
// Now replace it with a simple level using the level API.
guard
.set_level(Level::Warn)
.expect("set_level(Level::Warn) should succeed");
// After calling set_level, the directive should be a simple level directive.
assert_eq!(
guard.filter.as_directive(),
"warn",
"set_level should replace the filter with a simple level directive"
);
}
#[test]
fn set_target_level_after_raw_directive_adds_per_target_override() {
let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default());
guard
.set_filter(Filter::from_directive("info,my_crate=debug"))
.expect("initial set_filter should succeed");
// Add a per-target override after the raw directive has been applied.
guard
.set_target_level("demo", Level::Trace)
.expect("set_target_level should succeed after set_filter");
let directive = guard.filter.as_directive();
// The overall directive should still contain the base level and original override.
assert!(
directive.contains("info"),
"directive should still include base level after set_target_level: {directive}"
);
assert!(
directive.contains("my_crate=debug"),
"directive should retain existing per-target override after set_target_level: {directive}"
);
assert!(
directive.contains("demo=trace"),
"directive should include the new per-target override added via set_target_level: {directive}"
);
}
```
The replacement above assumes:
1. The snippet you shared is the full body of `build_test_guard`. If there is additional code in that function, reinsert it between the `let layers: Vec<BoxedLayer> = vec![ ... ];` line and the closing `}` so the helper continues to build and return a valid `(TracingGuard, TestSubscriber)`.
2. `Style` implements `Default`. If it does not, replace `Style::default()` in the tests with whatever style constructor you use elsewhere in your test suite (for example, `Style::Pretty` or a crate-specific default).
3. `Level` uses `CamelCase` variants (`Level::Info`, `Level::Warn`, `Level::Trace`) as implied by your review comment. If instead you re-export `tracing::Level`, adjust the variants to `Level::INFO`, `Level::WARN`, and `Level::TRACE`.
4. `Filter::from_directive(&str)` and `guard.filter` are available as shown in your comment. If `filter` is not public, replace direct field access with the appropriate accessor (for example, `guard.filter().as_directive()`).
</issue_to_address>
### Comment 5
<location path="src/color/style.rs" line_range="10" />
<code_context>
- let r = rgb.0;
- let g = rgb.1;
- let b = rgb.2;
+#[derive(Clone, Copy, Debug)]
+pub struct Styled {
+ rgb: Rgb,
</code_context>
<issue_to_address>
**issue (complexity):** 建议在 `Styled` 之上重新引入简单的公共辅助函数，并把 ANSI16 表提升为常量，这样既能让 API 意图更清晰，也能让实现更精简。
你可以保留现在新的共享转换逻辑，同时恢复更简单、更具意图表达能力的 API，并降低部分内部复杂度。
### 1. 在 `Styled` 之上重新引入小的辅助函数
暴露和之前相同形状的辅助函数，把 `Styled` + `From<Styled>` 作为内部实现细节。这可以保持调用点简单，避免泄露 `Styled` 这个抽象：
```rust
pub fn rgb_to_owo(rgb: (u8, u8, u8), depth: ColorDepth) -> OwoStyle {
Styled::new(Rgb(rgb.0, rgb.1, rgb.2), depth).into()
}
pub fn rgb_to_owo_on(r: u8, g: u8, b: u8, depth: ColorDepth) -> OwoStyle {
Styled::new(Rgb(r, g, b), depth).on().into()
}
/// Dims the foreground by dividing brightness by 4 to ensure high text legibility.
pub fn theme_fg_dimmed(rgb: (u8, u8, u8), depth: ColorDepth) -> OwoStyle {
Styled::new(Rgb(rgb.0, rgb.1, rgb.2), depth).dimmed().into()
}
```
这样调用点就可以恢复成：
```rust
let style = rgb_to_owo(theme.text, self.color_depth);
// or
let style = rgb_to_owo_on(bg.0, bg.1, bg.2, self.color_depth);
// or
let style = theme_fg_dimmed(theme.text, self.color_depth);
```
同时仍然可以在 `From<Styled>` 内部复用更新后的映射逻辑。
如果不希望 `Styled` 成为公共类型，也可以将其设为 `pub(crate)`，并保留这些辅助函数作为公共 API。
### 2. 避免在每次调用时重建 ANSI16 颜色表
目前 `Ansi16` 分支在每次转换时都会分配一个 16 元素数组。你可以通过将其提升为常量来降低复杂度和开销：
```rust
const ANSI16_TABLE: [AnsiColors; 16] = [
AnsiColors::Black,
AnsiColors::Red,
AnsiColors::Green,
AnsiColors::Yellow,
AnsiColors::Blue,
AnsiColors::Magenta,
AnsiColors::Cyan,
AnsiColors::White,
AnsiColors::BrightBlack,
AnsiColors::BrightRed,
AnsiColors::BrightGreen,
AnsiColors::BrightYellow,
AnsiColors::BrightBlue,
AnsiColors::BrightMagenta,
AnsiColors::BrightCyan,
AnsiColors::BrightWhite,
];
```
然后：
```rust
ColorDepth::Ansi16 => {
let idx = rgb_to_ansi(s.as_tuple().into(), Palette::default()) as usize;
let ansi = ANSI16_TABLE
.get(idx)
.unwrap_or(&AnsiColors::White);
if s.on {
return style.on_color(ansi);
}
style.color(ansi)
}
```
这些改动可以保留所有新的行为（变暗处理、背景色、更新后的映射），同时让公共使用方式更简单、内部实现也更精简。
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据反馈改进以后的评审。
Original comment in English
Hey - I've found 5 issues, and left some high level feedback:
- The new
Formatter::format_pathimplementation is quite hard to follow (multiple allocations, reverse-then-split logic, etc.); consider extracting the truncation logic into a small, documented helper and simplifying the directory tail calculation so the intended behavior is clearer and easier to reason about. - In
color::style::Styled::from, the ANSI16 mapping allocates a fresh 16-element array and callsPalette::default()on every conversion; you could reduce overhead by reusing a static palette and color table (or a small helper function) instead of rebuilding them per call.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `Formatter::format_path` implementation is quite hard to follow (multiple allocations, reverse-then-split logic, etc.); consider extracting the truncation logic into a small, documented helper and simplifying the directory tail calculation so the intended behavior is clearer and easier to reason about.
- In `color::style::Styled::from`, the ANSI16 mapping allocates a fresh 16-element array and calls `Palette::default()` on every conversion; you could reduce overhead by reusing a static palette and color table (or a small helper function) instead of rebuilding them per call.
## Individual Comments
### Comment 1
<location path="src/builder.rs" line_range="18-27" />
<code_context>
- }
-}
+pub(crate) type BoxedLayer = Box<dyn tracing_subscriber::Layer<Registry> + Send + Sync>;
+pub(crate) type InnerSubscriber = Layered<Vec<BoxedLayer>, Registry>;
+pub(crate) type ReloadHandle =
+ tracing_subscriber::reload::Handle<tracing_subscriber::EnvFilter, InnerSubscriber>;
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `Vec<BoxedLayer>` as a `Layer` for `Registry` appears unsound and likely won’t compile due to orphan rules.
`InnerSubscriber` assumes `Vec<BoxedLayer>` implements `Layer<Registry>` so that `Registry::default().with(layers)` type-checks, but you can’t add `impl Layer<Registry> for Vec<BoxedLayer>` here due to orphan rules (both `Layer` and `Vec` are foreign). That makes it impossible for `Vec<BoxedLayer>` to satisfy the `Layer<S>` bound in this crate.
If this currently compiles, it likely means `InnerSubscriber` no longer matches the actual subscriber type, so `reload::Handle<EnvFilter, InnerSubscriber>` would be incorrect. Instead, consider a local newtype wrapper, e.g. `struct Layers(Vec<BoxedLayer>); impl<S> Layer<S> for Layers where S: Subscriber + LookupSpan<'_> { … }`, and use `Layers` both in `.with(...)` and in the `InnerSubscriber` alias.
</issue_to_address>
### Comment 2
<location path="src/fmt/mod.rs" line_range="135-144" />
<code_context>
+ pub(crate) fn format_path(&self, file: &str, line: u32) -> String {
</code_context>
<issue_to_address>
**issue (bug_risk):** The new path-compaction logic is hard to reason about and likely not producing the intended directory prefix.
The new implementation reverses `[..last_slash]` and then splits that reversed string on `'/'`. Because the directory separators are reversed along with the characters, the resulting segments don’t map cleanly to original path components and are unlikely to represent “the trailing directory name that still fits.” It also creates several intermediate `String`s and does extra `char`-level work on a hot path.
A simpler, safer approach is to keep the old behaviour conceptually: operate on the forward path bytes, trim from the left until `len <= max_width`, and prepend `…` if trimming occurred. If you want to preserve filename + some directory context:
1. Build `file_with_line = format!("{filename}:{line}")`.
2. If it fits, right-align as you do now.
3. If not, compute how much of the *end* of `path_str` can be kept so that `format!("{tail}:{line}")` fits, or fall back to `…` + suffix logic—without reversing the string.
This keeps the logic easier to reason about and avoids mangled directory names.
</issue_to_address>
### Comment 3
<location path="src/fmt/test.rs" line_range="9" />
<code_context>
fn formatter_defaults() {
let fmt = Formatter::new();
assert_eq!(fmt.time_format, "%H:%M:%S");
- assert_eq!(fmt.path_width, BUILD_PATH_WIDTH);
+ assert_eq!(fmt.path_width, DEFAULT_PATH_WIDTH);
assert!(fmt.show_path);
assert!(fmt.show_spans);
</code_context>
<issue_to_address>
**suggestion (testing):** Add focused tests for the new `Formatter::format_path` truncation and normalization logic.
The new `Formatter::format_path` logic (string-based truncation, `src/` vs `src\` handling, ellipsis, and directory-tail heuristics) isn’t covered beyond this default width check.
Please add focused tests that:
- Cover short paths (no truncation, right-aligned),
- Cover long `src/` paths that become relative and truncated,
- Verify Windows-style paths are normalized to `/`,
- Hit the `file_with_line.len() + 2 <= max_width` branch,
- Hit the final leading-ellipsis branch.
You can make these deterministic by setting `fmt.path_width` to a small value and asserting exact formatted strings.
Suggested implementation:
```rust
fn formatter_defaults() {
let fmt = Formatter::new();
assert_eq!(fmt.time_format, "%H:%M:%S");
assert_eq!(fmt.path_width, DEFAULT_PATH_WIDTH);
assert!(fmt.show_path);
assert!(fmt.show_spans);
}
#[test]
fn format_path_short_no_truncation_right_aligned() {
let mut fmt = Formatter::new();
// Wide enough that no truncation happens
fmt.path_width = 40;
let formatted = fmt.format_path("src/lib.rs", 10);
// No truncation markers
assert!(!formatted.contains('…'), "expected no ellipsis: {formatted}");
assert!(!formatted.starts_with("..."), "expected no ascii ellipsis: {formatted}");
// Should end with the full file-with-line tail and be right-aligned within the field
assert!(formatted.ends_with("src/lib.rs:10"), "expected to end with full path: {formatted}");
assert!(
formatted.len() <= fmt.path_width,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
#[test]
fn format_path_long_src_path_truncated_relative() {
let mut fmt = Formatter::new();
// Force truncation even after making it relative to src/
fmt.path_width = 24;
let formatted = fmt.format_path(
"/home/user/project/src/module/submodule/deeply_nested_file_name.rs",
42,
);
// Should be made relative to src/ and then truncated
assert!(
!formatted.contains("/home/user/project/"),
"expected path to be made relative to src/: {formatted}",
);
assert!(
formatted.contains("deeply_nested_file_name.rs:42") || formatted.contains("file_name.rs:42"),
"expected file name with line to be present in truncated output: {formatted}",
);
// Truncation marker and width bound
assert!(
formatted.contains('…') || formatted.starts_with("..."),
"expected some form of ellipsis/truncation marker: {formatted}",
);
assert!(
formatted.len() <= fmt.path_width,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
#[test]
fn format_path_windows_style_paths_normalized() {
let mut fmt = Formatter::new();
fmt.path_width = 40;
let formatted = fmt.format_path(r"C:\project\src\module\file.rs", 7);
// Windows separators should be normalized to '/'
assert!(
!formatted.contains('\\'),
"expected backslashes to be normalized out: {formatted}",
);
assert!(
formatted.contains("src/module/file.rs:7"),
"expected normalized src path with line: {formatted}",
);
}
#[test]
fn format_path_hits_file_with_line_branch() {
let mut fmt = Formatter::new();
// Choose a width such that `file_with_line.len() + 2 <= max_width`
// so that branch is exercised without needing truncation.
let file_with_line = "src/main.rs:1";
let max_width = file_with_line.len() + 2;
fmt.path_width = max_width as u16;
let formatted = fmt.format_path("src/main.rs", 1);
// We expect the full file-with-line to appear without truncation markers
assert!(
formatted.contains(file_with_line),
"expected full file_with_line in output: {formatted}",
);
assert!(
!formatted.contains('…') && !formatted.starts_with("..."),
"expected no ellipsis for file_with_line branch: {formatted}",
);
assert!(
formatted.len() <= fmt.path_width as usize,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
#[test]
fn format_path_leading_ellipsis_for_very_long_paths() {
let mut fmt = Formatter::new();
// Very narrow width to ensure we hit the final leading-ellipsis truncation branch
fmt.path_width = 16;
let formatted = fmt.format_path(
"/very/long/prefix/that/is/not/src/and/should/be/trimmed/file.rs",
99,
);
// Final branch should produce a leading ellipsis and keep just the tail
assert!(
formatted.starts_with('…') || formatted.starts_with("..."),
"expected leading ellipsis for aggressively truncated path: {formatted}",
);
assert!(
formatted.contains("file.rs:99"),
"expected file name with line to be preserved in final-branch truncation: {formatted}",
);
assert!(
formatted.len() <= fmt.path_width,
"formatted length {} should not exceed path_width {}",
formatted.len(),
fmt.path_width
);
}
```
These tests assume:
1. `Formatter::format_path(&self, path: &str, line: u32) -> String` exists and is accessible from this module.
2. `Formatter::path_width` is a public or otherwise settable numeric field (here treated as `u16`; adjust the cast or type if it differs).
3. The implementation normalizes `\` to `/`, makes absolute `src/`-containing paths relative to `src/`, and uses either the Unicode ellipsis `…` or a `"..."` prefix for truncation.
If the actual behavior differs (e.g., different ellipsis marker, no right-alignment, different relativization rule), update the assertions to match the concrete formatting rules while keeping the test structure (short/no truncation, long `src/` + truncation, Windows normalization, `file_with_line.len() + 2 <= max_width` branch, and final leading-ellipsis branch) the same.
</issue_to_address>
### Comment 4
<location path="src/test.rs" line_range="18-27" />
<code_context>
+fn build_test_guard(level: Level, style: Style) -> (TracingGuard, TestSubscriber) {
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests that exercise the new `TracingGuard::set_filter` and level/target APIs with raw directives.
The existing tests cover `with_style`, `set_target_level`, and `remove_target_level`, but the new `TracingGuard::set_filter` API and tighter `Level`/`Filter` semantics are untested.
Please add tests that, for example:
- Build a guard via `build_test_guard(Level::Info, ...)`, call `set_filter(Filter::from_directive("info,my_crate=debug"))`, and assert `guard.filter.as_directive()` matches and the call returns `Ok(())`.
- Call `set_level(Level::Warn)` and assert `guard.filter.as_directive()` becomes `"warn"` without error.
- Call `set_target_level("demo", Level::Trace)` after a `set_filter` based on a raw directive, asserting per-target overrides are correctly reflected and succeed.
These will help lock in the new public API behavior without depending on `EnvFilter` internals.
Suggested implementation:
```rust
fn build_test_guard(level: Level, style: Style) -> (TracingGuard, TestSubscriber) {
let filter = Filter::new(level);
let env_filter =
tracing_subscriber::EnvFilter::try_new(filter.as_directive()).unwrap_or_default();
let (env_layer, raw): (_, ReloadHandle) = tracing_subscriber::reload::Layer::new(env_filter);
let layers: Vec<BoxedLayer> = vec![
tracing_subscriber::fmt::Layer::default()
.with_writer(io::sink)
.boxed(),
];
// NOTE: The rest of `build_test_guard` is assumed to remain unchanged.
}
#[cfg(test)]
mod tracing_guard_filter_tests {
use super::*;
#[test]
fn set_filter_with_raw_directive_updates_guard() {
// Start from an initial guard with a simple level-based filter.
let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default());
// Apply a raw directive that includes a per-target override.
let filter = Filter::from_directive("info,my_crate=debug");
guard
.set_filter(filter)
.expect("set_filter with raw directive should succeed");
// The guard's filter should now match the directive we provided.
assert_eq!(
guard.filter.as_directive(),
"info,my_crate=debug",
"guard.filter.as_directive() should reflect the raw directive applied via set_filter"
);
}
#[test]
fn set_level_after_filter_replaces_with_simple_level_directive() {
let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default());
// First, set a more complex filter.
guard
.set_filter(Filter::from_directive("info,my_crate=debug"))
.expect("set_filter should succeed");
// Now replace it with a simple level using the level API.
guard
.set_level(Level::Warn)
.expect("set_level(Level::Warn) should succeed");
// After calling set_level, the directive should be a simple level directive.
assert_eq!(
guard.filter.as_directive(),
"warn",
"set_level should replace the filter with a simple level directive"
);
}
#[test]
fn set_target_level_after_raw_directive_adds_per_target_override() {
let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default());
guard
.set_filter(Filter::from_directive("info,my_crate=debug"))
.expect("initial set_filter should succeed");
// Add a per-target override after the raw directive has been applied.
guard
.set_target_level("demo", Level::Trace)
.expect("set_target_level should succeed after set_filter");
let directive = guard.filter.as_directive();
// The overall directive should still contain the base level and original override.
assert!(
directive.contains("info"),
"directive should still include base level after set_target_level: {directive}"
);
assert!(
directive.contains("my_crate=debug"),
"directive should retain existing per-target override after set_target_level: {directive}"
);
assert!(
directive.contains("demo=trace"),
"directive should include the new per-target override added via set_target_level: {directive}"
);
}
```
The replacement above assumes:
1. The snippet you shared is the full body of `build_test_guard`. If there is additional code in that function, reinsert it between the `let layers: Vec<BoxedLayer> = vec![ ... ];` line and the closing `}` so the helper continues to build and return a valid `(TracingGuard, TestSubscriber)`.
2. `Style` implements `Default`. If it does not, replace `Style::default()` in the tests with whatever style constructor you use elsewhere in your test suite (for example, `Style::Pretty` or a crate-specific default).
3. `Level` uses `CamelCase` variants (`Level::Info`, `Level::Warn`, `Level::Trace`) as implied by your review comment. If instead you re-export `tracing::Level`, adjust the variants to `Level::INFO`, `Level::WARN`, and `Level::TRACE`.
4. `Filter::from_directive(&str)` and `guard.filter` are available as shown in your comment. If `filter` is not public, replace direct field access with the appropriate accessor (for example, `guard.filter().as_directive()`).
</issue_to_address>
### Comment 5
<location path="src/color/style.rs" line_range="10" />
<code_context>
- let r = rgb.0;
- let g = rgb.1;
- let b = rgb.2;
+#[derive(Clone, Copy, Debug)]
+pub struct Styled {
+ rgb: Rgb,
</code_context>
<issue_to_address>
**issue (complexity):** Consider reintroducing simple public helper functions over `Styled` and hoisting the ANSI16 table into a constant to keep the API intention‑revealing and the implementation leaner.
You can keep the new shared conversion logic while restoring the simpler, intention‑revealing API and reducing some internal complexity.
### 1. Reintroduce small helper functions on top of `Styled`
Expose the same shape of helpers as before and keep `Styled` + `From<Styled>` as an internal detail. This keeps call sites simple and avoids leaking the `Styled` abstraction:
```rust
pub fn rgb_to_owo(rgb: (u8, u8, u8), depth: ColorDepth) -> OwoStyle {
Styled::new(Rgb(rgb.0, rgb.1, rgb.2), depth).into()
}
pub fn rgb_to_owo_on(r: u8, g: u8, b: u8, depth: ColorDepth) -> OwoStyle {
Styled::new(Rgb(r, g, b), depth).on().into()
}
/// Dims the foreground by dividing brightness by 4 to ensure high text legibility.
pub fn theme_fg_dimmed(rgb: (u8, u8, u8), depth: ColorDepth) -> OwoStyle {
Styled::new(Rgb(rgb.0, rgb.1, rgb.2), depth).dimmed().into()
}
```
Call sites then go back to:
```rust
let style = rgb_to_owo(theme.text, self.color_depth);
// or
let style = rgb_to_owo_on(bg.0, bg.1, bg.2, self.color_depth);
// or
let style = theme_fg_dimmed(theme.text, self.color_depth);
```
while still benefiting from the updated mapping logic inside `From<Styled>`.
If `Styled` is not meant to be public, you can also make it `pub(crate)` and keep these helpers as the public API.
### 2. Avoid rebuilding the ANSI16 color table on every call
The `Ansi16` arm currently allocates a 16‑element array on every conversion. You can reduce both complexity and overhead by hoisting it to a constant:
```rust
const ANSI16_TABLE: [AnsiColors; 16] = [
AnsiColors::Black,
AnsiColors::Red,
AnsiColors::Green,
AnsiColors::Yellow,
AnsiColors::Blue,
AnsiColors::Magenta,
AnsiColors::Cyan,
AnsiColors::White,
AnsiColors::BrightBlack,
AnsiColors::BrightRed,
AnsiColors::BrightGreen,
AnsiColors::BrightYellow,
AnsiColors::BrightBlue,
AnsiColors::BrightMagenta,
AnsiColors::BrightCyan,
AnsiColors::BrightWhite,
];
```
and then:
```rust
ColorDepth::Ansi16 => {
let idx = rgb_to_ansi(s.as_tuple().into(), Palette::default()) as usize;
let ansi = ANSI16_TABLE
.get(idx)
.unwrap_or(&AnsiColors::White);
if s.on {
return style.on_color(ansi);
}
style.color(ansi)
}
```
These changes preserve all new behavior (dimming, background, updated mapping) while making the public usage simpler and the internal implementation slightly leaner.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Owner
Author
|
/gemini review |
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Code Review
This pull request refactors the library's configuration and initialization API, replacing the Level field with a more robust Filter system and simplifying the subscriber layer management. It also reduces external dependencies and refactors the path formatting logic. Review feedback identifies a bug in how source paths are stripped when multiple 'src/' directories exist and points out an alignment issue in the path formatter that breaks log column consistency. A cleanup of redundant variables in the initialization logic was also suggested.
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并且有一些整体性的反馈：
- 新的
Formatter::format_path逻辑依赖通过在字符串中按"src/"/"src\\"分割并手动切片，这在非标准目录结构或存在多个src片段时会比较脆弱；建议改用基于Path/Path::strip_prefix的规范化方式，这样前缀剥离和分隔符规范化会更不易出错、也更具可移植性。 - 在
acta-build::walk_src_max_width中，目前对strip_prefix的处理是对字符串化后的路径使用find(strip_prefix)；如果该前缀出现多次（或分隔符混用），可能会导致路径被错误地剥离——在可能的情况下使用Path::strip_prefix，仅在必要时回退到字符串操作，可以让计算出来的宽度在不同平台上更加准确。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The new `Formatter::format_path` logic relies on string splitting on `"src/"` / `"src\\"` and manual slicing, which can be brittle for non-standard layouts or multiple `src` segments; consider switching to `Path` / `Path::strip_prefix`-based normalization so prefix-stripping and separator normalization are less error-prone and more portable.
- In `acta-build::walk_src_max_width`, the `strip_prefix` handling currently uses `find(strip_prefix)` on a stringified path; if the prefix appears more than once (or with mixed separators), this may mis-strip paths—using `Path::strip_prefix` where possible and only falling back to string operations would make the computed width more accurate across platforms.
## Individual Comments
### Comment 1
<location path="src/fmt/test.rs" line_range="145-152" />
<code_context>
+}
+
+#[test]
+fn format_path_leading_ellipsis_for_very_long_path() {
+ let mut fmt = Formatter::new();
+ // Narrow enough that file_with_line alone exceeds width,
+ // forcing the leading-ellipsis fallback branch.
+ fmt.path_width = 11;
+ let result = fmt.format_path("/very/long/prefix/deep/nested/dirs/file.rs", 99);
+ assert!(result.starts_with('\u{2026}'), "expected leading ellipsis: {result}");
+ assert!(result.contains("file.rs:99"), "expected filename preserved: {result}");
+}
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test covering multi-byte (Unicode) paths to exercise the char-boundary-safe truncation logic in `format_path`
Given `format_path` now enforces UTF-8 char boundaries, please add a test with a multi-byte path segment (e.g. `"src/模块/文件.rs"`) and a narrow `path_width` that forces truncation. The test should verify the output remains valid UTF-8 (no split characters around the ellipsis) and still preserves the `file.rs:line` suffix. This helps catch off-by-one errors in the boundary-adjustment loop.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈来改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The new
Formatter::format_pathlogic relies on string splitting on"src/"/"src\\"and manual slicing, which can be brittle for non-standard layouts or multiplesrcsegments; consider switching toPath/Path::strip_prefix-based normalization so prefix-stripping and separator normalization are less error-prone and more portable. - In
acta-build::walk_src_max_width, thestrip_prefixhandling currently usesfind(strip_prefix)on a stringified path; if the prefix appears more than once (or with mixed separators), this may mis-strip paths—usingPath::strip_prefixwhere possible and only falling back to string operations would make the computed width more accurate across platforms.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `Formatter::format_path` logic relies on string splitting on `"src/"` / `"src\\"` and manual slicing, which can be brittle for non-standard layouts or multiple `src` segments; consider switching to `Path` / `Path::strip_prefix`-based normalization so prefix-stripping and separator normalization are less error-prone and more portable.
- In `acta-build::walk_src_max_width`, the `strip_prefix` handling currently uses `find(strip_prefix)` on a stringified path; if the prefix appears more than once (or with mixed separators), this may mis-strip paths—using `Path::strip_prefix` where possible and only falling back to string operations would make the computed width more accurate across platforms.
## Individual Comments
### Comment 1
<location path="src/fmt/test.rs" line_range="145-152" />
<code_context>
+}
+
+#[test]
+fn format_path_leading_ellipsis_for_very_long_path() {
+ let mut fmt = Formatter::new();
+ // Narrow enough that file_with_line alone exceeds width,
+ // forcing the leading-ellipsis fallback branch.
+ fmt.path_width = 11;
+ let result = fmt.format_path("/very/long/prefix/deep/nested/dirs/file.rs", 99);
+ assert!(result.starts_with('\u{2026}'), "expected leading ellipsis: {result}");
+ assert!(result.contains("file.rs:99"), "expected filename preserved: {result}");
+}
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test covering multi-byte (Unicode) paths to exercise the char-boundary-safe truncation logic in `format_path`
Given `format_path` now enforces UTF-8 char boundaries, please add a test with a multi-byte path segment (e.g. `"src/模块/文件.rs"`) and a narrow `path_width` that forces truncation. The test should verify the output remains valid UTF-8 (no split characters around the ellipsis) and still preserves the `file.rs:line` suffix. This helps catch off-by-one errors in the boundary-adjustment loop.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/builder.rs (1)
362-379:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win失败的过滤器更新会把
TracingGuard留在未提交状态。Line 367 先修改了
self.filter，Line 377-378 才去构造并 reloadEnvFilter。如果target生成了无效 directive，这个方法虽然返回Err，但self.filter已经变成新值了，之后的Debug、remove_target_level和后续更新都会基于一个并未真正生效的过滤器状态。先在临时Filter上验证成功后再回写会更稳妥。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/builder.rs` around lines 362 - 379, The current flow mutates self.filter (in set_target_level and remove_target_level) before attempting to construct and reload an EnvFilter in apply_current_filter, leaving self.filter changed even if EnvFilter::try_new fails; instead, perform the mutation on a temporary Filter copy, validate by calling tracing_subscriber::EnvFilter::try_new(temp.as_directive()) (or call apply_current_filter-like validation) and only when that succeeds assign the validated temp back to self.filter and then update the raw guard (i.e., change set_target_level and remove_target_level to operate on a cloned filter, validate the directive, then commit the clone to self.filter and call the existing reload logic).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.rs`:
- Around line 7-9: The write failure to the build artifact is currently only
logged as a warning; change the branch handling the std::fs::write(&path,
width.to_string()) Err(e) case to fail the build immediately (e.g., panic! or
call std::process::exit with non-zero) so the build stops on write errors;
update the error handling around std::fs::write (referencing path, width, and
the Err(e) match arm) to include the error details in the failure message so the
build aborts with a clear diagnostic.
In `@crates/acta-build/src/lib.rs`:
- Around line 60-61: The current code uses display.find(strip_prefix) which
matches the first occurrence anywhere instead of only at the start; replace this
with prefix semantics by using display.strip_prefix(strip_prefix) (or
starts_with + slicing) so you only strip when strip_prefix is at the beginning.
Concretely, change the .find(strip_prefix).map_or(display.len(), |i| display[i +
strip_prefix.len()..].len()) logic to use
display.strip_prefix(strip_prefix).map_or(display.len(), |s| s.len()) so the
computed length is based on a true prefix removal of strip_prefix.
In `@README.md`:
- Around line 112-115: Add a single blank line between the table ending with
"`Format::Json(LayerConfig)`" and the following "## File logging" heading so the
table and heading are separated (this prevents MD058/MD022 warnings); simply
insert one empty line after the table row for Format::Json.
In `@src/builder.rs`:
- Around line 141-143: build_layer currently lets WriterTarget::File be routed
to std::io::sink() via build_layer_with while remaining pub, causing callers who
use build_layer directly to silently drop file logs; change the API so callers
can't silently lose logs — either make build_layer return Result<BoxedLayer,
Error> and propagate an error when WriterTarget::File would use sink, or split
file-target construction into a separate non-public helper and keep build_layer
public only for safe targets; update build_layer, build_layer_with, and any code
that constructs WriterTarget::File to use the new error-returning or separate
constructor and ensure no path returns a BoxedLayer that wraps std::io::sink for
file targets.
- Around line 270-272: Current code creates a single shared ArcSwap
(shared_style) and passes it into build_formatter for multiple non-file writers,
so each writer's call to with_style_config(writer.style) overwrites the same
ArcSwap and later writers' styles win. Fix by giving each non-file writer its
own ArcSwap initialized from that writer's style instead of reusing
shared_style: inside the loop that constructs writers (the code that checks for
file vs non-file writers and calls build_formatter(..., shared_handle)), create
a new Arc::new(arc_swap::ArcSwap::new(Arc::new(writer.style))) per writer and
pass that per-writer ArcSwap into build_formatter; remove or stop using the
single shared_style for non-file writers so styles don't clobber each other.
- Around line 277-279: The current implementation in init stores only a single
file_guard and log_path (variables file_guard and log_path) and overwrites them
each time a WriterTarget::File is encountered, losing previous guards and paths;
either reject multiple WriterTarget::File entries in init (validate targets and
return an error if more than one file target is present) or change file_guard
and log_path to collections (e.g., Vec or HashMap) and push/store a guard/path
for each WriterTarget::File encountered and adjust any uses of log_path() and
the guard drop semantics accordingly so all file writers are retained; update
code paths that assume a single file (references around handling of file_guard,
log_path, and log_path()) to iterate over the collection.
In `@src/color/style.rs`:
- Around line 76-78: The code casts the anstyle::AnsiColor returned by
rgb_to_ansi to usize and indexes ANSI16_TABLE (ColorDepth::Ansi16 branch), which
relies on the enum's discriminant values and can break if upstream changes;
replace that fragile indexing with an explicit exhaustive mapping from each
anstyle::AnsiColor variant to the corresponding table entry (or to the desired
AnsiColors value) — locate where rgb_to_ansi(...) is used and instead of "as
usize" + ANSI16_TABLE.get(...).copied().unwrap_or(AnsiColors::White) implement a
match on the rgb_to_ansi(...) result (covering all AnsiColor variants) that
returns the correct entry and remove the silent White fallback so behavior is
explicit.
In `@src/test.rs`:
- Around line 137-189: Update the tests to assert actual filtering behavior via
the returned subscriber instead of only inspecting guard.filter; for example, in
set_filter_with_raw_directive_updates_guard (and/or
set_target_level_after_raw_directive_adds_per_target_override) use the
subscriber returned from build_test_guard to construct Metadata for different
targets/levels and call the subscriber's enabled check to assert that the base
level ("info") blocks lower levels and that the per-target override
("my_crate=debug" and later "demo=trace") allows the expected levels; ensure you
call build_test_guard(...) to get the subscriber and add assertions that the
subscriber.enabled(...) (or the equivalent API) returns true/false for the
specific target+level combinations that prove the filter is actually enforced at
runtime.
In `@src/writer/test.rs`:
- Around line 1-5: Remove the module-level allow for clippy::unnecessary_map_or
and rewrite the specific Option/Iterator map_or usages in this file to avoid
suppressing future warnings: search for occurrences of Option::map_or or
.map_or(...) in tests within src/writer/test.rs and replace them with more
explicit checks (e.g., use is_some_and for boolean predicates or match/if let to
handle values) so the linter rule can remain enabled; keep the other clippy
allows intact and only remove the clippy::unnecessary_map_or attribute.
---
Outside diff comments:
In `@src/builder.rs`:
- Around line 362-379: The current flow mutates self.filter (in set_target_level
and remove_target_level) before attempting to construct and reload an EnvFilter
in apply_current_filter, leaving self.filter changed even if EnvFilter::try_new
fails; instead, perform the mutation on a temporary Filter copy, validate by
calling tracing_subscriber::EnvFilter::try_new(temp.as_directive()) (or call
apply_current_filter-like validation) and only when that succeeds assign the
validated temp back to self.filter and then update the raw guard (i.e., change
set_target_level and remove_target_level to operate on a cloned filter, validate
the directive, then commit the clone to self.filter and call the existing reload
logic).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ea57346-f0d9-4895-93a1-203e971ae813
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.gitignoreCargo.tomlREADME.mdbuild.rscrates/acta-build/Cargo.tomlcrates/acta-build/src/lib.rscrates/acta-debug/src/main.rssrc/builder.rssrc/color/mapping.rssrc/color/mod.rssrc/color/style.rssrc/color/tests.rssrc/config/depth.rssrc/config/mod.rssrc/config/test.rssrc/fmt/mod.rssrc/fmt/test.rssrc/lib.rssrc/prelude.rssrc/test.rssrc/writer/custom.rssrc/writer/mod.rssrc/writer/test.rs
💤 Files with no reviewable changes (3)
- src/color/mapping.rs
- Cargo.toml
- src/config/depth.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build / Build
🧰 Additional context used
🪛 markdownlint-cli2 (0.22.1)
README.md
[warning] 114-114: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 115-115: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
🔇 Additional comments (7)
src/writer/custom.rs (1)
24-27: LGTM!Also applies to: 60-60
src/writer/mod.rs (1)
13-13: LGTM!src/lib.rs (1)
14-18: LGTM!Also applies to: 26-26
src/prelude.rs (1)
1-1: LGTM!Also applies to: 4-7, 12-12
crates/acta-debug/src/main.rs (1)
5-7: 本文件改动主要是演示代码与新 API 对齐，当前未发现需要在此处新增的问题评论。Also applies to: 43-43, 270-276, 291-291, 320-322, 343-362
src/config/mod.rs (2)
570-575: 🏗️ Heavy liftRequest missing inputs to rewrite review comment: No
<review_comment>content (and no verification results) was provided, so I can’t rewrite it. Paste the original review comment and any relevant command outputs/logs you want used.
425-428: ⚡ Quick winMissing review comment context: Paste the original <review_comment>...</review_comment> (and any related diff/snippet it refers to) so I can rewrite it accurately.
Comment on lines
60
to
+61
| .find(strip_prefix) | ||
| .map(|i| &display[i + strip_prefix.len()..]) | ||
| .unwrap_or(&display) | ||
| .len() | ||
| .map_or(display.len(), |i| display[i + strip_prefix.len()..].len()) |
There was a problem hiding this comment.
strip_prefix 计算应使用“前缀语义”，不要用 find。
Line 60 目前会匹配到路径中首次出现的位置，而不是开头前缀；当上层目录也包含同名片段时会错误截断并高估/低估宽度。建议改为仅在字符串开头剥离。
建议修改
- display
- .find(strip_prefix)
- .map_or(display.len(), |i| display[i + strip_prefix.len()..].len())
+ let normalized_prefix = strip_prefix.replace('\\', "/");
+ display
+ .strip_prefix(&normalized_prefix)
+ .unwrap_or(&display)
+ .len()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/acta-build/src/lib.rs` around lines 60 - 61, The current code uses
display.find(strip_prefix) which matches the first occurrence anywhere instead
of only at the start; replace this with prefix semantics by using
display.strip_prefix(strip_prefix) (or starts_with + slicing) so you only strip
when strip_prefix is at the beginning. Concretely, change the
.find(strip_prefix).map_or(display.len(), |i| display[i +
strip_prefix.len()..].len()) logic to use
display.strip_prefix(strip_prefix).map_or(display.len(), |s| s.len()) so the
computed length is based on a true prefix removal of strip_prefix.
Comment on lines
+112
to
115
| | `Format::Compact(LayerConfig)` | Default themed formatter with optional path and span display. | | ||
| | `Format::Pretty(LayerConfig)` | `tracing-subscriber` pretty formatter with file and line metadata. | | ||
| | `Format::Json(LayerConfig)` | Flattened JSON events without ANSI colors. | | ||
| ## File logging |
There was a problem hiding this comment.
补齐表格与标题之间的空行，避免 markdownlint 报警。
这里紧邻的表格和 ## File logging 标题缺少空行，当前会触发 MD058/MD022，可能影响文档检查流水线。
建议修复
| `Format::Compact(LayerConfig)` | Default themed formatter with optional path and span display. |
| `Format::Pretty(LayerConfig)` | `tracing-subscriber` pretty formatter with file and line metadata. |
| `Format::Json(LayerConfig)` | Flattened JSON events without ANSI colors. |
+
## File logging🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 114-114: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 115-115: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 112 - 115, Add a single blank line between the table
ending with "`Format::Json(LayerConfig)`" and the following "## File logging"
heading so the table and heading are separated (this prevents MD058/MD022
warnings); simply insert one empty line after the table row for Format::Json.
Comment on lines
+141
to
+143
| pub fn build_layer(writer: &Writer) -> BoxedLayer { | ||
| build_layer_with(writer, None) | ||
| } |
There was a problem hiding this comment.
不要让 build_layer 在文件目标上静默丢日志。
Line 191 把 WriterTarget::File 路由到了 std::io::sink()，但 build_layer 仍然是 pub。外部一旦直接用这个 API 构建文件 layer，会得到“初始化成功但永远不落盘”的结果。这里至少应返回错误，或把文件目标拆到单独的公开 API。
Also applies to: 167-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/builder.rs` around lines 141 - 143, build_layer currently lets
WriterTarget::File be routed to std::io::sink() via build_layer_with while
remaining pub, causing callers who use build_layer directly to silently drop
file logs; change the API so callers can't silently lose logs — either make
build_layer return Result<BoxedLayer, Error> and propagate an error when
WriterTarget::File would use sink, or split file-target construction into a
separate non-public helper and keep build_layer public only for safe targets;
update build_layer, build_layer_with, and any code that constructs
WriterTarget::File to use the new error-returning or separate constructor and
ensure no path returns a BoxedLayer that wraps std::io::sink for file targets.
Comment on lines
+270
to
+272
| let shared_style = Arc::new(arc_swap::ArcSwap::new(Arc::new( | ||
| writers.first().map_or_else(Style::default, |w| w.style), | ||
| ))); |
There was a problem hiding this comment.
共享的 style handle 会让后面的 writer 覆盖前面的样式。
Line 270 这里只创建了一个 shared_style，随后在 Line 287 和 Line 298 传给所有非文件 writer。build_formatter(..., shared_handle) 里的 with_style_config(writer.style) 会写回同一个 ArcSwap，所以多个 compact writer 一旦配置了不同的 theme/icons/labels，最终都会按最后一个 writer 的样式输出。
Also applies to: 287-298
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/builder.rs` around lines 270 - 272, Current code creates a single shared
ArcSwap (shared_style) and passes it into build_formatter for multiple non-file
writers, so each writer's call to with_style_config(writer.style) overwrites the
same ArcSwap and later writers' styles win. Fix by giving each non-file writer
its own ArcSwap initialized from that writer's style instead of reusing
shared_style: inside the loop that constructs writers (the code that checks for
file vs non-file writers and calls build_formatter(..., shared_handle)), create
a new Arc::new(arc_swap::ArcSwap::new(Arc::new(writer.style))) per writer and
pass that per-writer ArcSwap into build_formatter; remove or stop using the
single shared_style for non-file writers so styles don't clobber each other.
Comment on lines
277
to
279
| let mut file_guard = None; | ||
| #[cfg(feature = "file")] | ||
| let mut log_path = None; |
There was a problem hiding this comment.
多个文件 writer 这里只保留了最后一个 guard/path。
Line 277-279 只有一个 file_guard 和 log_path，而 Line 292-293 每遇到一个 WriterTarget::File 就覆盖一次。这样多文件输出时前一个 guard 会被立刻替换掉，log_path() 也只剩最后一个路径。若不打算支持多个文件 target，最好在 init 里显式拒绝；否则这里需要改成集合。
Also applies to: 289-295, 313-316
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/builder.rs` around lines 277 - 279, The current implementation in init
stores only a single file_guard and log_path (variables file_guard and log_path)
and overwrites them each time a WriterTarget::File is encountered, losing
previous guards and paths; either reject multiple WriterTarget::File entries in
init (validate targets and return an error if more than one file target is
present) or change file_guard and log_path to collections (e.g., Vec or HashMap)
and push/store a guard/path for each WriterTarget::File encountered and adjust
any uses of log_path() and the guard drop semantics accordingly so all file
writers are retained; update code paths that assume a single file (references
around handling of file_guard, log_path, and log_path()) to iterate over the
collection.
Comment on lines
+76
to
+78
| ColorDepth::Ansi16 => { | ||
| let idx = rgb_to_ansi(s.as_tuple().into(), Palette::default()) as usize; | ||
| let ansi = ANSI16_TABLE.get(idx).copied().unwrap_or(AnsiColors::White); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
不要把 anstyle::AnsiColor 的判别值当成稳定索引。
rgb_to_ansi 返回的是 4-bit anstyle::AnsiColor，这里再用 as usize 去索引本地表，相当于把正确性绑定到了上游枚举的判别值顺序；一旦上游显式指定或调整判别值，就会悄悄落到 White 回退。这里直接对 AnsiColor 做穷举映射会更稳，也能去掉这个静默兜底。 (docs.rs)
♻️ 建议改法
- let idx = rgb_to_ansi(s.as_tuple().into(), Palette::default()) as usize;
- let ansi = ANSI16_TABLE.get(idx).copied().unwrap_or(AnsiColors::White);
+ let ansi = match rgb_to_ansi(s.as_tuple().into(), Palette::default()) {
+ anstyle::AnsiColor::Black => AnsiColors::Black,
+ anstyle::AnsiColor::Red => AnsiColors::Red,
+ anstyle::AnsiColor::Green => AnsiColors::Green,
+ anstyle::AnsiColor::Yellow => AnsiColors::Yellow,
+ anstyle::AnsiColor::Blue => AnsiColors::Blue,
+ anstyle::AnsiColor::Magenta => AnsiColors::Magenta,
+ anstyle::AnsiColor::Cyan => AnsiColors::Cyan,
+ anstyle::AnsiColor::White => AnsiColors::White,
+ anstyle::AnsiColor::BrightBlack => AnsiColors::BrightBlack,
+ anstyle::AnsiColor::BrightRed => AnsiColors::BrightRed,
+ anstyle::AnsiColor::BrightGreen => AnsiColors::BrightGreen,
+ anstyle::AnsiColor::BrightYellow => AnsiColors::BrightYellow,
+ anstyle::AnsiColor::BrightBlue => AnsiColors::BrightBlue,
+ anstyle::AnsiColor::BrightMagenta => AnsiColors::BrightMagenta,
+ anstyle::AnsiColor::BrightCyan => AnsiColors::BrightCyan,
+ anstyle::AnsiColor::BrightWhite => AnsiColors::BrightWhite,
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ColorDepth::Ansi16 => { | |
| let idx = rgb_to_ansi(s.as_tuple().into(), Palette::default()) as usize; | |
| let ansi = ANSI16_TABLE.get(idx).copied().unwrap_or(AnsiColors::White); | |
| ColorDepth::Ansi16 => { | |
| let ansi = match rgb_to_ansi(s.as_tuple().into(), Palette::default()) { | |
| anstyle::AnsiColor::Black => AnsiColors::Black, | |
| anstyle::AnsiColor::Red => AnsiColors::Red, | |
| anstyle::AnsiColor::Green => AnsiColors::Green, | |
| anstyle::AnsiColor::Yellow => AnsiColors::Yellow, | |
| anstyle::AnsiColor::Blue => AnsiColors::Blue, | |
| anstyle::AnsiColor::Magenta => AnsiColors::Magenta, | |
| anstyle::AnsiColor::Cyan => AnsiColors::Cyan, | |
| anstyle::AnsiColor::White => AnsiColors::White, | |
| anstyle::AnsiColor::BrightBlack => AnsiColors::BrightBlack, | |
| anstyle::AnsiColor::BrightRed => AnsiColors::BrightRed, | |
| anstyle::AnsiColor::BrightGreen => AnsiColors::BrightGreen, | |
| anstyle::AnsiColor::BrightYellow => AnsiColors::BrightYellow, | |
| anstyle::AnsiColor::BrightBlue => AnsiColors::BrightBlue, | |
| anstyle::AnsiColor::BrightMagenta => AnsiColors::BrightMagenta, | |
| anstyle::AnsiColor::BrightCyan => AnsiColors::BrightCyan, | |
| anstyle::AnsiColor::BrightWhite => AnsiColors::BrightWhite, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/color/style.rs` around lines 76 - 78, The code casts the
anstyle::AnsiColor returned by rgb_to_ansi to usize and indexes ANSI16_TABLE
(ColorDepth::Ansi16 branch), which relies on the enum's discriminant values and
can break if upstream changes; replace that fragile indexing with an explicit
exhaustive mapping from each anstyle::AnsiColor variant to the corresponding
table entry (or to the desired AnsiColors value) — locate where rgb_to_ansi(...)
is used and instead of "as usize" +
ANSI16_TABLE.get(...).copied().unwrap_or(AnsiColors::White) implement a match on
the rgb_to_ansi(...) result (covering all AnsiColor variants) that returns the
correct entry and remove the silent White fallback so behavior is explicit.
Comment on lines
+137
to
+189
| #[test] | ||
| fn set_filter_with_raw_directive_updates_guard() { | ||
| let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default()); | ||
| let filter = Filter::from_directive("info,my_crate=debug"); | ||
| guard | ||
| .set_filter(filter) | ||
| .expect("set_filter with raw directive should succeed"); | ||
| assert_eq!( | ||
| guard.filter.as_directive(), | ||
| "info,my_crate=debug", | ||
| "guard.filter should reflect the raw directive applied via set_filter" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn set_level_after_filter_replaces_with_simple_level_directive() { | ||
| let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default()); | ||
| guard | ||
| .set_filter(Filter::from_directive("info,my_crate=debug")) | ||
| .expect("set_filter should succeed"); | ||
| guard | ||
| .set_level(Level::Warn) | ||
| .expect("set_level(Level::Warn) should succeed"); | ||
| assert_eq!( | ||
| guard.filter.as_directive(), | ||
| "warn", | ||
| "set_level should replace the filter with a simple level directive" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn set_target_level_after_raw_directive_adds_per_target_override() { | ||
| let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default()); | ||
| guard | ||
| .set_filter(Filter::from_directive("info,my_crate=debug")) | ||
| .expect("initial set_filter should succeed"); | ||
| guard | ||
| .set_target_level("demo", Level::Trace) | ||
| .expect("set_target_level should succeed after set_filter"); | ||
| let directive = guard.filter.as_directive(); | ||
| assert!( | ||
| directive.contains("info"), | ||
| "directive should still include base level after set_target_level: {directive}" | ||
| ); | ||
| assert!( | ||
| directive.contains("my_crate=debug"), | ||
| "directive should retain existing per-target override after set_target_level: {directive}" | ||
| ); | ||
| assert!( | ||
| directive.contains("demo=trace"), | ||
| "directive should include the new per-target override: {directive}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
过滤器测试仅校验内部字段，建议补一条“实际过滤生效”的行为断言。
这几条新增测试都只检查 guard.filter 字符串，若 reload 管道失效但字段仍更新，测试仍会通过。建议至少在一条用例里通过 returned subscriber 包裹执行并断言不同 target/level 的启用状态。
示例（可加到 set_filter 用例）
#[test]
fn set_filter_with_raw_directive_updates_guard() {
- let (mut guard, _subscriber) = build_test_guard(Level::Info, Style::default());
+ let (mut guard, subscriber) = build_test_guard(Level::Info, Style::default());
let filter = Filter::from_directive("info,my_crate=debug");
- guard
- .set_filter(filter)
- .expect("set_filter with raw directive should succeed");
+ tracing::subscriber::with_default(subscriber, || {
+ guard
+ .set_filter(filter)
+ .expect("set_filter with raw directive should succeed");
+ assert!(tracing::enabled!(target: "my_crate", tracing::Level::DEBUG));
+ assert!(!tracing::enabled!(target: "other_crate", tracing::Level::DEBUG));
+ });
assert_eq!(
guard.filter.as_directive(),
"info,my_crate=debug",
"guard.filter should reflect the raw directive applied via set_filter"
);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test.rs` around lines 137 - 189, Update the tests to assert actual
filtering behavior via the returned subscriber instead of only inspecting
guard.filter; for example, in set_filter_with_raw_directive_updates_guard
(and/or set_target_level_after_raw_directive_adds_per_target_override) use the
subscriber returned from build_test_guard to construct Metadata for different
targets/levels and call the subscriber's enabled check to assert that the base
level ("info") blocks lower levels and that the per-target override
("my_crate=debug" and later "demo=trace") allows the expected levels; ensure you
call build_test_guard(...) to get the subscriber and add assertions that the
subscriber.enabled(...) (or the equivalent API) returns true/false for the
specific target+level combinations that prove the filter is actually enforced at
runtime.
Comment on lines
+1
to
+5
| #![allow( | ||
| clippy::unwrap_used, | ||
| clippy::indexing_slicing, | ||
| clippy::unnecessary_map_or | ||
| )] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
收窄 unnecessary_map_or 的模块级放行范围
Line 1-5 现在对整个模块放行 clippy::unnecessary_map_or，会掩盖后续新增测试中的同类问题。建议移除该项并改写对应表达式（如改用 is_some_and）。
♻️ 建议修改
#![allow(
clippy::unwrap_used,
clippy::indexing_slicing,
- clippy::unnecessary_map_or
)]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/writer/test.rs` around lines 1 - 5, Remove the module-level allow for
clippy::unnecessary_map_or and rewrite the specific Option/Iterator map_or
usages in this file to avoid suppressing future warnings: search for occurrences
of Option::map_or or .map_or(...) in tests within src/writer/test.rs and replace
them with more explicit checks (e.g., use is_some_and for boolean predicates or
match/if let to handle values) so the linter rule can remain enabled; keep the
other clippy allows intact and only remove the clippy::unnecessary_map_or
attribute.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
重构 acta 的日志管道与配置，以简化分层结构、改进过滤器处理，并现代化颜色与格式化工具。
New Features:
Filter::from_directiveAPI，并在Config与ConfigBuilder上直接暴露过滤器配置。Layers复合类型，用于将多个tracing-subscriberlayer 作为单一单元进行管理。Bug Fixes:
set_filter、set_level和set_target_level能够与原始指令及按目标覆盖规则保持一致的交互。Enhancements:
Layers包装器替代深度嵌套的 layer 金字塔，简化 subscriber 构造。Styled抽象统一颜色处理，并切换到anstyle-lossy以进行 ANSI16 映射。from_palette辅助函数重构Theme的构造，并清理主题定义。String、增加更多测试并集成新的构建期宽度辅助工具，改进路径格式化。init/TracingGuard流程。Documentation:
acta-build辅助工具撰写文档，并展示如何将构建期路径宽度接入Formatter。Filter::from_directive、结构化级别以及基于LayerConfig的格式。Tests:
Original summary in English
Summary by Sourcery
Refactor acta’s logging pipeline and configuration to simplify layering, improve filter handling, and modernize color and formatting utilities.
New Features:
Filter::from_directiveAPI and expose filter configuration directly onConfigandConfigBuilder.Layerscomposite type to manage multipletracing-subscriberlayers as a single unit.Bug Fixes:
set_filter,set_level, andset_target_levelinteract consistently with raw directives and per-target overrides.Enhancements:
Layerswrapper.Styledabstraction and switch toanstyle-lossyfor ANSI16 mapping.Themeconstruction around afrom_palettehelper and clean up theme definitions.Strings, adding more tests, and integrating the new build-time width helper.init/TracingGuardflow.Documentation:
acta-buildhelper and show how to wire build-time path width intoFormatter.Filter::from_directive, structured levels, andLayerConfig-based formats.Tests:
Original summary in English
Summary by Sourcery
重构 acta 的日志管道与配置，以简化分层结构、改进过滤器处理，并现代化颜色与格式化工具。
New Features:
Filter::from_directiveAPI，并在Config与ConfigBuilder上直接暴露过滤器配置。Layers复合类型，用于将多个tracing-subscriberlayer 作为单一单元进行管理。Bug Fixes:
set_filter、set_level和set_target_level能够与原始指令及按目标覆盖规则保持一致的交互。Enhancements:
Layers包装器替代深度嵌套的 layer 金字塔，简化 subscriber 构造。Styled抽象统一颜色处理，并切换到anstyle-lossy以进行 ANSI16 映射。from_palette辅助函数重构Theme的构造，并清理主题定义。String、增加更多测试并集成新的构建期宽度辅助工具，改进路径格式化。init/TracingGuard流程。Documentation:
acta-build辅助工具撰写文档，并展示如何将构建期路径宽度接入Formatter。Filter::from_directive、结构化级别以及基于LayerConfig的格式。Tests:
Original summary in English
Summary by Sourcery
Refactor acta’s logging pipeline and configuration to simplify layering, improve filter handling, and modernize color and formatting utilities.
New Features:
Filter::from_directiveAPI and expose filter configuration directly onConfigandConfigBuilder.Layerscomposite type to manage multipletracing-subscriberlayers as a single unit.Bug Fixes:
set_filter,set_level, andset_target_levelinteract consistently with raw directives and per-target overrides.Enhancements:
Layerswrapper.Styledabstraction and switch toanstyle-lossyfor ANSI16 mapping.Themeconstruction around afrom_palettehelper and clean up theme definitions.Strings, adding more tests, and integrating the new build-time width helper.init/TracingGuardflow.Documentation:
acta-buildhelper and show how to wire build-time path width intoFormatter.Filter::from_directive, structured levels, andLayerConfig-based formats.Tests:
Original summary in English
Summary by Sourcery
重构 acta 的日志管道与配置，以简化分层结构、改进过滤器处理，并现代化颜色与格式化工具。
New Features:
Filter::from_directiveAPI，并在Config与ConfigBuilder上直接暴露过滤器配置。Layers复合类型，用于将多个tracing-subscriberlayer 作为单一单元进行管理。Bug Fixes:
set_filter、set_level和set_target_level能够与原始指令及按目标覆盖规则保持一致的交互。Enhancements:
Layers包装器替代深度嵌套的 layer 金字塔，简化 subscriber 构造。Styled抽象统一颜色处理，并切换到anstyle-lossy以进行 ANSI16 映射。from_palette辅助函数重构Theme的构造，并清理主题定义。String、增加更多测试并集成新的构建期宽度辅助工具，改进路径格式化。init/TracingGuard流程。Documentation:
acta-build辅助工具撰写文档，并展示如何将构建期路径宽度接入Formatter。Filter::from_directive、结构化级别以及基于LayerConfig的格式。Tests:
Original summary in English
Summary by Sourcery
Refactor acta’s logging pipeline and configuration to simplify layering, improve filter handling, and modernize color and formatting utilities.
New Features:
Filter::from_directiveAPI and expose filter configuration directly onConfigandConfigBuilder.Layerscomposite type to manage multipletracing-subscriberlayers as a single unit.Bug Fixes:
set_filter,set_level, andset_target_levelinteract consistently with raw directives and per-target overrides.Enhancements:
Layerswrapper.Styledabstraction and switch toanstyle-lossyfor ANSI16 mapping.Themeconstruction around afrom_palettehelper and clean up theme definitions.Strings, adding more tests, and integrating the new build-time width helper.init/TracingGuardflow.Documentation:
acta-buildhelper and show how to wire build-time path width intoFormatter.Filter::from_directive, structured levels, andLayerConfig-based formats.Tests:
Original summary in English
Summary by Sourcery
重构 acta 的日志管道与配置，以简化分层结构、改进过滤器处理，并现代化颜色与格式化工具。
New Features:
Filter::from_directiveAPI，并在Config与ConfigBuilder上直接暴露过滤器配置。Layers复合类型，用于将多个tracing-subscriberlayer 作为单一单元进行管理。Bug Fixes:
set_filter、set_level和set_target_level能够与原始指令及按目标覆盖规则保持一致的交互。Enhancements:
Layers包装器替代深度嵌套的 layer 金字塔，简化 subscriber 构造。Styled抽象统一颜色处理，并切换到anstyle-lossy以进行 ANSI16 映射。from_palette辅助函数重构Theme的构造，并清理主题定义。String、增加更多测试并集成新的构建期宽度辅助工具，改进路径格式化。init/TracingGuard流程。Documentation:
acta-build辅助工具撰写文档，并展示如何将构建期路径宽度接入Formatter。Filter::from_directive、结构化级别以及基于LayerConfig的格式。Tests:
Original summary in English
Summary by Sourcery
Refactor acta’s logging pipeline and configuration to simplify layering, improve filter handling, and modernize color and formatting utilities.
New Features:
Filter::from_directiveAPI and expose filter configuration directly onConfigandConfigBuilder.Layerscomposite type to manage multipletracing-subscriberlayers as a single unit.Bug Fixes:
set_filter,set_level, andset_target_levelinteract consistently with raw directives and per-target overrides.Enhancements:
Layerswrapper.Styledabstraction and switch toanstyle-lossyfor ANSI16 mapping.Themeconstruction around afrom_palettehelper and clean up theme definitions.Strings, adding more tests, and integrating the new build-time width helper.init/TracingGuardflow.Documentation:
acta-buildhelper and show how to wire build-time path width intoFormatter.Filter::from_directive, structured levels, andLayerConfig-based formats.Tests: