fix: use ideographic spaces for Japanese mnemonics#90
Conversation
elsirion
left a comment
elsirion
left a comment
There was a problem hiding this comment.
I think the two problems worth discussing here:
- How does it fit into our
no_stdsupport story - Is the additional complexity warranted
I think if we just generally deal with japanese whitespaces this might be a good trade-off. But I'll leave that to you. Pinging @uncomputable as a knower of rust and the japanese language 😆
| pub fn separator(self) -> &'static str { | ||
| match self { | ||
| #[cfg(feature = "japanese")] | ||
| Language::Japanese => " ", // U+3000 ideographic space |
There was a problem hiding this comment.
Could you please make this an escape string so it's easier to review if it is the correct space please?
| /// Split mnemonic into words using language-appropriate separators. | ||
| /// Japanese uses ideographic spaces (U+3000), others use standard whitespace. | ||
| pub fn split_mnemonic<'a>(self, mnemonic: &'a str) -> Box<dyn Iterator<Item = &'a str> + 'a> { | ||
| match self { | ||
| #[cfg(feature = "japanese")] | ||
| Language::Japanese => { | ||
| // For Japanese, only split on ideographic spaces (U+3000) | ||
| Box::new(mnemonic.split(' ').filter(|s| !s.is_empty())) | ||
| } | ||
| _ => { | ||
| Box::new(mnemonic.split_whitespace()) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This only works with the alloc feature, should be gated
There was a problem hiding this comment.
Maybe, instead of doing it this way, always return a struct like:
struct SplitMnemonic<'a> {
parts: [&'a str; 24],
len: usize,
}And implement Iterator for that. So you could collect into the struct, it has constant size without allocation and you can apply different splitting algorithms. Alternatively, idk if it would be the end of the world to just always also split on japaneses whitespaces.
| Mnemonic::language_of_iter(mnemonic.as_ref().split_whitespace()) | ||
| // Split on both standard whitespace and ideographic space to handle Japanese | ||
| let words: Vec<&str> = mnemonic.as_ref() | ||
| .split(|c: char| c.is_whitespace() || c == ' ') |
There was a problem hiding this comment.
Worth making the special space a cosntant
|
I can confirm that ideographic spaces significantly increase readability of Japanese BIP 39 mnemonics. (I also think that Japanese mnemonics are reckless to use, but that is not the topic of this PR.)
The spec seems to enforce ideographic spaces except for rare cases, so implementing this here seems worthwhile. |
3 participants
Hi, I noticed that when generating a Japanese mnemonic, the output sentence does not use ideographic spaces, and the library is unable to parse it correctly while they are recommended in bip39 Japanese section
I've also added some test vectors used for Japanese mnemonics