Skip to content

Commit

Permalink
Fix (properly) the logic around prompt re-use & Host Command handling (
Browse files Browse the repository at this point in the history
…#770)

* Fix (properly) the logic around prompt re-use & Host Command handling

* Move to dedicated selector function, add tests

* cargo fmt
  • Loading branch information
bew authored Apr 23, 2024
1 parent cc9a957 commit 455b9a3
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 36 deletions.
34 changes: 19 additions & 15 deletions src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use {
FileBackedHistory, History, HistoryCursor, HistoryItem, HistoryItemId,
HistoryNavigationQuery, HistorySessionId, SearchDirection, SearchQuery,
},
painting::{Painter, PromptLines},
painting::{Painter, PainterSuspendedState, PromptLines},
prompt::{PromptEditMode, PromptHistorySearchStatus},
result::{ReedlineError, ReedlineErrorVariants},
terminal_extensions::{bracketed_paste::BracketedPasteGuard, kitty::KittyProtocolGuard},
Expand Down Expand Up @@ -109,8 +109,9 @@ pub struct Reedline {
history_cursor_on_excluded: bool,
input_mode: InputMode,

// Yielded to the host program after a `ReedlineEvent::ExecuteHostCommand`, thus redraw in-place
executing_host_command: bool,
// State of the painter after a `ReedlineEvent::ExecuteHostCommand` was requested, used after
// execution to decide if we can re-use the previous prompt or paint a new one.
suspended_state: Option<PainterSuspendedState>,

// Validator
validator: Option<Box<dyn Validator>>,
Expand Down Expand Up @@ -210,7 +211,7 @@ impl Reedline {
history_excluded_item: None,
history_cursor_on_excluded: false,
input_mode: InputMode::Regular,
executing_host_command: false,
suspended_state: None,
painter,
transient_prompt: None,
edit_mode,
Expand Down Expand Up @@ -671,12 +672,14 @@ impl Reedline {
/// Helper implementing the logic for [`Reedline::read_line()`] to be wrapped
/// in a `raw_mode` context.
fn read_line_helper(&mut self, prompt: &dyn Prompt) -> Result<Signal> {
if self.executing_host_command {
self.executing_host_command = false;
} else {
self.painter.initialize_prompt_position()?;
self.hide_hints = false;
self.painter
.initialize_prompt_position(self.suspended_state.as_ref())?;
if self.suspended_state.is_some() {
// Last editor was suspended to run a ExecuteHostCommand event,
// we are resuming operation now.
self.suspended_state = None;
}
self.hide_hints = false;

self.repaint(prompt)?;

Expand Down Expand Up @@ -773,8 +776,11 @@ impl Reedline {
for event in reedline_events.drain(..) {
match self.handle_event(prompt, event)? {
EventStatus::Exits(signal) => {
if !self.executing_host_command {
// Move the cursor below the input area, for external commands or new read_line call
// Check if we are merely suspended (to process an ExecuteHostCommand event)
// or if we're about to quit the editor.
if self.suspended_state.is_none() {
// We are about to quit the editor, move the cursor below the input
// area, for external commands or new read_line call
self.painter.move_cursor_to_end()?;
}
return Ok(signal);
Expand Down Expand Up @@ -851,8 +857,7 @@ impl Reedline {
Ok(EventStatus::Handled)
}
ReedlineEvent::ExecuteHostCommand(host_command) => {
// TODO: Decide if we need to do something special to have a nicer painter state on the next go
self.executing_host_command = true;
self.suspended_state = Some(self.painter.state_before_suspension());
Ok(EventStatus::Exits(Signal::Success(host_command)))
}
ReedlineEvent::Edit(commands) => {
Expand Down Expand Up @@ -1122,8 +1127,7 @@ impl Reedline {
}
}
ReedlineEvent::ExecuteHostCommand(host_command) => {
// TODO: Decide if we need to do something special to have a nicer painter state on the next go
self.executing_host_command = true;
self.suspended_state = Some(self.painter.state_before_suspension());
Ok(EventStatus::Exits(Signal::Success(host_command)))
}
ReedlineEvent::Edit(commands) => {
Expand Down
2 changes: 1 addition & 1 deletion src/painting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ mod prompt_lines;
mod styled_text;
mod utils;

pub use painter::Painter;
pub use painter::{Painter, PainterSuspendedState};
pub(crate) use prompt_lines::PromptLines;
pub use styled_text::StyledText;
pub(crate) use utils::estimate_single_line_wraps;
122 changes: 102 additions & 20 deletions src/painting/painter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use {
QueueableCommand,
},
std::io::{Result, Write},
std::ops::RangeInclusive,
};
#[cfg(feature = "external_printer")]
use {crate::LineBuffer, crossterm::cursor::MoveUp};
Expand Down Expand Up @@ -49,6 +50,42 @@ fn skip_buffer_lines(string: &str, skip: usize, offset: Option<usize>) -> &str {
/// the type used by crossterm operations
pub type W = std::io::BufWriter<std::io::Stderr>;

#[derive(Debug, PartialEq, Eq)]
pub struct PainterSuspendedState {
previous_prompt_rows_range: RangeInclusive<u16>,
}

#[derive(Debug, PartialEq, Eq)]
enum PromptRowSelector {
UseExistingPrompt { start_row: u16 },
MakeNewPrompt { new_row: u16 },
}

// Selects the row where the next prompt should start on, taking into account and whether it should re-use a previous
// prompt.
fn select_prompt_row(
suspended_state: Option<&PainterSuspendedState>,
(column, row): (u16, u16), // NOTE: Positions are 0 based here
) -> PromptRowSelector {
if let Some(painter_state) = suspended_state {
// The painter was suspended, try to re-use the last prompt position to avoid
// unnecessarily making new prompts.
if painter_state.previous_prompt_rows_range.contains(&row) {
// Cursor is still in the range of the previous prompt, re-use it.
let start_row = *painter_state.previous_prompt_rows_range.start();
return PromptRowSelector::UseExistingPrompt { start_row };
} else {
// There was some output or cursor is outside of the range of previous prompt make a
// fresh new prompt.
}
}

// Assumption: if the cursor is not on the zeroth column,
// there is content we want to leave intact, thus advance to the next row.
let new_row = if column > 0 { row + 1 } else { row };
PromptRowSelector::MakeNewPrompt { new_row }
}

/// Implementation of the output to the terminal
pub struct Painter {
// Stdout
Expand Down Expand Up @@ -85,12 +122,26 @@ impl Painter {
self.screen_height().saturating_sub(self.prompt_start_row)
}

/// Returns the state necessary before suspending the painter (to run a host command event).
///
/// This state will be used to re-initialize the painter to re-use last prompt if possible.
pub fn state_before_suspension(&self) -> PainterSuspendedState {
let start_row = self.prompt_start_row;
let final_row = start_row + self.last_required_lines;
PainterSuspendedState {
previous_prompt_rows_range: start_row..=final_row,
}
}

/// Sets the prompt origin position and screen size for a new line editor
/// invocation
///
/// Not to be used for resizes during a running line editor, use
/// [`Painter::handle_resize()`] instead
pub(crate) fn initialize_prompt_position(&mut self) -> Result<()> {
pub(crate) fn initialize_prompt_position(
&mut self,
suspended_state: Option<&PainterSuspendedState>,
) -> Result<()> {
// Update the terminal size
self.terminal_size = {
let size = terminal::size()?;
Expand All @@ -102,26 +153,26 @@ impl Painter {
size
}
};
// Cursor positions are 0 based here.
let (column, row) = cursor::position()?;
// Assumption: if the cursor is not on the zeroth column,
// there is content we want to leave intact, thus advance to the next row
let new_row = if column > 0 { row + 1 } else { row };
// If we are on the last line and would move beyond the last line due to
// the condition above, we need to make room for the prompt.
// Otherwise printing the prompt would scroll of the stored prompt
// origin, causing issues after repaints.
let new_row = if new_row == self.screen_height() {
self.print_crlf()?;
new_row.saturating_sub(1)
} else {
new_row
let prompt_selector = select_prompt_row(suspended_state, cursor::position()?);
self.prompt_start_row = match prompt_selector {
PromptRowSelector::UseExistingPrompt { start_row } => start_row,
PromptRowSelector::MakeNewPrompt { new_row } => {
// If we are on the last line and would move beyond the last line, we need to make
// room for the prompt.
// Otherwise printing the prompt would scroll off the stored prompt
// origin, causing issues after repaints.
if new_row == self.screen_height() {
self.print_crlf()?;
new_row.saturating_sub(1)
} else {
new_row
}
}
};
self.prompt_start_row = new_row;
Ok(())
}

/// Main pain painter for the prompt and buffer
/// Main painter for the prompt and buffer
/// It queues all the actions required to print the prompt together with
/// lines that make the buffer.
/// Using the prompt lines object in this function it is estimated how the
Expand Down Expand Up @@ -461,7 +512,7 @@ impl Painter {
self.stdout.queue(cursor::Show)?;

self.stdout.flush()?;
self.initialize_prompt_position()
self.initialize_prompt_position(None)
}

pub(crate) fn clear_scrollback(&mut self) -> Result<()> {
Expand All @@ -470,11 +521,11 @@ impl Painter {
.queue(crossterm::terminal::Clear(ClearType::Purge))?
.queue(cursor::MoveTo(0, 0))?
.flush()?;
self.initialize_prompt_position()
self.initialize_prompt_position(None)
}

// The prompt is moved to the end of the buffer after the event was handled
// If the prompt is in the middle of a multiline buffer, then the output to stdout
// If the prompt is in the middle of a multiline buffer, then the next output to stdout
// could overwrite the buffer writing
pub(crate) fn move_cursor_to_end(&mut self) -> Result<()> {
let final_row = self.prompt_start_row + self.last_required_lines;
Expand Down Expand Up @@ -619,4 +670,35 @@ mod tests {
assert_eq!(skip_buffer_lines(string, 0, Some(0)), "sentence1",);
assert_eq!(skip_buffer_lines(string, 1, Some(0)), "sentence2",);
}

#[test]
fn test_select_new_prompt_with_no_state_no_output() {
assert_eq!(
select_prompt_row(None, (0, 12)),
PromptRowSelector::MakeNewPrompt { new_row: 12 }
);
}

#[test]
fn test_select_new_prompt_with_no_state_but_output() {
assert_eq!(
select_prompt_row(None, (3, 12)),
PromptRowSelector::MakeNewPrompt { new_row: 13 }
);
}

#[test]
fn test_select_existing_prompt() {
let state = PainterSuspendedState {
previous_prompt_rows_range: 11..=13,
};
assert_eq!(
select_prompt_row(Some(&state), (0, 12)),
PromptRowSelector::UseExistingPrompt { start_row: 11 }
);
assert_eq!(
select_prompt_row(Some(&state), (3, 12)),
PromptRowSelector::UseExistingPrompt { start_row: 11 }
);
}
}

0 comments on commit 455b9a3

Please sign in to comment.