fix: surface min loadout list parse failures on stderr and exit non-zero - #1152
Conversation
`min loadout list` folded every discovered entry, including files that failed to parse, into the output table. The multi-line TOML parse error landed in the counts column and corrupted the layout, the command still returned `Ok(())` so callers saw a success, and each error printed the file path twice because the row format prefixed a path that `LoadError`'s Display already includes. Partition the entries instead: parseable loadouts become table rows, while parse failures are reported on stderr (no duplicate path) and make the command exit non-zero when any file is broken.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
Reviewed — soundThe partition is the right shape: parse failures go to stderr, the table of valid loadouts still prints, and the non-zero exit comes after the output so a human still sees the usable rows while a script can detect the breakage. That is a better answer than either dropping the table or aborting on first error. Keeping failures out of the table is load-bearing for a reason the comment states well — multi-line parse errors were corrupting the column layout, which was one of the three defects in gominimal/inbox#391, not just cosmetic fallout. Two details worth noting as correct rather than accidental:
No changes requested. |
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABKmgtQQ
min loadout listfolded every discovered.tomlfile — including ones that fail to parse — into its table. Three defects followed: a multi-line TOML parse error in the counts column corrupted the layout; the command exited 0 even when a file failed to parse, hiding the failure from scripts; and each error printed the file path twice (the row prefixed a pathLoadErroralready names). Now parse failures go to stderr (single path) and are kept out of the table, and the command exits non-zero when any file fails; the table of valid loadouts stays intact. Surfacing CLI errors this way follows the existing convention (informed by #95).Verification
cargo fmt --all --check --manifest-path target/Cargo.toml— clean, no driftcargo clippy --workspace --locked --manifest-path target/Cargo.toml -- -D warnings— 0 warningscargo build --workspace --locked --manifest-path target/Cargo.toml— okcargo test --workspace --locked --manifest-path target/Cargo.toml— all suites pass; new testcmd_loadout_list_errors_on_malformed_entryokNote
Surface
loadout listparse failures on stderr and exit non-zerocmd_loadout_listnow partitions discovered loadout entries into successes and failures instead of embedding errors as table rows.LoadoutRow::from_entryin loadouts.rs now accepts a pre-parsed loadout, removing the error-embedding code path..tomlfiles previously appeared as rows with an(error at …)message; they are now excluded from the table and cause a non-zero exit.Macroscope summarized 4aa6b2a.