Flexible offset formatting [1/3] #1082
Conversation
pitdicker
force-pushed
the
offset_formatting_1
branch
3 times, most recently
from
10e49a9
to
df6a2d0
Compare
pitdicker
force-pushed
the
offset_formatting_1
branch
from
df6a2d0
to
492c83c
Compare
src/format/mod.rs
Outdated
| } else { | ||
| locale.am_pm[0] | ||
| }); | ||
| let ret = match *spec { |
There was a problem hiding this comment.
To make it easier for you to see what is changed, and what is caused by rustfmt.
There was a problem hiding this comment.
So this is old code that was previously not formatted? What changed that rustfmt is now formatting it?
There was a problem hiding this comment.
I did some experiments at the time, but couldn't pin it down. In #1126 (comment) I run into the same issue.
pitdicker
force-pushed
the
offset_formatting_1
branch
3 times, most recently
from
e687926
to
d27793b
Compare
pitdicker
force-pushed
the
offset_formatting_1
branch
7 times, most recently
from
f84e948
to
fd0f41b
Compare
src/format/formatting.rs
Outdated
| @@ -23,6 +23,8 @@ use crate::{Datelike, Timelike, Weekday}; | |||
| #[cfg(feature = "unstable-locales")] | |||
| use super::locales; | |||
| #[cfg(any(feature = "alloc", feature = "std"))] | |||
| use super::{Colons, OffsetFormat, OffsetPrecision}; | |||
src/format/formatting.rs
Outdated
| off.map(|&(_, off)| write_local_minus_utc(result, off, true, Colons::None)) | ||
| } | ||
| TimezoneOffset | TimezoneOffsetZ => off.map(|&(_, off)| { | ||
| let allow_zulu = *spec == TimezoneOffsetZ; |
There was a problem hiding this comment.
I'd prefer inlining this value in the OffsetFormat::new() call.
src/format/formatting.rs
Outdated
| .format(result, off) | ||
| }), | ||
| TimezoneOffsetColon | TimezoneOffsetColonZ => off.map(|&(_, off)| { | ||
| let allow_zulu = *spec == TimezoneOffsetColonZ; |
src/format/mod.rs
Outdated
|
|
||
| impl OffsetFormat { | ||
| /// Create a new `OffsetFormat`. | ||
| pub const fn new( |
There was a problem hiding this comment.
Given that the new() calls will still be spread over the same number of lines in practice, I feel like this method isn't really buying us anything, whereas having the field names in the initializer would make the code a little easier to understand (particularly for the allow_zulu field).
src/format/mod.rs
Outdated
| pub struct OffsetFormat { | ||
| /// See `OffsetPrecision`. | ||
| pub precision: OffsetPrecision, | ||
| /// Seperator between hours, minutes and seconds. |
There was a problem hiding this comment.
Thank you for being thorough, I thought I had them all.
pitdicker
force-pushed
the
offset_formatting_1
branch
from
fd0f41b
to
2cce338
Compare
pitdicker
force-pushed
the
offset_formatting_1
branch
from
2cce338
to
edf413d
Compare
There are a lot of issues with our current code to parse time zone offsets. For example
%z,%:z,%::zand%:::zare all treated identical in the parser. In some casesZis accepted, and there is a hack to acceptUTC. There is no way to parse seconds.Formatting is in a better state, but it would be nice if it could be more flexible. For example to be able to print seconds optionally, support padding, or print
Z.This is part 1 of a series of PRs to improve offset formatting and parsing. (1) adds a flexible formatter, (2) adds a flexible parser and fixes current parsing bugs, and (3) exposes the functionality in the format string.
With this PR it is now possible to specify the format of an offset:
Zused for+00:00?