From fda4e40c043f8378d188ae5deb1fa1d4cf0cb40c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 3 Nov 2025 08:43:55 +0100 Subject: [PATCH 1/2] fix!: make `Time::to_zoned()` fallible and add `Time::format_or_raw()`. After all, the timezone can be invalid, which makes the conversion (and formatting) fail. The `format_or_raw()` method brings back an infallible version. --- gix-date/src/time/format.rs | 26 +++++++++------ gix-date/tests/time/baseline.rs | 20 ++++++------ gix-date/tests/time/format.rs | 57 +++++++++++++++++++-------------- 3 files changed, 60 insertions(+), 43 deletions(-) diff --git a/gix-date/src/time/format.rs b/gix-date/src/time/format.rs index 76f45f25563..552c143018f 100644 --- a/gix-date/src/time/format.rs +++ b/gix-date/src/time/format.rs @@ -36,25 +36,31 @@ impl Time { /// /// Use [`Format::Unix`], [`Format::Raw`] or one of the custom formats /// defined in the [`format`](mod@crate::time::format) submodule. - pub fn format(&self, format: impl Into) -> String { + /// + /// Note that this can fail if the timezone isn't valid and the format requires a conversion to [`jiff::Zoned`]. + pub fn format(&self, format: impl Into) -> Result { self.format_inner(format.into()) } - fn format_inner(&self, format: Format) -> String { - match format { - Format::Custom(CustomFormat(format)) => self.to_zoned().strftime(format).to_string(), + /// Like [`Self::format()`], but on time conversion error, produce the [RAW] format instead to make it + /// infallible. + pub fn format_or_raw(&self, format: impl Into) -> String { + self.format_inner(format.into()).unwrap_or_else(|_| self.to_string()) + } + + fn format_inner(&self, format: Format) -> Result { + Ok(match format { + Format::Custom(CustomFormat(format)) => self.to_zoned()?.strftime(format).to_string(), Format::Unix => self.seconds.to_string(), Format::Raw => self.to_string(), - } + }) } } impl Time { /// Produce a `Zoned` time for complex time computations and limitless formatting. - pub fn to_zoned(self) -> jiff::Zoned { - let offset = jiff::tz::Offset::from_seconds(self.offset).expect("valid offset"); - jiff::Timestamp::from_second(self.seconds) - .expect("always valid unix time") - .to_zoned(offset.to_time_zone()) + pub fn to_zoned(self) -> Result { + let offset = jiff::tz::Offset::from_seconds(self.offset)?; + Ok(jiff::Timestamp::from_second(self.seconds)?.to_zoned(offset.to_time_zone())) } } diff --git a/gix-date/tests/time/baseline.rs b/gix-date/tests/time/baseline.rs index 4eff7cef066..67b9e95f8cd 100644 --- a/gix-date/tests/time/baseline.rs +++ b/gix-date/tests/time/baseline.rs @@ -66,15 +66,17 @@ fn parse_compare_format() { "{pattern:?} disagrees with baseline seconds since epoch: {actual:?}" ); if let Some(format_name) = format_name { - let reformatted = t.format(match format_name.as_str() { - "RFC2822" => Format::Custom(format::RFC2822), - "ISO8601" => Format::Custom(format::ISO8601), - "ISO8601_STRICT" => Format::Custom(format::ISO8601_STRICT), - "GITOXIDE" => Format::Custom(format::GITOXIDE), - "UNIX" => Format::Unix, - "RAW" => Format::Raw, - unknown => unreachable!("All formats should be well-known and implemented: {unknown:?}"), - }); + let reformatted = t + .format(match format_name.as_str() { + "RFC2822" => Format::Custom(format::RFC2822), + "ISO8601" => Format::Custom(format::ISO8601), + "ISO8601_STRICT" => Format::Custom(format::ISO8601_STRICT), + "GITOXIDE" => Format::Custom(format::GITOXIDE), + "UNIX" => Format::Unix, + "RAW" => Format::Raw, + unknown => unreachable!("All formats should be well-known and implemented: {unknown:?}"), + }) + .expect("valid input time"); assert_eq!( reformatted, *pattern, "{reformatted:?} disagrees with baseline pattern: {pattern:?}" diff --git a/gix-date/tests/time/format.rs b/gix-date/tests/time/format.rs index 06e5b8b774c..d5376aa19ec 100644 --- a/gix-date/tests/time/format.rs +++ b/gix-date/tests/time/format.rs @@ -4,19 +4,21 @@ use gix_date::{ }; #[test] -fn short() { - assert_eq!(time().format(format::SHORT), "1973-11-30"); +fn short() -> gix_testtools::Result { + assert_eq!(time().format(format::SHORT)?, "1973-11-30"); + Ok(()) } #[test] -fn unix() { +fn unix() -> gix_testtools::Result { let expected = "123456789"; - assert_eq!(time().format(Format::Unix), expected); - assert_eq!(time().format(format::UNIX), expected); + assert_eq!(time().format(Format::Unix)?, expected); + assert_eq!(time().format(format::UNIX)?, expected); + Ok(()) } #[test] -fn raw() { +fn raw() -> gix_testtools::Result { for (time, expected) in [ (time(), "123456789 +0230"), ( @@ -27,58 +29,65 @@ fn raw() { "1112911993 +0100", ), ] { - assert_eq!(time.format(Format::Raw), expected); - assert_eq!(time.format(format::RAW), expected); + assert_eq!(time.format(Format::Raw)?, expected); + assert_eq!(time.format(format::RAW)?, expected); } + Ok(()) } #[test] -fn iso8601() { - assert_eq!(time().format(format::ISO8601), "1973-11-30 00:03:09 +0230"); +fn iso8601() -> gix_testtools::Result { + assert_eq!(time().format(format::ISO8601)?, "1973-11-30 00:03:09 +0230"); + Ok(()) } #[test] -fn iso8601_strict() { - assert_eq!(time().format(format::ISO8601_STRICT), "1973-11-30T00:03:09+02:30"); +fn iso8601_strict() -> gix_testtools::Result { + assert_eq!(time().format(format::ISO8601_STRICT)?, "1973-11-30T00:03:09+02:30"); + Ok(()) } #[test] -fn rfc2822() { - assert_eq!(time().format(format::RFC2822), "Fri, 30 Nov 1973 00:03:09 +0230"); - assert_eq!(time_dec1().format(format::RFC2822), "Sat, 01 Dec 1973 00:03:09 +0230"); +fn rfc2822() -> gix_testtools::Result { + assert_eq!(time().format(format::RFC2822)?, "Fri, 30 Nov 1973 00:03:09 +0230"); + assert_eq!(time_dec1().format(format::RFC2822)?, "Sat, 01 Dec 1973 00:03:09 +0230"); + Ok(()) } #[test] -fn git_rfc2822() { - assert_eq!(time().format(format::GIT_RFC2822), "Fri, 30 Nov 1973 00:03:09 +0230"); +fn git_rfc2822() -> gix_testtools::Result { + assert_eq!(time().format(format::GIT_RFC2822)?, "Fri, 30 Nov 1973 00:03:09 +0230"); assert_eq!( - time_dec1().format(format::GIT_RFC2822), + time_dec1().format(format::GIT_RFC2822)?, "Sat, 1 Dec 1973 00:03:09 +0230" ); + Ok(()) } #[test] -fn default() { +fn default() -> gix_testtools::Result { assert_eq!( - time().format(gix_date::time::format::GITOXIDE), + time().format(gix_date::time::format::GITOXIDE)?, "Fri Nov 30 1973 00:03:09 +0230" ); assert_eq!( - time_dec1().format(gix_date::time::format::GITOXIDE), + time_dec1().format(gix_date::time::format::GITOXIDE)?, "Sat Dec 01 1973 00:03:09 +0230" ); + Ok(()) } #[test] -fn git_default() { +fn git_default() -> gix_testtools::Result { assert_eq!( - time().format(gix_date::time::format::DEFAULT), + time().format(gix_date::time::format::DEFAULT)?, "Fri Nov 30 00:03:09 1973 +0230" ); assert_eq!( - time_dec1().format(gix_date::time::format::DEFAULT), + time_dec1().format(gix_date::time::format::DEFAULT)?, "Sat Dec 1 00:03:09 1973 +0230" ); + Ok(()) } fn time() -> Time { From d5e194d0ab0e58b02d159b4f48c4e2edd946f355 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 3 Nov 2025 08:49:19 +0100 Subject: [PATCH 2/2] adapt to changes in `gix-date` --- examples/log.rs | 2 +- gitoxide-core/src/query/engine/command.rs | 4 ++-- gitoxide-core/src/repository/revision/explain.rs | 2 +- gix/src/repository/identity.rs | 2 +- gix/src/revision/spec/parse/error.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/log.rs b/examples/log.rs index 0ca592cb57d..76a651e3171 100644 --- a/examples/log.rs +++ b/examples/log.rs @@ -150,7 +150,7 @@ fn run(args: Args) -> anyhow::Result<()> { commit_ref.author.actor().write_to(&mut buf)?; buf.into() }, - time: commit_ref.author.time()?.format(format::DEFAULT), + time: commit_ref.author.time()?.format_or_raw(format::DEFAULT), message: commit_ref.message.to_owned(), }) }), diff --git a/gitoxide-core/src/query/engine/command.rs b/gitoxide-core/src/query/engine/command.rs index 1b020030821..ef3e6f85414 100644 --- a/gitoxide-core/src/query/engine/command.rs +++ b/gitoxide-core/src/query/engine/command.rs @@ -180,7 +180,7 @@ mod trace_path { out, "{}| {} | {} {} {} ➡ {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format(gix::date::time::format::SHORT), + self.commit_time.format_or_raw(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&source_id], @@ -192,7 +192,7 @@ mod trace_path { out, "{}| {} | {} {} {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format(gix::date::time::format::SHORT), + self.commit_time.format_or_raw(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&self.file_id] diff --git a/gitoxide-core/src/repository/revision/explain.rs b/gitoxide-core/src/repository/revision/explain.rs index 07bee89cb89..ecc75c47d3f 100644 --- a/gitoxide-core/src/repository/revision/explain.rs +++ b/gitoxide-core/src/repository/revision/explain.rs @@ -88,7 +88,7 @@ impl delegate::Revision for Explain<'_> { ReflogLookup::Date(time) => writeln!( self.out, "Find entry closest to time {} in reflog of '{}' reference", - time.format(gix::date::time::format::ISO8601), + time.format_or_raw(gix::date::time::format::ISO8601), ref_name ) .ok(), diff --git a/gix/src/repository/identity.rs b/gix/src/repository/identity.rs index 7837e9f2d65..212fe78b4b3 100644 --- a/gix/src/repository/identity.rs +++ b/gix/src/repository/identity.rs @@ -140,7 +140,7 @@ impl Personas { .and_then(|date| gix_date::parse(date, Some(SystemTime::now())).ok()) }) .or_else(|| Some(gix_date::Time::now_local_or_utc())) - .map(|time| time.format(gix_date::time::Format::Raw)) + .map(|time| time.format_or_raw(gix_date::time::Format::Raw)) }; let fallback = ( diff --git a/gix/src/revision/spec/parse/error.rs b/gix/src/revision/spec/parse/error.rs index eb361238995..513b3e886e2 100644 --- a/gix/src/revision/spec/parse/error.rs +++ b/gix/src/revision/spec/parse/error.rs @@ -44,7 +44,7 @@ impl std::fmt::Display for CandidateInfo { "commit {} {title:?}", gix_date::parse_header(date) .unwrap_or_default() - .format(gix_date::time::format::SHORT) + .format_or_raw(gix_date::time::format::SHORT) ) } }