&str.words should be using the unicode word boundary algorithm

[huonw]

Currently it's just splitting on whitespace (i.e. is equivalent to splitting with regex!("\s+")). Preferably it should be using http://www.unicode.org/reports/tr29/#Word_Boundaries

(The old behaviour is easy to replicate with the above regex, or with s.split(|c: char| c.is_whitespace()).filter(|s| !s.is_empty()).)

[kwantam]

Note that this gives some possibly unintuitive results.

("Hello there") gives ( " Hello there " ), which might not be what one expects.

[kwantam]

It occurs to me that we could take a bool indicating whether the user wants the full UAX#29 algorithm or the simple one. A lot of other languages say words() when they mean "split on whitespace", and it's a trivial thing to continue offering the present behavior even after implementing the UAX#29 algorithm.

[lucy]

I think taking a bool for this is bad design. It should be a separate method.

[mahkoh]

This has to be decided before 1.0.

[mahkoh]

Here is the implementation I've just written for a program I'm working on:

    fn word_at(&self, idx: usize) -> &str {
        let s = &self[idx..];
        let mut take_prev = true;
        let mut take_cur = true;
        let mut state = WState::Start;
        let mut prev_idx = 0;
        let mut idx = 0;

        for (curr, c) in s.char_indices() {
            prev_idx = idx;
            idx = curr;

            let cat = word::category(c);
            if cat == WCat::Extend || cat == WCat::Format {
                if state == WState::Start {
                    break;
                } else {
                    continue;
                }
            }

            state = match state {
                WState::Start if c == '\r' => {
                    if idx + 1 != s.len() && s.char_at(idx + 1) == '\n' {
                        idx += 1;
                    }
                    break;
                },
                WState::Start => match cat {
                    WCat::ALetter           => WState::ALetter,
                    WCat::HebrewLetter      => WState::HLetter,
                    WCat::Numeric           => WState::Numeric,
                    WCat::Katakana          => WState::Katakana,
                    WCat::ExtendNumLet      => WState::ExtendNumLet,
                    WCat::RegionalIndicator => WState::RegionalIndicator,
                    _ => break,
                },
                WState::ALetter => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::MidLetter | WCat::MidNumLet |
                        WCat::SingleQuote => WState::ALetterExt,
                    WCat::Numeric => WState::Numeric,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::ALetterExt => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    _ => {
                        take_cur = false;
                        take_prev = false;
                        break;
                    }
                },
                WState::HLetter => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::MidLetter | WCat::MidNumLet => WState::HLetterExt(0),
                    WCat::SingleQuote => WState::HLetterExt(1),
                    WCat::DoubleQuote => WState::HLetterExt(2),
                    WCat::Numeric => WState::Numeric,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::HLetterExt(n) => match cat {
                    WCat::ALetter if n < 2 => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    _ => {
                        take_cur = false;
                        take_prev = n == 1;
                        break;
                    }
                },
                WState::Numeric => match cat {
                    WCat::Numeric => WState::Numeric,
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::MidNum | WCat::MidNumLet |
                        WCat::SingleQuote => WState::NumericExt,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::NumericExt => match cat {
                    WCat::Numeric => WState::Numeric,
                    _ => {
                        take_cur = false;
                        take_prev = false;
                        break;
                    }
                },
                WState::Katakana => match cat {
                    WCat::Katakana => WState::Katakana,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::ExtendNumLet => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::Numeric => WState::Numeric,
                    WCat::Katakana => WState::Katakana,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::RegionalIndicator => match cat {
                    WCat::RegionalIndicator => WState::RegionalIndicator,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::SingleQuote => unreachable!(),
            }
        }

        if take_cur {
            idx += s.char_at(idx).len_utf8();
        } else if !take_prev {
            idx = prev_idx;
        }

        &s[..idx]
    }

    fn word_at_reverse(&self, idx: usize) -> &str {
        let s = &self[..idx];

        let mut take_prev = true;
        let mut take_cur = true;
        let mut state = WState::Start;
        let mut prev_prev_idx = s.len();
        let mut prev_idx = s.len();
        let mut idx = s.len();

        for (curr, c) in s.char_indices().rev() {
            let cat = word::category(c);
            if cat == WCat::Extend || cat == WCat::Format {
                continue;
            }

            prev_prev_idx = prev_idx;
            prev_idx = idx;
            idx = curr;

            state = match state {
                WState::Start if c == '\n' => {
                    if idx > 0 && s.char_at_reverse(idx) == '\r' {
                        idx -= 1;
                    }
                    break;
                },
                WState::Start => match cat {
                    WCat::ALetter           => WState::ALetter,
                    WCat::HebrewLetter      => WState::HLetter,
                    WCat::SingleQuote       => WState::SingleQuote,
                    WCat::Numeric           => WState::Numeric,
                    WCat::Katakana          => WState::Katakana,
                    WCat::ExtendNumLet      => WState::ExtendNumLet,
                    WCat::RegionalIndicator => WState::RegionalIndicator,
                    _ => break,
                },
                WState::ALetter => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::MidLetter | WCat::MidNumLet |
                        WCat::SingleQuote => WState::ALetterExt,
                    WCat::Numeric => WState::Numeric,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::ALetterExt => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    _ => {
                        take_cur = false;
                        take_prev = false;
                        break;
                    }
                },
                WState::HLetter => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::MidLetter | WCat::MidNumLet => WState::HLetterExt(0),
                    WCat::DoubleQuote => WState::HLetterExt(2),
                    WCat::Numeric => WState::Numeric,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::HLetterExt(n) => match cat {
                    WCat::ALetter if n < 2 => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    _ => {
                        take_cur = false;
                        take_prev = false;
                        break;
                    }
                },
                WState::SingleQuote => match cat {
                    WCat::HebrewLetter => WState::HLetter,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::Numeric => match cat {
                    WCat::Numeric => WState::Numeric,
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::MidNum | WCat::MidNumLet |
                        WCat::SingleQuote => WState::NumericExt,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::NumericExt => match cat {
                    WCat::Numeric => WState::Numeric,
                    _ => {
                        take_cur = false;
                        take_prev = false;
                        break;
                    }
                },
                WState::Katakana => match cat {
                    WCat::Katakana => WState::Katakana,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::ExtendNumLet => match cat {
                    WCat::ALetter => WState::ALetter,
                    WCat::HebrewLetter => WState::HLetter,
                    WCat::Numeric => WState::Numeric,
                    WCat::Katakana => WState::Katakana,
                    WCat::ExtendNumLet => WState::ExtendNumLet,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
                WState::RegionalIndicator => match cat {
                    WCat::RegionalIndicator => WState::RegionalIndicator,
                    _ => {
                        take_cur = false;
                        break;
                    }
                },
            }
        }

        if idx == s.len() {
            idx = 0;
        } else if !take_prev {
            idx = prev_prev_idx;
        } else if !take_cur {
            idx = prev_idx;
        }

        &s[idx..]
    }

[kwantam]

It's still not clear to me that we want "words" to be UAX#29 vs "normal" splitting. Most people expect to break on whitespace based on their experiences in other languages. That's not a reason to do or not do it; I'm just pointing out that some thought would be good here.

I'm convinced that it's worthwhile to have a UAX#29 implementation available in the library, since there is good reason to have strong Unicode support. I'm just unclear on how the naming should be handled.

[ColonelJ]

I think words should stay as it is. In a language with overloading or optional arguments having an extra argument to specify this as an alternate strategy would be nice, but we do not have this luxury.

This seems like a nice function though. I suppose it's quite specific so it could have a longer name... I can give some suggestions: words_by_boundary (my preference), unicode_words, words_unicode, words_proper, word_extract, sel_words, extract_words, word_break, delimited_words, bounded_words, word_divide, word_divisions, word_intervals, word_segments, just segments or word_segs, word_sep, unicode_word_split, uax29_words, words_precise...

And if you wanted to be ultra precise/unambiguous/mean you could remove words and force people to use words_by_space or words_by_boundary!

[mahkoh]

which might not be what one expects

One might want to read the documentation then.

That's not a reason to do or not do it; I'm just pointing out that some thought would be good here.

It's clear that no thought went into the current implementation. A function that uses a degenerate algorithm which barely finds words in ascii does not belong in a unicode library. I'm not sure what purpose this algorithm serves at all. Chrome certainly doesn't use it to find word boundaries and neither does vim. If someone actually needs this behavior then they can get it with the one-liner in the first post.

[kwantam]

This is a fair observation.

In that case, probably we should

• use your UAXA#29 implementation for words()
• include something like the one-liner @huonw posted above in the words() documentation (along with some examples and a pointer to the UAX#29 document, of course)

This works for me if it works for you.

From a practical standpoint: are you interested in putting together a pull request to integrate the above into libunicode? If not, I can probably find some time to do it over the weekend.

[mahkoh]

I don't know python so I won't do it. Besides, the current python implementation seems bad because it doesn't use the normative file for the grapheme classes. Also, the names of the functions should reflect what they are actually doing.