refactor(ops)!: rename try_filter_map to try_map_filter - #884
Merged
Conversation
The catalog's fallible-twin convention is `try_` plus the exact base name: `map` -> `try_map`, `join` -> `try_join`, `join_passive` -> `try_join_passive`. The base here is `map_filter`, so the twin is `try_map_filter`. The op's own rustdoc already called it "the `try_` counterpart to `map_filter`" while carrying a name that said otherwise. The concrete trap this removes is on the `Signal` facade, which already has a `filter_map` taking `Fn(&T) -> Option<B>`. Landing a `try_filter_map` taking `Fn(&T) -> Result<(B, bool)>` next to it put two methods side by side that read as an infallible/fallible pair and are not one — different shapes, different call sites, no relationship. `try_map_filter` sits beside `map_filter`, whose `(value, emit?)` shape it does mirror exactly. Renaming rather than reshaping to `Result<Option<B>>` is deliberate. The dummy-`B` on the reject path is a real wart, but it is inherited from `map_filter`'s `(B, bool)` shape; giving the fallible twin an `Option` while the infallible one keeps a `bool` would make the pair less consistent, not more. An `Option`-shaped convenience belongs on `Signal`, layered on this op exactly as `Signal::filter_map` layers on `map_filter` today. Also included, from the same review: - A `Signal` facade test (`tests/signal.rs`). The facade forward had no test at all — deleting the `__wf_signal_try_map_filter!` line left the build and the full suite green, which is the drift the op recipe's step 4b documents as having already cost 15 methods once. Verified the new test closes it: removing the macro line now fails to compile. - A disambiguating note on `Signal::filter_map` pointing at the shape difference, so the two neighbours explain themselves. No release carries the old name, so nothing downstream breaks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gtvqds3f7LPbxT2RT2NzG6
This was referenced Aug 18, 2026
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
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
Follow-up to #880 (which resolved #801), acting on two findings from its review. No release carries the old name, so nothing downstream breaks — this is free now and breaking later.
The rename
The catalog's fallible-twin convention is
try_plus the exact base name:map→try_map,join→try_join,join_passive→try_join_passive. The base here ismap_filter, so the twin istry_map_filter. The op's own rustdoc already described it as "thetry_counterpart tomap_filter" while carrying a name that said otherwise.The concrete trap this removes is on the
Signalfacade, which already has afilter_maptakingFn(&T) -> Option<B>. Landing atry_filter_maptakingFn(&T) -> Result<(B, bool)>next to it put two methods side by side that read as an infallible/fallible pair and are not one — different shapes, different call sites, no relationship between them.try_map_filtersits besidemap_filter, whose(value, emit?)shape it does mirror exactly.Why rename rather than reshape to
Result<Option<B>>. The dummyBon the reject path (Ok((B::default(), false))) is a real wart, but it is inherited frommap_filter's(B, bool)shape — it is not introduced here. Giving the fallible twin anOptionwhile the infallible one keeps aboolwould make the pair less consistent, not more; matching shapes are what make "fallible twin" checkable at a glance. AnOption-shaped convenience belongs onSignal, layered over this op exactly asSignal::filter_maplayers overmap_filtertoday. Happy to add that separately if wanted.Also included, from the same review
Signalfacade test (tests/signal.rs). The facade forward shipped with no test anywhere — deleting the__wf_signal_try_map_filter!(T)line left the build and the full suite green. That is exactly the facade drift the op recipe's step 4b records as having already cost 15 methods once. Verified the new test closes the hole: with the macro line removed the test crate now fails to compile (no method named try_map_filter found for struct Signal<T>).Signal::filter_mappointing at the shape difference, so the two neighbours explain themselves to the next reader.Changes
crates/wingfoil/src/ops.rs—TryFilterMap→TryMapFilter,build = try_map_filtercrates/wingfoil/src/fluent.rs—StreamOpsdeclaration and macro invocationcrates/wingfoil/src/signal.rs— facade forward, plus the note onfilter_mapcrates/wingfoil/tests/{catalog,fallibility,op_completeness}.rs— call sites and themsg.contains("TryMapFilter")assertioncrates/wingfoil/tests/signal.rs— newlegacy_try_map_filter_maps_and_filtersThe generated macro names follow
build =automatically, so__wf_fluent_try_filter_map!/__wf_signal_try_filter_map!become__wf_fluent_try_map_filter!/__wf_signal_try_map_filter!with no separate edit.Test plan
cargo fmt --all -- --check— cleancargo test -p wingfoil— full suite green (95 unit + all integration targets)cargo test --doc -p wingfoil— green; the new intra-doc link resolves (cargo doc --no-depssurfaces no new warnings)cargo test -p wingfoil-derive— greencargo lint(default features) — cleantry_filter_map/TryFilterMapoccurrences remain in the treecargo lint-all/--all-features— not run here; this box has noprotoc. The rename touches no feature-gated code, and CI's all-features leg covers it.Note for #801
Issue #801 specified the name and the
#[op(build = try_filter_map, fluent)]attribute verbatim, so @zaydmulani09 implemented exactly what was asked — this is a spec correction, not a defect in that PR. Worth editing the issue text if any of it is still being used as a template.Generated by Claude Code