From a19593b383f208106d019cbca3546cb0748eb377 Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Fri, 20 Mar 2026 23:53:30 +0200 Subject: [PATCH] improvements from PR review 1. Added `Deserialize` derive and `#[serde(try_from = "RawMatcher")]` to the `Matcher` struct. With `try_from`, serde generates a `Deserialize` impl that first deserializes into `RawMatcher`, then calls `TryFrom for Matcher`. It never attempts to deserialize `Matcher`'s fields directly, so `Case` and `MatcherSegment` don't need `Deserialize` impls. 2. Replaced the manual `impl<'de> serde::Deserialize<'de> for Matcher` with a standard `impl TryFrom for Matcher`. The logic is identical - the conversion is fallible because `Matcher::from_str` returns `Result<_, anyhow::Error>`, so `try_from` (not `from`) is the correct choice, avoiding any panics. The net effect: same behavior, same error handling, but using the idiomatic serde pattern instead of a manual deserializer impl. The `RawMatcher` intermediate type and its `#[serde(untagged)]` derive remain unchanged. --- build/syntax_mapping.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/build/syntax_mapping.rs b/build/syntax_mapping.rs index bc4601fc..c7133be9 100644 --- a/build/syntax_mapping.rs +++ b/build/syntax_mapping.rs @@ -66,7 +66,8 @@ impl ToTokens for Case { } } -#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash, Deserialize)] +#[serde(try_from = "RawMatcher")] /// A single matcher. /// /// Codegen converts this into a `Lazy>`. @@ -161,16 +162,17 @@ enum RawMatcher { }, } -impl<'de> serde::Deserialize<'de> for Matcher { - fn deserialize>(deserializer: D) -> Result { - let raw = RawMatcher::deserialize(deserializer)?; +impl TryFrom for Matcher { + type Error = anyhow::Error; + + fn try_from(raw: RawMatcher) -> Result { match raw { - RawMatcher::Simple(s) => Matcher::from_str(&s).map_err(serde::de::Error::custom), + RawMatcher::Simple(s) => Matcher::from_str(&s), RawMatcher::Full { glob, case_sensitive, } => { - let mut matcher = Matcher::from_str(&glob).map_err(serde::de::Error::custom)?; + let mut matcher = Matcher::from_str(&glob)?; matcher.case = if case_sensitive { Case::Sensitive } else {