expand: address a cognitive_complexity warnings #9930
Conversation
CodSpeed Performance ReportMerging #9930 will degrade performance by 3.05%Comparing Summary
Benchmarks breakdown
Footnotes
|
sylvestre
force-pushed
the
expand-cognitive_complexity
branch
from
969f0e0 to
14e2b56
Compare
| /// | ||
| /// Returns `(CharType, display_width, byte_length)`. | ||
| #[inline] | ||
| fn classify_char(buf: &[u8], byte: usize, uflag: bool) -> (CharType, usize, usize) { |
There was a problem hiding this comment.
I would use a more descriptive name than uflag. Or alternatively, split the function into two functions so you don't need uflag.
| fn classify_char(buf: &[u8], byte: usize, uflag: bool) -> (CharType, usize, usize) { | ||
| use self::CharType::{Backspace, Other, Tab}; | ||
|
|
||
| if uflag { |
There was a problem hiding this comment.
Maybe I'm misunderstanding the meaning of uflag, but shouldn't the if and else blocks be switched? From the help:
-U, --no-utf8 interpret input file as 8-bit ASCII rather than UTF-8
Currently, the code does the opposite: if uflag is true, the char is handled as UTF-8.
| Some('\t') => (Tab, 0, nbytes), | ||
| Some('\x08') => (Backspace, 0, nbytes), |
There was a problem hiding this comment.
Is it correct that the display_width for Tab and Backspace differs between UTF-8 and ASCII?
| Some('\t') => (Tab, 0, nbytes), | ||
| Some('\x08') => (Backspace, 0, nbytes), |
There was a problem hiding this comment.
I would use the concrete byte length instead of nbytes:
| Some('\t') => (Tab, 0, nbytes), | |
| Some('\x08') => (Backspace, 0, nbytes), | |
| Some('\t') => (Tab, 0, 1), | |
| Some('\x08') => (Backspace, 0, 1), |
2 participants
No description provided.