feat!: remove Time::sign field as it's not needed anymore to round-trip.

Round-tripping is now handled in the `gix-actor` crate using the `SignatureRef`
type.

Author: Byron

gix-date/src/lib.rs

@@ -24,8 +24,6 @@ pub struct Time {
     pub seconds: SecondsSinceUnixEpoch,
     /// The time's offset in seconds, which may be negative to match the `sign` field.
     pub offset: OffsetInSeconds,
-    /// the sign of `offset`, used to encode `-0000` which would otherwise lose sign information.
-    pub sign: time::Sign,
 }
 
 /// The number of seconds since unix epoch.

gix-date/src/parse.rs

@@ -77,11 +77,8 @@ pub(crate) mod function {
 
     use crate::{
         parse::{relative, Error},
-        time::{
-            format::{DEFAULT, GITOXIDE, ISO8601, ISO8601_STRICT, SHORT},
-            Sign,
-        },
-        SecondsSinceUnixEpoch, Time,
+        time::format::{DEFAULT, GITOXIDE, ISO8601, ISO8601_STRICT, SHORT},
+        OffsetInSeconds, SecondsSinceUnixEpoch, Time,
     };
 
     /// Parse `input` as any time that Git can parse when inputting a date.
@@ -169,46 +166,73 @@ pub(crate) mod function {
         } else if let Ok(val) = SecondsSinceUnixEpoch::from_str(input) {
             // Format::Unix
             Time::new(val, 0)
+        } else if let Some(val) = relative::parse(input, now).transpose()? {
+            Time::new(val.timestamp().as_second(), val.offset().seconds())
         } else if let Some(val) = parse_header(input) {
             // Format::Raw
             val
-        } else if let Some(val) = relative::parse(input, now).transpose()? {
-            Time::new(val.timestamp().as_second(), val.offset().seconds())
         } else {
             return Err(Error::InvalidDateString { input: input.into() });
         })
     }
 
     /// Unlike [`parse()`] which handles all kinds of input, this function only parses the commit-header format
     /// like `1745582210 +0200`.
