Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse-zoneinfo: replace rule parser with simple state machine #172

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion chrono-tz-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ case-insensitive = ["uncased", "phf/uncased"]
regex = ["dep:regex"]

[dependencies]
parse-zoneinfo = { version = "0.3" }
parse-zoneinfo = { version = "0.3", path = "../parse-zoneinfo" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking to make these changes in my next PR 👍.

regex = { default-features = false, version = "1", optional = true }
phf = { version = "0.11", default-features = false }
phf_codegen = { version = "0.11", default-features = false }
Expand Down
22 changes: 5 additions & 17 deletions chrono-tz-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,26 +509,14 @@ fn detect_iana_db_version() -> String {
pub fn main() {
println!("cargo:rerun-if-env-changed={}", FILTER_ENV_VAR_NAME);

let parser = LineParser::new();
let parser = LineParser::default();
let mut table = TableBuilder::new();

let tzfiles = [
"tz/africa",
"tz/antarctica",
"tz/asia",
"tz/australasia",
"tz/backward",
"tz/etcetera",
"tz/europe",
"tz/northamerica",
"tz/southamerica",
];

let lines = tzfiles
let lines = parse_zoneinfo::FILES
.iter()
.map(Path::new)
.map(|p| {
Path::new(&env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| String::new())).join(p)
Path::new(&env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| String::new()))
.join("tz")
.join(p)
})
.map(|path| {
File::open(&path).unwrap_or_else(|e| panic!("cannot open {}: {}", path.display(), e))
Expand Down
6 changes: 2 additions & 4 deletions parse-zoneinfo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,5 @@ readme = "README.md"
license = "MIT"
keywords = ["date", "time", "timezone", "zone", "calendar"]

[dependencies.regex]
version = "1.3.1"
default-features = false
features = ["std", "unicode-perl"]
[dev-dependencies]
insta = "1.38"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why you added this test. Not sure about it though.

Would it be better to add this test as a separate crate in the workspace?

12 changes: 12 additions & 0 deletions parse-zoneinfo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,15 @@ pub mod line;
pub mod structure;
pub mod table;
pub mod transitions;

pub const FILES: &[&str] = &[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure we want to hardcode this list in parse-zoneinfo.

For my personal experiments the past year I removed backward, included backzone, occasionally included factory, and filtered parts of etcetera.

Maybe move the change out of this PR so we can discuss it separately?

"africa",
"antarctica",
"asia",
"australasia",
"backward",
"etcetera",
"europe",
"northamerica",
"southamerica",
];
Loading
Loading