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

Make TIMEZONES map public, so that users may iterate over all timezones. #95

Closed
wants to merge 2 commits into from

Conversation

siedentop
Copy link

@siedentop siedentop commented Feb 9, 2022

I could imagine, that this PR is not implemented in the way you'd prefer. I mostly want to get the conversation started and offer one possible implementation.

For a feature, I thought it would be nice to create an auto-correct dictionary for all known timezones.

So, in this PR I am changing static TIMEZONES to pub static TIMEZONES. On the user side, I am simply creating a list off all timezones like this: chrono_tz::TIMEZONES.keys().map(|x| x.to_string()).collect().

Is this something you would be willing to support? Would you prefer a new function that hid the access to TIMEZONES behind a function?


Pinging previous authors of the modified file: @quodlibetor @petrosagg

@siedentop siedentop marked this pull request as draft February 9, 2022 02:54
@ritchie46
Copy link

This might help.

let timezone = "Africa/Asmara";
timezone.parse::<chrono_tz::Tz>()

@djc
Copy link
Member

djc commented May 11, 2022

I'm open to having this, but I'd like to have TIMEZONES be renamed to TIME_ZONES (in a separate commit, please).

@siedentop
Copy link
Author

siedentop commented May 20, 2022

@djc like this, or renaming it everywhere? pub use timezones::TIMEZONES as TIME_ZONES;

EDIT: I just did the right thing, and renamed it everywhere.

siedentop added a commit to siedentop/chrono-tz that referenced this pull request May 20, 2022
@siedentop siedentop marked this pull request as ready for review May 20, 2022 23:18
@djc
Copy link
Member

djc commented May 23, 2022

Thanks for the rename! I'm having some second thoughts as this makes phf part of the public API. How about we wrap the phf::Map in a newtype wrapper that exposes a minimal API (maybe just get() and iter() for now)?

@siedentop
Copy link
Author

Sorry for the delay. I've just discovered that TZ_VARIANTS is already public and exported.

Here's what I want to do: chrono_tz::TZ_VARIANTS.iter().map(|x| x.to_string())

So, let's close this PR.

@siedentop siedentop closed this Jun 20, 2022
@siedentop siedentop deleted the pub-timezones branch June 21, 2022 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants