line_wrapped code smell
I'm not sure what line_wrapped
and m_line_wrapped
are exactly supposed to do, but they don't look good to me.
The middle of process_incoming_utf8()
looks for a run of printable plain old ASCII characters, calls insert_char()
for each, and then calls post_GRAPHIC()
:
while (ip < iend && *ip >= 0x20 && *ip < 0x7f) [[likely]] {
insert_char(*ip, false, false);
ip++;
}
context.post_GRAPHIC(*this);
Each insert_char()
flips the method's internal line_wrapped
to true
if that char caused a line wrap (why not set the m_
member directly?), and at the end of the method sets m_line_wrapped = line_wrapped;
.
So, after inserting all the chars of an ASCII run, m_line_wrapped
contains whether the very last character caused a wrap. A pretty unusable piece of information.
Then, post_GRAPHIC()
does some bbox correcting only if this is set. I'm not sure if forgetting to adjust the bbox if an earlier character caused a line wrap may result in forgetting to update the display (although I couldn't trigger faulty end-user behavior yet).
Should perhaps that m_line_wrapped = ...
be an |=
instead (or rather, set m_line_wrapped
directly to true
whenever wrapping occurs and leave unchanged otherwise)? Or is this whole thing a leftover that's no longer needed?