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

Suggestion about is_valid function of LineBuffer struct #760

Open
Laurent45 opened this issue Feb 27, 2024 · 1 comment
Open

Suggestion about is_valid function of LineBuffer struct #760

Laurent45 opened this issue Feb 27, 2024 · 1 comment

Comments

@Laurent45
Copy link

Laurent45 commented Feb 27, 2024

A little suggestion. It seems like first check and second check do the same check implicitly.
Is there a case where the first check is false and the second true ? What do you think ?

pub fn is_valid(&self) -> bool {
        self.lines.is_char_boundary(self.insertion_point())  // first check
            && (self
                .lines
                .grapheme_indices(true)
                .any(|(i, _)| i == self.insertion_point())
                || self.insertion_point() == self.lines.len())    // second check
            && std::str::from_utf8(self.lines.as_bytes()).is_ok()
    }
@sholderbach
Copy link
Member

You are correct that if the second test (grapheme cluster boundary) is true the first one (UTF-8 char boundary) will always be true.

The first check is local and only needs to check the byte at the index (or length), while the follow-on second check requires us to traverse the string. Without benchmarking with real data (that is most likely valid), there could be a questionable argument about short-circuiting saving us the work for the second check. Given that we expect valid data anyways, we could probably elide the first check.

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

No branches or pull requests

2 participants