+    ///
+    /// Note that failure to parse the time zone isn't fatal, instead it will default to `0`. To know if
+    /// the time is wonky, serialize the return value to see if it matches the `input.`
     pub fn parse_header(input: &str) -> Option<Time> {
-        let mut split = input.split_whitespace();
-        let seconds: SecondsSinceUnixEpoch = split.next()?.parse().ok()?;
-        let offset = split.next()?;
-        if (offset.len() != 5) && (offset.len() != 7) || split.next().is_some() {
-            return None;
+        pub enum Sign {
+            Plus,
+            Minus,
         }
-        let sign = match offset.get(..1)? {
-            "-" => Some(Sign::Minus),
-            "+" => Some(Sign::Plus),
-            _ => None,
-        }?;
-        let hours: i32 = offset.get(1..3)?.parse().ok()?;
-        let minutes: i32 = offset.get(3..5)?.parse().ok()?;
-        let offset_seconds: i32 = if offset.len() == 7 {
-            offset.get(5..7)?.parse().ok()?
-        } else {
-            0
-        };
-        let mut offset_in_seconds = hours * 3600 + minutes * 60 + offset_seconds;
-        if sign == Sign::Minus {
-            offset_in_seconds *= -1;
+        fn parse_offset(offset: &str) -> Option<OffsetInSeconds> {
+            if (offset.len() != 5) && (offset.len() != 7) {
+                return None;
+            }
+            let sign = match offset.get(..1)? {
+                "-" => Some(Sign::Minus),
+                "+" => Some(Sign::Plus),
+                _ => None,
+            }?;
+            if offset.as_bytes().get(1).is_some_and(|b| !b.is_ascii_digit()) {
+                return None;
+            }
+            let hours: i32 = offset.get(1..3)?.parse().ok()?;
+            let minutes: i32 = offset.get(3..5)?.parse().ok()?;
+            let offset_seconds: i32 = if offset.len() == 7 {
+                offset.get(5..7)?.parse().ok()?
+            } else {
+                0
+            };
+            let mut offset_in_seconds = hours * 3600 + minutes * 60 + offset_seconds;
+            if matches!(sign, Sign::Minus) {
+                offset_in_seconds *= -1;
+            }
+            Some(offset_in_seconds)
         }
-        let time = Time {
-            seconds,
-            offset: offset_in_seconds,
-            sign,
+
+        let mut split = input.split_whitespace();
+        let seconds = split.next()?;
+        let seconds = match seconds.parse::<SecondsSinceUnixEpoch>() {
+            Ok(s) => s,
+            Err(_err) => {
+                // Inefficient, but it's not the common case.
+                let first_digits: String = seconds.chars().take_while(|b| b.is_ascii_digit()).collect();
+                first_digits.parse().ok()?
+            }
+        };
+        let offset = match split.next() {
+            None => 0,
+            Some(offset) => {
+                if split.next().is_some() {
+                    0
+                } else {
+                    parse_offset(offset).unwrap_or_default()
+                }
+            }
         };
+        let time = Time { seconds, offset };
         Some(time)
     }
 

gix-date/src/time/init.rs

@@ -1,24 +1,16 @@
-use crate::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time};
+use crate::{OffsetInSeconds, SecondsSinceUnixEpoch, Time};
 
 /// Instantiation
 impl Time {
     /// Create a new instance from seconds and offset.
     pub fn new(seconds: SecondsSinceUnixEpoch, offset: OffsetInSeconds) -> Self {
-        Time {
-            seconds,
-            offset,
-            sign: offset.into(),
-        }
+        Time { seconds, offset }
     }
 
     /// Return the current time without figuring out a timezone offset
     pub fn now_utc() -> Self {
         let seconds = jiff::Timestamp::now().as_second();
-        Self {
-            seconds,
-            offset: 0,
-            sign: Sign::Plus,
-        }
+        Self { seconds, offset: 0 }
     }
 
     /// Return the current local time, or `None` if the local time wasn't available.
@@ -31,10 +23,6 @@ impl Time {
         let zdt = jiff::Zoned::now();
         let seconds = zdt.timestamp().as_second();
         let offset = zdt.offset().seconds();
-        Self {
-            seconds,
-            offset,
-            sign: offset.into(),
-        }
+        Self { seconds, offset }
     }
 }

gix-date/src/time/mod.rs

@@ -8,15 +8,6 @@ impl Time {
     }
 }
 
-/// Indicates if a number is positive or negative for use in [`Time`].
-#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-#[allow(missing_docs)]
-pub enum Sign {
-    Plus,
-    Minus,
-}
-
 /// Various ways to describe a time format.
 #[derive(Debug, Clone, Copy)]
 pub enum Format {
@@ -50,30 +41,12 @@ pub mod format;
 mod init;
 mod write;
 
-mod sign {
-    use crate::time::Sign;
-
-    impl From<i32> for Sign {
-        fn from(v: i32) -> Self {
-            if v < 0 {
-                Sign::Minus
-            } else {
-                Sign::Plus
-            }
-        }
-    }
-}
-
 mod impls {
-    use crate::{time::Sign, Time};
+    use crate::Time;
 
     impl Default for Time {
         fn default() -> Self {
-            Time {
-                seconds: 0,
-                offset: 0,
-                sign: Sign::Plus,
-            }
+            Time { seconds: 0, offset: 0 }
         }
     }
 }

gix-date/src/time/write.rs

@@ -1,6 +1,6 @@
 use bstr::ByteSlice;
 
-use crate::{time::Sign, SecondsSinceUnixEpoch, Time};
+use crate::{SecondsSinceUnixEpoch, Time};
 
 /// Serialize this instance as string, similar to what [`write_to()`](Self::write_to()) would do.
 impl std::fmt::Display for Time {
@@ -33,10 +33,7 @@ impl Time {
         let mut itoa = itoa::Buffer::new();
         out.write_all(itoa.format(self.seconds).as_bytes())?;
         out.write_all(b" ")?;
-        out.write_all(match self.sign {
-            Sign::Plus => b"+",
-            Sign::Minus => b"-",
-        })?;
+        out.write_all(if self.offset < 0 { b"-" } else { b"+" })?;
 
         const ZERO: &[u8; 1] = b"0";
 
@@ -138,6 +135,5 @@ impl Time {
     pub const MAX: Time = Time {
         seconds: SecondsSinceUnixEpoch::MAX,
         offset: 99 * 60 * 60 + 59 * 60 + 59,
-        sign: Sign::Plus,
     };
 }

gix-date/tests/time/format.rs

@@ -1,5 +1,5 @@
 use gix_date::{
-    time::{format, Format, Sign},
+    time::{format, Format},
     Time,
 };
 
@@ -23,7 +23,6 @@ fn raw() {
             Time {
                 seconds: 1112911993,
                 offset: 3600,
-                sign: Sign::Plus,
             },
             "1112911993 +0100",
         ),
@@ -86,14 +85,12 @@ fn time() -> Time {
     Time {
         seconds: 123456789,
         offset: 9000,
-        sign: Sign::Plus,
     }
 }
 
 fn time_dec1() -> Time {
     Time {
         seconds: 123543189,
         offset: 9000,
-        sign: Sign::Plus,
     }
 }

gix-date/tests/time/mod.rs

@@ -3,23 +3,6 @@ use gix_date::Time;
 mod baseline;
 mod format;
 mod parse;
-mod init {
-    use gix_date::Time;
-
-    #[test]
-    fn utc_local_handles_signs_correctly() {
-        for time in [
-            Time::now_local_or_utc(),
-            Time::now_local().unwrap_or_else(Time::now_utc),
-        ] {
-            assert_eq!(
-                time.sign,
-                time.offset.into(),
-                "the sign matches the sign of the date offset"
-            );
-        }
-    }
-}
 
 #[test]
 fn is_set() {
@@ -34,15 +17,13 @@ fn is_set() {
 mod write_to {
     use bstr::ByteSlice;
     use gix_date::parse::TimeBuf;
-    use gix_date::time::Sign;
     use gix_date::{SecondsSinceUnixEpoch, Time};
 
     #[test]
     fn invalid() {
         let time = Time {
             seconds: 0,
             offset: (100 * 60 * 60) + 30 * 60,
-            sign: Sign::Plus,
         };
         let err = time.write_to(&mut Vec::new()).unwrap_err();
         assert_eq!(err.to_string(), "Cannot represent offsets larger than +-9900");
@@ -55,79 +36,63 @@ mod write_to {
                 Time {
                     seconds: SecondsSinceUnixEpoch::MAX,
                     offset: 0,
-                    sign: Sign::Minus,
                 },
-                "9223372036854775807 -0000",
+                "9223372036854775807 +0000",
             ),
             (
                 Time {
                     seconds: SecondsSinceUnixEpoch::MIN,
                     offset: 0,
-                    sign: Sign::Minus,
                 },
-                "-9223372036854775808 -0000",
+                "-9223372036854775808 +0000",
             ),
             (
                 Time {
                     seconds: 500,
                     offset: 9000,
-                    sign: Sign::Plus,
                 },
                 "500 +0230",
             ),
             (
                 Time {
                     seconds: 189009009,
                     offset: -36000,
-                    sign: Sign::Minus,
                 },
                 "189009009 -1000",
             ),
-            (
-                Time {
-                    seconds: 0,
-                    offset: 0,
-                    sign: Sign::Minus,
-                },
-                "0 -0000",
-            ),
+            (Time { seconds: 0, offset: 0 }, "0 +0000"),
             (
                 Time {
                     seconds: 0,
                     offset: -24 * 60 * 60,
-                    sign: Sign::Minus,
                 },
                 "0 -2400",
             ),
             (
                 Time {
                     seconds: 0,
                     offset: 24 * 60 * 60,
-                    sign: Sign::Plus,
                 },
                 "0 +2400",
             ),
             (
                 Time {
                     seconds: 0,
                     offset: (25 * 60 * 60) + 30 * 60,
-                    sign: Sign::Plus,
                 },
                 "0 +2530",
             ),
             (
                 Time {
                     seconds: 0,
                     offset: (-25 * 60 * 60) - 30 * 60,
-                    sign: Sign::Minus,
                 },
                 "0 -2530",
             ),
             (
                 Time {
                     seconds: 0,
                     offset: (99 * 60 * 60) + 59 * 60,
-                    sign: Sign::Plus,
                 },
                 "0 +9959",
             ),