Remove partial support for handling -0000 offset
#1411
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1411 +/- ##
==========================================
- Coverage 91.84% 91.84% -0.01%
==========================================
Files 38 38
Lines 17518 17517 -1
==========================================
- Hits 16090 16089 -1
Misses 1428 1428 ☔ View full report in Codecov by Sentry. |
pitdicker
force-pushed
the
remove_unknown_offset
branch
3 times, most recently
from
1d1b336 to
dbf2d27
Compare
src/format/parse.rs
Outdated
| // This range check is similar to the one in `FixedOffset::east_opt`, so it would be redundant. | ||
| // But it is possible to read the offset directly from `Parsed`. We want to only successfully | ||
| // populate `Parsed` if the input is fully valid RFC 3339. | ||
| const MAX_OFFSET: i32 = 23 * 3600 + 59 * 60; |
There was a problem hiding this comment.
I'd like for this to be two commits:
- One for using a constant and the reformulated
Range::contains() - One for changing the value from 86400 to 863400
I also think 23 * 3600 + 59 * 60 is a surprising way to express this? I think (24 * 60 - 1) * 60 might be clearer.
There was a problem hiding this comment.
I also think
23 * 3600 + 59 * 60is a surprising way to express this? I think (24 * 60 - 1) * 60 might be clearer.
It comes from the definition in RFC 3339 as copied in comment of this function:
// time-hour = 2DIGIT ; 00-23
// time-numoffset = ("+" / "-") time-hour ":" time-minute
// time-minute = 2DIGIT ; 00-59
// time-offset = "Z" / time-numoffset
There was a problem hiding this comment.
Right. (23 * 60 + 59) * 60 could also work, I guess, but I find the previous suggestion more intuitive. I'll leave it up to you.
There was a problem hiding this comment.
Done. Added 'Max for the hours field is 23, and for the minutes field 59.' as a comment.
pitdicker
force-pushed
the
remove_unknown_offset
branch
from
dbf2d27 to
b476e7d
Compare
pitdicker
force-pushed
the
remove_unknown_offset
branch
from
b476e7d to
18d8459
Compare
2 participants
RFC 2822 and others have a special encoding to indicate the offset of a datetime is unknown:
-00:00.We currently have code in
format::parsethat ensures we do not provide an offset to theParsedstruct if the offset is-0000.timezone_offset_2822returns anOption<i32>that should beNoneto differentiate this case from+0000.But then
timezone_offset_2822does returnSome(0)on the-0000input. That makes theOptionand the code to handle it useless.I would like to see proper support for encoding this value in a
FixedOffset, as in #1042.But for now the inconsistency just messes up the error variants we return.
Because I am currently working on converting the parsing code from
ParsedErrorto a newErrortype for 0.5, I just need something consistent to work on.