chronotope / chrono-tz

TimeZone implementations for rust-chrono from the IANA database
Other
229 stars 55 forks source link

Case insensitive timezone matching #125

Open IanWorthington opened 1 year ago

IanWorthington commented 1 year ago

I read a timezone from the command line into str and parse it using:

let ps: Result<Tz, String> = str.parse();

This works fine when I specify the timezone as CET or Europe/Paris, for example, but fails if I don't match the case correctly.

This is slightly irritating from a user's perspective. Is there any way to perform a case insensitive matching?

esheppa commented 1 year ago

Thanks @IanWorthington

We are using the phf library to store the tzdb data in the library, so this may not be trivial as we need to ensure that it works both with the correct casing and other casing, but we can only store one version as the key

One thing for us to consider is that using the to_lowercase functions allocates, which may not be ideal from a performance perspective, but we could investigate using make_ascii_lowercase to resolve this.

If you have time we would greatly appreciate a PR to resolve this

IanWorthington commented 1 year ago

Thanks for your response @esheppa.

I am very much a neophyte Rust programmer, but I'm happy to take a look.

The first thing that occurs to me though is to ask if you /really/ would want to fail a lookup simply on a case mismatch? Are there (or could there ever be) entries that differ only by case?

djc commented 1 year ago

Note that we do have a case-insensitive Cargo feature, so it looks like there are some facilities for case-insensitive timezone matching. Among other things, this passes an uncased feature flag to phf, so something is happening there. I couldn't quite figure out how this is exposed as part of the chrono-tz API, though.

briansorahan commented 10 months ago

I am working on an API (in rust of course) that has a column iana_timezone which sometimes has the value Etc/Gmt+5 (valid value according to pytz). I've noticed that this library crashes when I try to parse this (using the FromStr impl for Tz). I tried running my test program with the case-insensitive feature turned on, but still got the same error

Etc/Gmt+5' is not a valid timezone

I changed my test string to Etc/GMT+5 and it worked FYI. Either the case-insensitive thing isn't working or I'm doing something wrong. Maybe it's not as simple as just enabling the feature? Is there a different FromStr method that would need to get invoked?

Thanks in advance!

Here is my Cargo.toml

[package]
name = "timezones"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
chrono-tz = { version = "0.8.3", features = ["case-insensitive"] }

and my test program

fn main() {
    let iana_timezone = String::from("Etc/Gmt+5");
    let tz: &chrono_tz::Tz = &iana_timezone.parse().unwrap();
    println!("tz = {:?}", tz);
}
djc commented 10 months ago

Yes, it looks like you have to use chrono_tz::from_str_insensitive(). Unfortunately because this API is added by the build script depending on the Cargo features, it doesn't make it into the crate docs on docs.rs...

If you have any suggestions for how to improve the documentation around this, please submit a PR.