-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
More documentation and traits for Locale
#8
Changes from all commits
83ec654
33eec76
d874371
77f98f6
b3c331d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -491,29 +491,86 @@ impl CodeGenerator { | |
f, | ||
r#" | ||
|
||
/// Locales matching the locales in `glibc`. | ||
/// | ||
/// Most locale names follow the syntax `language[_territory][@modifier]`. | ||
/// The `@` is replaced with `_` in the `enum` variant names. | ||
/// | ||
/// The default locale is `POSIX`. | ||
/// | ||
/// License note: The Free Software Foundation does not claim any copyright interest in the locale | ||
/// data of the GNU C Library; they believe it is not copyrightable. | ||
#[allow(non_camel_case_types,dead_code)] | ||
#[derive(Debug, Copy, Clone, PartialEq)] | ||
#[derive(Copy, Clone, Default, PartialEq, Eq, Hash)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I always wonder what's "default" on an enum like that ^_^ it's like, the first entry in the enum? If that's the case, I'm not sure this is a good idea. Do we really need a default on something like this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, that is tricky. According to https://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap07.html#tag_07_02:
In chrono I added a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be happy with posix |
||
pub enum Locale {{ | ||
"#, | ||
)?; | ||
f.indent(1); | ||
|
||
for (lang, norm) in self.normalized_langs.iter() { | ||
let desc = match self | ||
.by_language | ||
.get(lang) | ||
.and_then(|l| l.get("LC_IDENTIFICATION")) | ||
{ | ||
Some(Category::Fields(fields)) => match fields.get("TITLE") { | ||
Some(Value::Literal(title)) => { | ||
let mut title = title.clone(); | ||
if !title.ends_with('.') { | ||
title.push('.'); | ||
} | ||
title | ||
} | ||
_ => match lang == "POSIX" { | ||
true => "POSIX Standard Locale.".to_string(), | ||
false => "".to_string(), | ||
}, | ||
}, | ||
_ => "".to_string(), | ||
}; | ||
write!(f, "\n/// `{}`: {}\n", lang, desc)?; | ||
if lang == &"POSIX" { | ||
writeln!(f, "\n#[default]\n")?; | ||
} | ||
Comment on lines
+532
to
+534
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh that's the default |
||
writeln!(f, "\n{},\n", norm)?; | ||
} | ||
|
||
f.dedent(1); | ||
write!( | ||
f, | ||
r#" | ||
}} | ||
|
||
impl core::fmt::Display for Locale {{ | ||
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {{ | ||
f.write_str(match self {{ | ||
"#, | ||
)?; | ||
f.indent(3); | ||
|
||
for (lang, norm) in self.normalized_langs.iter() { | ||
write!( | ||
f, | ||
r#" | ||
/// {lang} | ||
{norm}, | ||
Locale::{norm} => {lang:?}, | ||
"#, | ||
lang = lang, | ||
norm = norm, | ||
)?; | ||
} | ||
|
||
f.dedent(1); | ||
f.dedent(3); | ||
write!( | ||
f, | ||
r#" | ||
}}) | ||
}} | ||
}} | ||
|
||
impl core::fmt::Debug for Locale {{ | ||
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {{ | ||
core::fmt::Display::fmt(self, f) | ||
}} | ||
}} | ||
|
||
impl core::str::FromStr for Locale {{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This license note seems quite out of place 😄.
When a someone browses the documentation of an MIT-licensed library and then seeds "glibc" and "GNU" I expect this will be a question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's great to show that we have done our due diligence on this.
Here are the emails I exchanged with them from back in the days:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see the complete context, chronotope/chrono#453 (comment) had the important bit of it 👍.