GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Add localeconv function to locale module by minhrongcon2000 · Pull Request #4558 · RustPython/RustPython Conversation
|
| #[pyattr] | ||
| use libc::{ | ||
| ABDAY_1, ABDAY_2, ABDAY_3, ABDAY_4, ABDAY_5, ABDAY_6, ABDAY_7, ABMON_1, ABMON_10, ABMON_11, | ||
| ABMON_12, ABMON_2, ABMON_3, ABMON_4, ABMON_5, ABMON_6, ABMON_7, ABMON_8, ABMON_9, | ||
| ALT_DIGITS, AM_STR, CODESET, CRNCYSTR, DAY_1, DAY_2, DAY_3, DAY_4, DAY_5, DAY_6, DAY_7, | ||
| D_FMT, D_T_FMT, ERA, ERA_D_FMT, ERA_D_T_FMT, ERA_T_FMT, LC_ALL, LC_COLLATE, LC_CTYPE, | ||
| LC_MESSAGES, LC_MONETARY, LC_NUMERIC, LC_TIME, MON_1, MON_10, MON_11, MON_12, MON_2, MON_3, | ||
| MON_4, MON_5, MON_6, MON_7, MON_8, MON_9, NOEXPR, PM_STR, RADIXCHAR, THOUSEP, T_FMT, | ||
| T_FMT_AMPM, YESEXPR, | ||
| }; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Not all of these constants are available on every platform. When I encountered this sort of problem in the past, I looked up each item individually in the libc repository to see which platforms support that item. (See #3840, for example.)
I realized back then (and I still maintain) that this process would prove to be tedious and error-prone. Perhaps I should look into using the autocfg crate instead, but that's ultimately beyond the scope of this PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I tried with Window locale.h this afternoon and found out that Window API uses different naming convention (prefix with W, if I remember). Hence, if we would like to fix it, I'm afraid that we need to implement our own locale interaction. I will do it in another PR once this module takes shape.
Right now, I put this module limit to unix system only!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…localeconv-function
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The windows failure is fine. But test_type failure on linux is... what's happened |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
https://github.com/RustPython/RustPython/actions/runs/4271112901/jobs/7435359720#step:12:12102 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@youknowone it turns out the locale code is inconsistent between platform, so after some fixes, it's normal now! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
77af913 to
26f04e2
Compare
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| let result = match args.locale.flatten() { | ||
| None => libc::setlocale(args.category, ptr::null()), | ||
| Some(l) => { | ||
| let l_str = CString::new(l.to_string()).expect("expect to be always converted"); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
this is not always successful
>>> _locale.setlocale(0, '\0')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: embedded null character
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| None => libc::setlocale(args.category, ptr::null()), | ||
| Some(l) => { | ||
| let l_str = CString::new(l.to_string()).expect("expect to be always converted"); | ||
| let l_ptr = CStr::as_ptr(&l_str); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
because CString implements std::ops::Deref<Target=CStr>, this is redundant.
libc::setlocale(args.category, lstr.as_ptr())
works exactly same way.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thank you for hard working for this tricky issue. It will be a great base of the future locale works.
Maintaining note: Though a few tests regressed, I think this is the way to go forward with true locale support.
squash is desired when merging.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.