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

no excessive highlighting #263

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

Johann150
Copy link
Contributor

This should resolve #261.

Is this just okay like this, or should this be configurable?

@brendanzab
Copy link
Owner

Sorry for my slow review on this - need to figure out whether I'm happy with how we're iterating over the characters of a source line.

Comment on lines +247 to +253
let text_end = source
.char_indices()
.rev()
.take_while(|(_, c)| c.is_whitespace())
.last()
.map(|(index, _)| index)
.unwrap_or_else(|| source.len());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be rewritten using str::rfind, but because we have to find the first trailing non-whitespace character, we would have to find the char lengt of the character to adjust, which makes it not much clearer than the curren solution.

Suggested change
let text_end = source
.char_indices()
.rev()
.take_while(|(_, c)| c.is_whitespace())
.last()
.map(|(index, _)| index)
.unwrap_or_else(|| source.len());
let text_end = source
.rfind(|c| !char::is_whitespace(c))
.map(|index|
index + source[index..]
.chars()
.next()
.map_or(0, |c| c.len_utf8())
)
.unwrap_or_else(|| source.len());

@Johann150
Copy link
Contributor Author

@brendanzab any feedback on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants