Input area improvements/tasks
I was reading the code today and found some potential improvements and a problem in the code.
-
[x] Starting with the problem:
InputArea::heightis never set. It starts its life asNoneand stays that way. That meansInputArea::get_heightalways recomputes the height even if nothing changes inInputArea. -
[x] I think it'd be better to invalidate
InputLine::ws_indices(renamed fromInputLine::line_starts) on update. Currently we don't update it untilcalculate_heightis called, and if I update a line and forget to callcalculate_heightthendrawwill render input area incorrectly. -
[x] We should have a fast path for the common case of inserting at the end. ~~Currently this causes quadratic behavior on paste, where we turn a long string into a series of key press events (see
handle_input_event), and for every key press we compute the whitespace indices from scratch.~~ (this comments was incorrect, see my update in the comments section below) I thinkInputLine::insertcould be easily updated to handle this and efficiently updatews_indiceswhen inserting at the end.This also means
InputLine::calculate_heightshouldn't generatews_indicesfrom scratch if it's already up-to-date (related to the second point above), otherwise the update ininsertwill never be used. -
[ ] Similar to the item above, deleting from the end shouldn't require generating the state from scratch.
cc @trevarj
Thanks for taking a look and fixing it up. I read through your changes and they were really helpful for my learning.
I edited my original post: pasting causes "quadratic" behavior, not "exponential"!
-
This first issue seems like it may be as as simple as putting
self.height = Some(line_count);at the end ofInputArea::calculate_lines(). I tested it a little, but not much. -
Invalidating
ws_indicesseems like a good idea. When would you update and forget to call calculate height though? Does this just mean if the code were to change? -
Should we also re-write the paste functionality to do a memcpy into the line buffer instead, then invalidate
ws_indicesandInputArea::height?
Copying my comment from #210:
Btw, I realized that my remark about quadratic behavior on paste in #208 is incorrect: we call
TUI::keypressedfor every char in the pasted string, butkeypresseddoesn't draw after handling the key. So we calldrawjust once after handling all the keypresses. Sorry for the false alarm!
#210 fixed the issues here, but I just added one more relevant item which is for efficiently removing from the end.