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

Case insensitive timezone matching #125

Open
IanWorthington opened this issue Feb 24, 2023 · 5 comments
Open

Case insensitive timezone matching #125

IanWorthington opened this issue Feb 24, 2023 · 5 comments

Comments

@IanWorthington
Copy link

IanWorthington commented Feb 24, 2023

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 esheppa transferred this issue from chronotope/chrono Feb 25, 2023
@esheppa
Copy link

esheppa commented Feb 25, 2023

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
Copy link
Author

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
Copy link
Member

djc commented Feb 27, 2023

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
Copy link

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
Copy link
Member

djc commented Aug 29, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants