fix: order ui accent before nested config tables - #2698
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe default configuration now documents ChangesUI accent configuration ordering
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main.rs (1)
927-934: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the regression test reject duplicate accent documentation.
findreturns the first matching block. If the removed later block is reintroduced,accent < sidebarstill passes. Count the accent documentation marker and assert that it occurs exactly once before checking the ordering.Proposed test improvement
#[test] fn default_config_lists_ui_accent_before_nested_tables() { - let accent = DEFAULT_CONFIG.find("# accent = \"cyan\"").unwrap(); - let sidebar = DEFAULT_CONFIG.find("# [ui.sidebar.agents]").unwrap(); + let accent_marker = "# Accent color for highlights, borders, and navigation UI."; + assert_eq!(DEFAULT_CONFIG.matches(accent_marker).count(), 1); + let accent = DEFAULT_CONFIG.find("# accent = \"cyan\"").unwrap(); + let sidebar = DEFAULT_CONFIG.find("# [ui.sidebar.agents]").unwrap(); assert!(accent < sidebar); }
|
CodeRabbit review 4913662039: agreed, fixed in dfb0ca1. |
Greptile SummaryThe PR corrects the generated default configuration’s TOML ordering and adds focused regression coverage.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security issues identified. The generated configuration now places the
|
| Filename | Overview |
|---|---|
| src/main.rs | Reorders the ui.accent example in DEFAULT_CONFIG and adds a focused ordering regression test; no actionable issues found. |
Reviews (1): Last reviewed commit: "test: reject duplicate ui accent example..." | Re-trigger Greptile
Summary
ui.accentexample before nested[ui.*]tables in--default-configChecks
cargo test default_config_lists_ui_accent_before_nested_tables -- --nocapture--default-config, uncommentedui.accentand[ui.sidebar.spaces]in place, thenherdr config check(config: ok)cargo fmt --checkcargo clippy --all-targets --locked -- -D warningsjust checkreached 959 passing tests, then hit the pre-existing live-handoff test harness assumption that test binaries live under<checkout>/target; this issue environment requires an external isolatedCARGO_TARGET_DIRrefs #2697