Skip to content

Commit

Permalink
Remove broken wrapping logic
Browse files Browse the repository at this point in the history
The exiting line wrapping logic does not solve the issue described in nushell#176 but the constant polling of position as soon as the cursor reached the first line ending causes performance issues
  • Loading branch information
sholderbach committed Nov 7, 2021
1 parent 8a3ec4c commit 4010656
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 78 deletions.
4 changes: 0 additions & 4 deletions src/core_editor/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,6 @@ impl Editor {
self.line_buffer.offset()
}

pub fn line(&self) -> usize {
self.line_buffer.line()
}

pub fn num_lines(&self) -> usize {
self.line_buffer.num_lines()
}
Expand Down
80 changes: 6 additions & 74 deletions src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ impl Default for PromptCoordinates {
}

impl PromptCoordinates {
fn input_start_col(&self) -> u16 {
self.input_start.0
}
fn prompt_start_col(&self) -> u16 {
self.prompt_start.0
}
Expand Down Expand Up @@ -504,12 +501,12 @@ impl Reedline {
self.editor.reset_undo_stack();
Ok(Some(Signal::CtrlD))
} else {
self.run_edit_commands(&[EditCommand::Delete], prompt)?;
self.run_edit_commands(&[EditCommand::Delete])?;
Ok(None)
}
}
ReedlineEvent::CtrlC => {
self.run_edit_commands(&[EditCommand::Clear], prompt)?;
self.run_edit_commands(&[EditCommand::Clear])?;
self.editor.reset_undo_stack();
Ok(Some(Signal::CtrlC))
}
Expand All @@ -518,21 +515,21 @@ impl Reedline {
let buffer = self.editor.get_buffer().to_string();
if matches!(self.validator.validate(&buffer), ValidationResult::Complete) {
self.append_to_history();
self.run_edit_commands(&[EditCommand::Clear], prompt)?;
self.run_edit_commands(&[EditCommand::Clear])?;
self.print_crlf()?;
self.editor.reset_undo_stack();

Ok(Some(Signal::Success(buffer)))
} else {
self.run_edit_commands(&[EditCommand::InsertChar('\n')], prompt)?;
self.run_edit_commands(&[EditCommand::InsertChar('\n')])?;
self.adjust_prompt_position()?;
self.full_repaint(prompt)?;

Ok(None)
}
}
ReedlineEvent::Edit(commands) => {
self.run_edit_commands(&commands, prompt)?;
self.run_edit_commands(&commands)?;
self.repaint(prompt)?;
Ok(None)
}
Expand Down Expand Up @@ -714,7 +711,6 @@ impl Reedline {
fn run_edit_commands(
&mut self,
commands: &[EditCommand],
prompt: &dyn Prompt,
) -> io::Result<()> {
if self.input_mode == InputMode::HistoryTraversal {
if matches!(
Expand All @@ -737,33 +733,10 @@ impl Reedline {
EditCommand::MoveRight => self.editor.move_right(),
EditCommand::MoveWordLeft => self.editor.move_word_left(),
EditCommand::MoveWordRight => self.editor.move_word_right(),
// 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) => {
let line_start = if self.editor.line() == 0 {
self.prompt_coords.input_start_col()
} else {
// TODO: Would not hold if a continuation prompt is printed
0
};

// TODO: This check should only be triggered if the **cursor** might wrap not just the line being long enough to wrap!
// Two cases were corrections might be necessary:
// 1. Cursor is placed in such a way that the insertion will wrap it to the next line. (Respond correctly to potential bottom scrolling)
// 2. The length of the line at the bottom of the screen will wrap thus shifting the cursor one line up in screen coordinates
// The position polling twice is quite expensive!
if self.might_require_wrapping(line_start, *c) {
let position = cursor::position()?;
self.editor.insert_char(*c);
self.wrap_cursor_position(position)?;
} else {
self.editor.insert_char(*c);
}
self.editor.insert_char(*c);
self.editor.remember_undo_state(false);

self.repaint(prompt)?;
}
EditCommand::Backspace => self.editor.backspace(),
EditCommand::Delete => self.editor.delete(),
Expand Down Expand Up @@ -814,10 +787,6 @@ impl Reedline {
self.editor.set_insertion_point(pos)
}

fn terminal_columns(&self) -> u16 {
self.terminal_size.0
}

fn terminal_rows(&self) -> u16 {
self.terminal_size.1
}
Expand All @@ -842,43 +811,6 @@ impl Reedline {
}
}

/// TODO! provide an accurate doccomment
fn wrap_cursor_position(&mut self, original_position: (u16, u16)) -> io::Result<()> {
let (original_column, original_row) = original_position;
self.buffer_paint(self.prompt_coords.input_start)?;

let (new_column, _) = cursor::position()?;

if new_column < original_column && original_row + 1 == self.terminal_rows() {
// We have wrapped off bottom of screen, and prompt is on new row
// We need to update the prompt location in this case
let (input_start_col, input_start_row) = self.prompt_coords.input_start;
let (prompt_start_col, prompt_start_row) = self.prompt_coords.prompt_start;
self.prompt_coords
.set_input_start(input_start_col, input_start_row - 1);
self.prompt_coords
.set_prompt_start(prompt_start_col, prompt_start_row - 1);
}

Ok(())
}

/// Heuristic to predetermine if we need to poll the terminal if the text wrapped around.
fn might_require_wrapping(&self, start_offset: u16, c: char) -> bool {
use unicode_width::UnicodeWidthStr;

let terminal_width = self.terminal_columns();

// TODO: We also need to consider effects when inserting inside the string
let mut test_buffer = self.editor.get_buffer().to_string();
test_buffer.push(c);

// TODO: Start offset may differ in a multiple wrapping lines scenario!
let display_width = UnicodeWidthStr::width(test_buffer.as_str()) + start_offset as usize;

display_width >= terminal_width as usize
}

/// Display only the prompt components preceding the buffer
///
/// Used to restore the prompt indicator after a search etc. that affected
Expand Down

0 comments on commit 4010656

Please sign in to comment.