Merged #14
Merged
Merged #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The equals signs are included in the day-of-transition capture sign.
Not all time specs need to have a time type associated with them - only one particular kind. This commit mandates that, by removing the type from the struct. Also, rename RulesSave to Saving, which is what it's being called everywhere else.
This code uses an algorithm cribbed from the C examples in the tzinfo database itself, so they're actually more correct than whatever I was using! - Rename GMT to UTC wherever it's used. It means pretty much the same, but is more modern (I hope this commit message isn't used to make fun of me in the future) - Use the Time Type enum in datetime - there's no point having two of them. - Add a lookup function in the data crate.
This introduces a Format enum, which parses the formats beforehand, and figures out which to use based on that. It still panicks, though.
This makes it easier to test whether things have changed between crate compilations.
Add a test for the transition-optimisation function, which uses some of the Antarctica/Macquarie rules. However, fix all the other tests to get it to compile at all! - I had to replace some of the end times (123456 and 234567) with their ZoneTime equivalent. - This required some of the fields for the spec structs in line.rs to be made public.
This makes the code more Rustful, and has no changes to the resulting output.
This makes it compile without warnings again!
The error in question appears to have gone away! Current version: rustc 1.5.0-nightly (95fb8d1c8 2015-10-27)
Commit 2551f9224a1c188cc7f963f504b27c8cea4d92ae to datetime changes the Transition struct to only have one offset field, rather than two. This commit changes the data crate builder to use the new version of the struct. Also, work around a linker error that I'm not even sure why I'm getting.
This makes it easier to see how the consolidated offset was produced.
This marks a change to the way transition data was calculated and stored. Previously, each timespan—a period of consistent time zone name, GMT offset, and DST offset—was stored as a transition with an optional timestamp at which the transition occurs. This doesn’t solve the problem of what the time details should be *before* the first transition has occurred, which is why the timestamp was optional: the first period would simply have a `None` timestamp. This approach had problems, namely: 1. It was possible to have multiple `None` timestamps, which would confuse the datetime calculator; 2. It was not only possible but actually manifest to have time zones with *zero* `None` timestamps, which would also confuse the datetime calculator. To remove the possibility of having invalid data, each time zone now has one fixed “ante” period, and at least zero transitions, each of which now has a non-Optional timestamp. It’s a rather simple change, but most of the terminology has been updated to reflect this—Transition to ZoneDetails, among other structs—which is where most of the lines in the diff come from. The only time zones that change in zoneinfo-data are the “fixed” ones like CET or EST5EDT.
This uses the PartialEq implementation on ZoneDetails to simplify some of the code, which was getting rather repetitive.
build-data-crate.rs now uses the new ZoneSet instead of Transition, which is great because Transition doesn't exist anymore. See 74f69f6 for more information on why this is necessary.
ZoneDetails to FixedTimespan, and ZoneSet to FixedTimespanSet. Also, add the transition instant time for clarity.
Many types have been moved to the root of the crate, with others (such as TimeType) accessible from a different module now. See datetime@b86a5752678d2cf410b37c909309138895a61f8a for the commit that necessitated. this.
The ‘get’ function (that *will* be in the data crate’s root) was confused over the elided lifetimes of the ‘lookup’ function (in the data module). Adding an explicit 'static fixes this.
- Update nomenclature in test code (FixedTimespan, FixedTimespanSet, no more Transition) - Add local mean times to the rules tests to make them stop failing (it’ll never be valid to have a time zone that *begins* with a rule—it should always have a local mean time first!) - Removed the only feature flag to remove dependence on Rust Nightly
|
I will push this without relying on the web UI so we can get a clean merge commit in the commit history. Then we can work from there -- I suggest we tackle chronotope/chrono-tz#166 next. |
|
We seem to have done almost the same thing. How did you do the merge? |
|
pitdicker
reviewed
Apr 15, 2024
|
Ah, you went for a proper merge! |
|
Maybe compare with my commit in #12 to port the documentation to avoid more duplicate work? |
pitdicker
reviewed
Apr 15, 2024
| @@ -948,7 +1024,7 @@ impl LineParser { | |||
| }) | |||
| } | |||
|
|
|||
| fn parse_zone<'a>(&self, input: &'a str) -> Result<Zone<'a>, Error> { | |||
| pub fn parse_zone<'a>(&self, input: &'a str) -> Result<Zone<'a>, Error> { | |||
There was a problem hiding this comment.
Why did this method and parse_link become public?
pitdicker
reviewed
Apr 15, 2024
| //! | ||
| //! let parser = LineParser::new(); | ||
| //! let zone = parser.parse_zone("Zone Pacific/Auckland 11:39:04 - LMT 1868 Nov 2").unwrap(); | ||
| //! let link = parser.parse_link("Link Pacific/Auckland Antarctica/McMurdo").unwrap(); |
There was a problem hiding this comment.
If it is because of this doctest: I changed it to show how parse-zoneinfo is expected to be used in 536ac28.
There was a problem hiding this comment.
It is because of that doctest. I want to keep this merge commit as small as possible -- the doctest is what it is, and then in future commits/PRs we can clean all this stuff up.
Merged the original history.
|
Brought over more of the docstrings. I think we should merge this and then work from there. |
|
Compliments on the merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
parse-zoneinfo was originally forked from zoneinfo-parse in 2017 (presumably at the latest released version, rust-datetime/zoneinfo-parse@d533d0a). This brings the original history back into this repo and merges both branches (zoneinfo-parse had some additional changes after the fork).