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

Timezones: Build auto-suggestion from all known timezones. #12

Merged
merged 7 commits into from Jul 2, 2022

Conversation

siedentop
Copy link
Contributor

@siedentop siedentop commented Feb 9, 2022

Proof of concept for #9.

chris@christophs-air rizzy % cargo run -- -z America/Chi
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
     Running `target/debug/rizzy -z America/Chi`
Error: 
   0: Unknown timezone 'America/Chi', did you mean: ["America/Chihuahua", "America/Chicago"]

Location:
   src/main.rs:103

Backtrace omitted.
Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

@siedentop siedentop mentioned this pull request Feb 9, 2022
@siedentop siedentop marked this pull request as ready for review June 20, 2022 21:08
This means we can use the released version of chrono_tz.
@siedentop
Copy link
Contributor Author

@mattgodbolt I just found that the chrono-tz crate already has a way to get the names of all timezones. This means, there's no upstream changes necessary to make this feature work.

IMO, this is ready to merge. For unrelated reasons, I won't be able to work on it from tomorrow onwards. Please feel free to merge, modify or close as suits your project best. Many thanks for letting me work on rizzy a bit!

@mattgodbolt
Copy link
Member

Sorry for the slowness, I'll take a look!

Copy link
Member

@mattgodbolt mattgodbolt left a comment

Choose a reason for hiding this comment

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

LGTM! thanks!

}
} else {
panic!("Failed to open file '{}'", file);
let lines =
Copy link
Member

Choose a reason for hiding this comment

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

TIL!

use super::*;

#[test]
fn test_correct_spelling() {
Copy link
Member

Choose a reason for hiding this comment

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

love it, thanks

.stderr(predicate::str::contains("America/New_York"));
}

/// Make sure the examples from the README work.
Copy link
Member

Choose a reason for hiding this comment

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

Briliant!

tests/cli.rs Show resolved Hide resolved
@mattgodbolt mattgodbolt merged commit 245f1d8 into aquanauts:main Jul 2, 2022
@siedentop siedentop deleted the cs/tz-suggest branch August 30, 2022 15:54
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.

None yet

2 participants