1
0
mirror of https://github.com/sharkdp/bat synced 2026-06-09 10:03:18 +00:00

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<RawMatcher> 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<RawMatcher> 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.
This commit is contained in:
Keith Hall
2026-03-20 23:53:30 +02:00
parent 7a6f442c86
commit a19593b383
+8 -6
View File
@@ -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<Option<GlobMatcher>>`.
@@ -161,16 +162,17 @@ enum RawMatcher {
},
}
impl<'de> serde::Deserialize<'de> for Matcher {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let raw = RawMatcher::deserialize(deserializer)?;
impl TryFrom<RawMatcher> for Matcher {
type Error = anyhow::Error;
fn try_from(raw: RawMatcher) -> Result<Self, Self::Error> {
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 {