helix
helix copied to clipboard
fix: Recalculate completion when going through prompt history
Fixes a panic related to completion when using the arrow keys to navigate through the prompt history.
Minimal reproduction steps for the panic
<launch helix>
:asdf<Enter> <- Anything that is shorter than what you will be trying to complete next works.
:theme d <- Note that triggering completion is not necessary since it's recalculated on each character entry.
<up arrow>
<tab>
It should not be possible to modify the prompt line
without also updating completion list. Most functions modifying the line
update the completions. Prompt::insert_str
and Prompt::change_history
does not which allows this panic to happen. This means that the <c-s>
shortcut probably also can create a panic and there are also possible panics in the DAP prompt as well (helix-term/src/commands/dap.rs
).
This PR fixes the immediate problem, but I think I real solution should force update the completion list when updating the prompt line in some centralized way. Another solution might be to tie the completion list more tightly to the prompt line itself so that it can be recalculated if needed when completing.
With the last two commits it's no longer possible to call a function that modifies the prompt line without also updating the completion.
The only direct calls to recalculate the completion now happen when you want to have a completion with an empty line. I think this makes sense since you might not always want to have the completion list.
I've done some manual testing locally and it has not crashed on me yet.
Great!
I also noticed the pairing of exit_selection and recalculate_completion and am wondering if an extra (private) function to join them is warranted to keep the code short. But as the order may or may not be of importance (reversed in clear function) this may just add noise, especially since this is local to this module.
I looked into it. exit_selection
only clears an index into the completion list so order here shouldn't matter. I've added a call to exit_selection
in recalculate_completion
since it doesn't make sense to keep the old index when the list has changed.
In the future you could try to match the previous selection into the new list if possible, but I don't think the extra complexity would be worth it.