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

Move wrapping detection out of the single character insertion case #186

Closed
sholderbach opened this issue Nov 28, 2021 · 1 comment
Closed
Labels
A-Display Area: Correctness of the display output and additional features needed there A-Multiline Area: Support for multiline editing (Validation and interaction with `A-Display`)

Comments

@sholderbach
Copy link
Member

Currently wrapping detection is only performed when handling the insert of a single character...

reedline/src/engine.rs

Lines 813 to 828 in 75d9b4b

// Performing mutation here might incur a perf hit down this line when
// we would like to do multiple inserts.
// A simple solution that we can do is to queue up these and perform the wrapping
// check after the loop finishes. Will need to sort out the details.
EditCommand::InsertChar(c) => {
self.editor.insert_char(*c);
if self.require_wrapping() {
let position = cursor::position()?;
self.wrap(position)?;
}
self.editor.remember_undo_state(false);
self.repaint(prompt)?;
}

... but this is wrong as the line's content could also overflow on other operations (e.g. using the paste operations or undo) resulting in the requirement to move the cursor.

Thus we either do such a detection explicitly on inserting operations or move the check and cursor position update into the painter and do it there in the flow of the output.

#176 relates to other technical issues with the current implementation. #180 improved some of the details, but insertions in the middle of the line that might shift the cursor are not fully covered yet.

@sholderbach sholderbach added A-Display Area: Correctness of the display output and additional features needed there A-Multiline Area: Support for multiline editing (Validation and interaction with `A-Display`) labels Nov 28, 2021
@sholderbach
Copy link
Member Author

#217 eliminated the insertion specific wrapping detection and moves much of the required logic into the painter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Display Area: Correctness of the display output and additional features needed there A-Multiline Area: Support for multiline editing (Validation and interaction with `A-Display`)
Projects
None yet
Development

No branches or pull requests

1 participant