Skip to content

Commit d559fa7

Browse files
committed
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.
1 parent afdf1a5 commit d559fa7

File tree

8 files changed

+83
-152
lines changed

8 files changed

+83
-152
lines changed

gix-date/src/lib.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ pub struct Time {
2424
pub seconds: SecondsSinceUnixEpoch,
2525
/// The time's offset in seconds, which may be negative to match the `sign` field.
2626
pub offset: OffsetInSeconds,
27-
/// the sign of `offset`, used to encode `-0000` which would otherwise lose sign information.
28-
pub sign: time::Sign,
2927
}
3028

3129
/// The number of seconds since unix epoch.

gix-date/src/parse.rs

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,8 @@ pub(crate) mod function {
7777

7878
use crate::{
7979
parse::{relative, Error},
80-
time::{
81-
format::{DEFAULT, GITOXIDE, ISO8601, ISO8601_STRICT, SHORT},
82-
Sign,
83-
},
84-
SecondsSinceUnixEpoch, Time,
80+
time::format::{DEFAULT, GITOXIDE, ISO8601, ISO8601_STRICT, SHORT},
81+
OffsetInSeconds, SecondsSinceUnixEpoch, Time,
8582
};
8683

8784
/// Parse `input` as any time that Git can parse when inputting a date.
@@ -169,46 +166,73 @@ pub(crate) mod function {
169166
} else if let Ok(val) = SecondsSinceUnixEpoch::from_str(input) {
170167
// Format::Unix
171168
Time::new(val, 0)
169+
} else if let Some(val) = relative::parse(input, now).transpose()? {
170+
Time::new(val.timestamp().as_second(), val.offset().seconds())
172171
} else if let Some(val) = parse_header(input) {
173172
// Format::Raw
174173
val
175-
} else if let Some(val) = relative::parse(input, now).transpose()? {
176-
Time::new(val.timestamp().as_second(), val.offset().seconds())
177174
} else {
178175
return Err(Error::InvalidDateString { input: input.into() });
179176
})
180177
}
181178

