feat(filters): add cmake output filter#267
Conversation
Fixes h5i-dev#207 Co-Authored-By: Hermes Agent <noreply@nousresearch.com>
Koukyosyumei
left a comment
There was a problem hiding this comment.
Summary
Adds a single new declarative filter asset, assets/filters/cmake.toml, matching ^cmake\b. It strips CMake configure probe chatter while preserving warnings, errors, and the final configure/generate/build-files status, plus four inline golden tests. It's a clean, additive, self-contained change that follows the established pattern in src/filter_rules.rs.
What's good
- Correct engine usage. All fields used (
description,match_command,strip_ansi,strip_lines_matching,truncate_lines_at,max_lines,on_empty) are valid underFilterDef, which is#[serde(deny_unknown_fields)](src/filter_rules.rs:66), so the file parses andbuiltin_rules_all_parsestays green. - Golden tests pass. I traced all four cases through
apply_filter(src/filter_rules.rs:248) and reproduced the engine (.lines()drops the trailing newline,^\s*$strips blanks, empty result →on_empty). All four PASS: success keeps final status, error/warning blocks are preserved, empty run collapses tocmake: clean. Good coverage of the meaningful states. - Preserves the important negatives.
-- Could NOT find …andCMake Deprecation/Warninglines do not match any strip pattern, so real signal is retained — exactly right. - Conventions match siblings. Structure mirrors
make.toml/gcc.toml; the em-dash indescriptionis consistent withgcc.toml's description, andon_emptymirrorsmake: ok. No manual registration needed (RustEmbed auto-includesassets/filters/*.toml), and there's nomatch_commandoverlap with any other rule, so routing stays unambiguous.
Minor notes (non-blocking)
-
Redundant strip patterns. Patterns 3 and 4 are largely subsumed by pattern 2:
^-- (Detecting|Check for working|Detecting .* done)— real CMakeDetecting …lines always have a trailing space, already caught by pattern 2'sDetecting, andCheck for workingis caught by pattern 2'sCheck(ing)?.^-- (C|CXX|CUDA|ASM) compiler identification is— real output is-- The C compiler identification is …, already caught by pattern 2'sThe.
They work as harmless belt-and-suspenders (they'd catch hypothetical output lacking the
The/space prefix), but consider trimming to keep the rule tight. Not required. -
-- Found <pkg>is stripped. This drops successful-find lines like-- Found OpenSSL: … (found version …). That's a reasonable probe-chatter tradeoff since warnings/errors are kept, but worth being aware it also removes the version-confirmation some users scan for. -
Build-phase interaction. Since routing is first-match-per-command,
cmake --build .is claimed by this rule, so compiler errors emitted during the build pass through without thegccrule's include-chain stripping. Acceptable given the one-rule-per-command model; just noting the scope. -
Test plan. The author notes
cargo test builtin_golden_tests_passwas blocked locally (nocc). I confirmed the four cases pass via a faithful simulation of the engine, but CI should be the source of truth here.
Verdict
No blocking issues. Correct, well-tested, convention-consistent additive change. The regex-redundancy trim is optional polish and can be done in a follow-up or ignored.
Summary
Fixes #207
Adds a declarative built-in output filter for
cmakeconfigure/build output, preserving warnings, errors, and final configure/generate status while dropping routine probe chatter.Changes
assets/filters/cmake.tomlwith a^cmake\bmatcher-- Detecting/-- Found/ compiler probe lines and blanksTest Plan
python3semantic check of the inline golden cases: passH5I_SKIP_WEB_BUILD=1 cargo test builtin_golden_tests_pass -- --nocapture(blocked in this environment: linkerccis not installed)