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

Invalid datetime format for korean(ko_KR) #985

Closed
scarf005 opened this issue Mar 9, 2023 · 5 comments
Closed

Invalid datetime format for korean(ko_KR) #985

scarf005 opened this issue Mar 9, 2023 · 5 comments

Comments

@scarf005
Copy link
Contributor

scarf005 commented Mar 9, 2023

Summary

South korea uses Year-Month-Day date format like rest of east asia. however, it's invalidly rendered as Month-Day-Year with %c.

$ date '+%c
2023년 03월 09일 (목) 오전 11시 59분 40초
$ cargo run # chrono +%c
03/09/23 (목) 11:59:36 오전

To reproduce

(tested in 0.4.23)

use chrono::{Local, Locale};

fn main() {
    let time = Local::now();
    let display = |fmt: &str| {
        let localized = time.format_localized(fmt, Locale::ko_KR);
        println!("{fmt}: {localized}");
    };
    display("%c");
    display("%r");
}
%c: 03/09/23 (목) 11:59:36 오전
%r: 11:59:36 오전
 ~/repo   …  locale  date '+%c'
2023년 03월 09일 (목) 오전 11시 59분 40초
 ~/repo   …  locale  date '+%r'
오전 11시 59분 42초
@djc
Copy link
Member

djc commented Mar 9, 2023

I think you'll need to file an issue against https://github.com/cecton/pure-rust-locales.

@scarf005
Copy link
Contributor Author

scarf005 commented Mar 9, 2023

@djc i've looked at pure-rust-locales, it defines korean datetime format as "%x (%a) %r".

#[test]
fn locale_match_ko_kr() {
    let locale = "ko_KR".try_into().unwrap();
    let d_fmt = pure_rust_locales::locale_match!(locale => LC_TIME::D_FMT);
    assert_eq!(d_fmt, "%Y년 %m월 %d일");

    let d_t_fmt = pure_rust_locales::locale_match!(locale => LC_TIME::D_T_FMT);
    assert_eq!(d_t_fmt, "%x (%a) %r");
}

i've modified strftime.rs to debug how d_t_fmt was used as following:

    pub fn new_with_locale(s: &'a str, locale: Locale) -> StrftimeItems<'a> {
		// ...
        let d_t_fmt = StrftimeItems::new(locales::d_t_fmt(locale)).collect();
        println!("locales: {:?}", locales::d_t_fmt(locale));
        println!("d_t_fmt: {:?}", d_t_fmt);
		// ...
    }
/*
locales: "%x (%a) %r"
d_t_fmt: [Numeric(Month, Zero), Literal("/"), Numeric(Day, Zero), Literal("/"), Numeric(YearMod100, Zero), Space(" "), Literal("("), Fixed(ShortWeekdayName), Literal(")"), Space(" "), Numeric(Hour12, Zero), Literal(":"), Numeric(Minute, Zero), Literal(":"), Numeric(Second, Zero), Space(" "), Fixed(UpperAmPm)]
thread 'format::strftime::test_strftime_localized_korean' panicked at 'assertion failed: `(left == right)`
*/

does parsing %x take locale into account? because

assert_eq!(dt.format_localized("%x", Locale::ko_KR).to_string(), "2001년 07월 08일");

will pass.

@djc
Copy link
Member

djc commented Mar 9, 2023

Maybe it doesn't? Unfortunately I don't have much time to investigate right now. Would be happy to review a PR if you're able to figure it out!

@scarf005
Copy link
Contributor Author

scarf005 commented Mar 9, 2023

fn with_remainer(s: &'a str) -> StrftimeItems<'a> {
StrftimeItems {
remainder: s,
recons: Vec::new(),
d_fmt: D_FMT.to_vec(),
d_t_fmt: D_T_FMT.to_vec(),
t_fmt: T_FMT.to_vec(),
}
}
}

alright, i'm investigating, and i think i found the cause: with_remainer does not take d_fmt into account. d_t_fmt in ko_KR depends on d_fmt, so it was failing.

for verification, when i changed with_remainers into this, test passes:

    #[cfg(feature = "unstable-locales")]
    fn with_remainer(s: &'a str) -> StrftimeItems<'a> {
        println!("with remainer s: {:?}", s);
        StrftimeItems {
            remainder: s,
            recons: Vec::new(),
            // d_fmt: D_FMT.to_vec(),
            d_fmt: StrftimeItems::new(locales::d_fmt(Locale::ko_KR)).collect(),
            d_t_fmt: D_T_FMT.to_vec(),
            t_fmt: T_FMT.to_vec(),
        }
    }

@pitdicker
Copy link
Collaborator

Fixed in #1165.

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 a pull request may close this issue.

3 participants