182179
/// Unlike [`parse()`] which handles all kinds of input, this function only parses the commit-header format
183180
/// like `1745582210 +0200`.
181+
///
182+
/// Note that failure to parse the time zone isn't fatal, instead it will default to `0`. To know if
183+
/// the time is wonky, serialize the return value to see if it matches the `input.`
184184
pub fn parse_header(input: &str) -> Option<Time> {
185-
let mut split = input.split_whitespace();
186-
let seconds: SecondsSinceUnixEpoch = split.next()?.parse().ok()?;
187-
let offset = split.next()?;
188-
if (offset.len() != 5) && (offset.len() != 7) || split.next().is_some() {
189-
return None;
185+
pub enum Sign {
186+
Plus,
187+
Minus,
190188
}
191-
let sign = match offset.get(..1)? {
192-
"-" => Some(Sign::Minus),
193-
"+" => Some(Sign::Plus),
194-
_ => None,
195-
}?;
196-
let hours: i32 = offset.get(1..3)?.parse().ok()?;
197-
let minutes: i32 = offset.get(3..5)?.parse().ok()?;
198-
let offset_seconds: i32 = if offset.len() == 7 {
199-
offset.get(5..7)?.parse().ok()?
200-
} else {
201-
0
202-
};
203-
let mut offset_in_seconds = hours * 3600 + minutes * 60 + offset_seconds;
204-
if sign == Sign::Minus {
205-
offset_in_seconds *= -1;
189+
fn parse_offset(offset: &str) -> Option<OffsetInSeconds> {
190+
if (offset.len() != 5) && (offset.len() != 7) {
191+
return None;
192+
}
193+
let sign = match offset.get(..1)? {
194+
"-" => Some(Sign::Minus),
195+
"+" => Some(Sign::Plus),
196+
_ => None,
197+
}?;
198+
if offset.as_bytes().get(1).is_some_and(|b| !b.is_ascii_digit()) {
199+
return None;
200+
}
201+
let hours: i32 = offset.get(1..3)?.parse().ok()?;
202+
let minutes: i32 = offset.get(3..5)?.parse().ok()?;
203+
let offset_seconds: i32 = if offset.len() == 7 {
204+
offset.get(5..7)?.parse().ok()?
205+
} else {
206+
0
207+
};
208+
let mut offset_in_seconds = hours * 3600 + minutes * 60 + offset_seconds;
209+
if matches!(sign, Sign::Minus) {
210+
offset_in_seconds *= -1;
211+
}
212+
Some(offset_in_seconds)
206213
}
207-
let time = Time {
208-
seconds,
209-
offset: offset_in_seconds,
210-
sign,
214+
215+
let mut split = input.split_whitespace();
216+
let seconds = split.next()?;
217+
let seconds = match seconds.parse::<SecondsSinceUnixEpoch>() {
218+
Ok(s) => s,
219+
Err(_err) => {
220+
// Inefficient, but it's not the common case.
221+
let first_digits: String = seconds.chars().take_while(|b| b.is_ascii_digit()).collect();
222+
first_digits.parse().ok()?
223+
}
224+
};
225+
let offset = match split.next() {
226+
None => 0,
227+
Some(offset) => {
228+
if split.next().is_some() {
229+
0
230+
} else {
231+
parse_offset(offset).unwrap_or_default()
232+
}
233+
}
211234
};
235+
let time = Time { seconds, offset };
212236
Some(time)
213237
}
214238

gix-date/src/time/init.rs

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,16 @@
1-
use crate::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time};
1+
use crate::{OffsetInSeconds, SecondsSinceUnixEpoch, Time};
22

33
/// Instantiation
44
impl Time {
55
/// Create a new instance from seconds and offset.
66
pub fn new(seconds: SecondsSinceUnixEpoch, offset: OffsetInSeconds) -> Self {
7-
Time {
8-
seconds,
9-
offset,
10-
sign: offset.into(),
11-
}
7+
Time { seconds, offset }
128
}
139

1410
/// Return the current time without figuring out a timezone offset
1511
pub fn now_utc() -> Self {
1612
let seconds = jiff::Timestamp::now().as_second();
17-
Self {
18-
seconds,
19-
offset: 0,
20-
sign: Sign::Plus,
21-
}
13+
Self { seconds, offset: 0 }
2214
}
2315

2416
/// Return the current local time, or `None` if the local time wasn't available.
@@ -31,10 +23,6 @@ impl Time {
3123
let zdt = jiff::Zoned::now();
3224
let seconds = zdt.timestamp().as_second();
3325
let offset = zdt.offset().seconds();
34-
Self {
35-
seconds,
36-
offset,
37-
sign: offset.into(),
38-
}
26+
Self { seconds, offset }
3927
}
4028
}

gix-date/src/time/mod.rs

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,6 @@ impl Time {
88
}
99
}
1010

11-
/// Indicates if a number is positive or negative for use in [`Time`].
12-
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
13-
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
14-
#[allow(missing_docs)]
15-
pub enum Sign {
16-
Plus,
17-
Minus,
18-
}
19-
2011
/// Various ways to describe a time format.
2112
#[derive(Debug, Clone, Copy)]
2213
pub enum Format {
@@ -50,30 +41,12 @@ pub mod format;
5041
mod init;
5142
mod write;
5243

53-
mod sign {
54-
use crate::time::Sign;
55-
56-
impl From<i32> for Sign {
57-
fn from(v: i32) -> Self {
58-
if v < 0 {
59-
Sign::Minus
60-
} else {
61-
Sign::Plus
62-
}
63-
}
64-
}
65-
}
66-
6744
mod impls {
68-
use crate::{time::Sign, Time};
45+
use crate::Time;
6946

7047
impl Default for Time {
7148
fn default() -> Self {
72-
Time {
73-
seconds: 0,
74-
offset: 0,
75-
sign: Sign::Plus,
76-
}
49+
Time { seconds: 0, offset: 0 }
7750
}
7851
}
7952
}

gix-date/src/time/write.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use bstr::ByteSlice;
22

3-
use crate::{time::Sign, SecondsSinceUnixEpoch, Time};
3+
use crate::{SecondsSinceUnixEpoch, Time};
44

55
/// Serialize this instance as string, similar to what [`write_to()`](Self::write_to()) would do.
66
impl std::fmt::Display for Time {
@@ -33,10 +33,7 @@ impl Time {
3333
let mut itoa = itoa::Buffer::new();
3434
out.write_all(itoa.format(self.seconds).as_bytes())?;
3535
out.write_all(b" ")?;
36-
out.write_all(match self.sign {
37-
Sign::Plus => b"+",
38-
Sign::Minus => b"-",
39-
})?;
36+
out.write_all(if self.offset < 0 { b"-" } else { b"+" })?;
4037

4138
const ZERO: &[u8; 1] = b"0";
4239

@@ -138,6 +135,5 @@ impl Time {
138135
pub const MAX: Time = Time {
139136
seconds: SecondsSinceUnixEpoch::MAX,
140137
offset: 99 * 60 * 60 + 59 * 60 + 59,
141-
sign: Sign::Plus,
142138
};
143139
}

gix-date/tests/time/format.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use gix_date::{
2-
time::{format, Format, Sign},
2+
time::{format, Format},
33
Time,
44
};
55

@@ -23,7 +23,6 @@ fn raw() {
2323
Time {
2424
seconds: 1112911993,
2525
offset: 3600,
26-
sign: Sign::Plus,
2726
},
2827
"1112911993 +0100",
2928
),
@@ -86,14 +85,12 @@ fn time() -> Time {
8685
Time {
8786
seconds: 123456789,
8887
offset: 9000,
89-
sign: Sign::Plus,
9088
}
9189
}
9290

9391
fn time_dec1() -> Time {
9492
Time {
9593
seconds: 123543189,
9694
offset: 9000,
97-
sign: Sign::Plus,
9895
}
9996
}

gix-date/tests/time/mod.rs

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,6 @@ use gix_date::Time;
33
mod baseline;
44
mod format;
55
mod parse;
6-
mod init {
7-
use gix_date::Time;
8-
9-
#[test]
10-
fn utc_local_handles_signs_correctly() {
11-
for time in [
12-
Time::now_local_or_utc(),
13-
Time::now_local().unwrap_or_else(Time::now_utc),
14-
] {
15-
assert_eq!(
16-
time.sign,
17-
time.offset.into(),
18-
"the sign matches the sign of the date offset"
19-
);
20-
}
21-
}
22-
}
236

247
#[test]
258
fn is_set() {
@@ -34,15 +17,13 @@ fn is_set() {
3417
mod write_to {
3518
use bstr::ByteSlice;
3619
use gix_date::parse::TimeBuf;
37-
use gix_date::time::Sign;
3820
use gix_date::{SecondsSinceUnixEpoch, Time};
3921

4022
#[test]
4123
fn invalid() {
4224
let time = Time {
4325
seconds: 0,
4426
offset: (100 * 60 * 60) + 30 * 60,
45-
sign: Sign::Plus,
4627
};
4728
let err = time.write_to(&mut Vec::new()).unwrap_err();
4829
assert_eq!(err.to_string(), "Cannot represent offsets larger than +-9900");
@@ -55,79 +36,63 @@ mod write_to {
5536
Time {
5637
seconds: SecondsSinceUnixEpoch::MAX,
5738
offset: 0,
58-
sign: Sign::Minus,
5939
},
60-
"9223372036854775807 -0000",
40+
"9223372036854775807 +0000",
6141
),
6242
(
6343
Time {
6444
seconds: SecondsSinceUnixEpoch::MIN,
6545
offset: 0,
66-
sign: Sign::Minus,
6746
},
68-
"-9223372036854775808 -0000",
47+
"-9223372036854775808 +0000",
6948
),
7049
(
7150
Time {
7251
seconds: 500,
7352
offset: 9000,
74-
sign: Sign::Plus,
7553
},
7654
"500 +0230",
7755
),
7856
(
7957
Time {
8058
seconds: 189009009,
8159
offset: -36000,
82-
sign: Sign::Minus,
8360
},
8461
"189009009 -1000",
8562
),
86-
(
87-
Time {
88-
seconds: 0,
89-
offset: 0,
90-
sign: Sign::Minus,
91-
},
92-
"0 -0000",
93-
),
63+
(Time { seconds: 0, offset: 0 }, "0 +0000"),
9464
(
9565
Time {
9666
seconds: 0,
9767
offset: -24 * 60 * 60,
98-
sign: Sign::Minus,
9968
},
10069
"0 -2400",
10170
),
10271
(
10372
Time {
10473
seconds: 0,
10574
offset: 24 * 60 * 60,
106-
sign: Sign::Plus,
10775
},
10876
"0 +2400",
10977
),
11078
(
11179
Time {
11280
seconds: 0,
11381
offset: (25 * 60 * 60) + 30 * 60,
114-
sign: Sign::Plus,
11582
},
11683
"0 +2530",
11784
),
11885
(
11986
Time {
12087
seconds: 0,
12188
offset: (-25 * 60 * 60) - 30 * 60,
122-
sign: Sign::Minus,
12389
},
12490
"0 -2530",
12591
),
12692
(
12793
Time {
12894
seconds: 0,
12995
offset: (99 * 60 * 60) + 59 * 60,
130-
sign: Sign::Plus,
13196
},
13297
"0 +9959",
13398
),

0 commit comments

Comments
 (0)