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

Add Option for Prefix-Based Reverse Search #424

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion examples/example.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::borrow::Cow::{self, Borrowed, Owned};

use rustyline::completion::{Completer, FilenameCompleter, Pair};
use rustyline::config::OutputStreamType;
use rustyline::config::{HistorySearchBehaviour, OutputStreamType};
use rustyline::error::ReadlineError;
use rustyline::highlight::{Highlighter, MatchingBracketHighlighter};
use rustyline::hint::{Hinter, HistoryHinter};
Expand Down Expand Up @@ -87,6 +87,7 @@ fn main() -> rustyline::Result<()> {
.completion_type(CompletionType::List)
.edit_mode(EditMode::Emacs)
.output_stream(OutputStreamType::Stdout)
.history_search_behaviour(HistorySearchBehaviour::LineByLine)
.build();
let h = MyHelper {
completer: FilenameCompleter::new(),
Expand Down
4 changes: 2 additions & 2 deletions src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ pub fn execute<H: Helper>(
s.edit_history_next(false)?
}
}
Cmd::HistorySearchBackward => s.edit_history_search(Direction::Reverse)?,
Cmd::HistorySearchForward => s.edit_history_search(Direction::Forward)?,
Cmd::HistorySearchBackward => s.edit_history_search(Direction::Reverse, false)?,
Cmd::HistorySearchForward => s.edit_history_search(Direction::Forward, false)?,
Cmd::TransposeChars => {
// Exchange the char before cursor with the character at cursor.
s.edit_transpose_chars()?
Expand Down
51 changes: 51 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ pub struct Config {
check_cursor_position: bool,
/// Bracketed paste on unix platform
enable_bracketed_paste: bool,
/// Whether Arrow-Up/Down and C-p/n should go through the history line-by-line or perform a
/// reverse-search with the current prefix before the cursor.
history_search_behaviour: HistorySearchBehaviour,
}

impl Config {
Expand Down Expand Up @@ -176,6 +179,21 @@ impl Config {
pub fn enable_bracketed_paste(&self) -> bool {
self.enable_bracketed_paste
}

/// Whether Arrow-Up/Down and C-p/n should go through the history line-by-line or perform a
/// reverse-search with the current prefix before the cursor.
///
/// By default, go through the history line-by-line.
pub fn history_search_behaviour(&self) -> HistorySearchBehaviour {
self.history_search_behaviour
}

pub(crate) fn set_history_search_behaviour(
&mut self,
history_search_behaviour: HistorySearchBehaviour,
) {
self.history_search_behaviour = history_search_behaviour;
}
}

impl Default for Config {
Expand All @@ -196,6 +214,7 @@ impl Default for Config {
indent_size: 2,
check_cursor_position: false,
enable_bracketed_paste: true,
history_search_behaviour: HistorySearchBehaviour::LineByLine,
}
}
}
Expand Down Expand Up @@ -286,6 +305,17 @@ pub enum OutputStreamType {
Stdout,
}

/// Control going through the history with Arrow-Up/Down and C-p/n
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[non_exhaustive]
pub enum HistorySearchBehaviour {
/// Go backwards through the history line-by-line.
LineByLine,
/// Implicit reverse search: Only show history entries which start with the current prefix
/// before the cursor.
PrefixReverseSearch,
}

/// Configuration builder
#[derive(Clone, Debug, Default)]
pub struct Builder {
Expand Down Expand Up @@ -415,6 +445,18 @@ impl Builder {
self
}

/// Whether Arrow-Up/Down and C-p/n should go through the history line-by-line or perform a
/// reverse-search with the current prefix before the cursor.
///
/// By default, go through the history line-by-line.
pub fn history_search_behaviour(
mut self,
history_search_behaviour: HistorySearchBehaviour,
) -> Self {
self.set_history_search_behaviour(history_search_behaviour);
self
}

/// Builds a `Config` with the settings specified so far.
pub fn build(self) -> Config {
self.p
Expand Down Expand Up @@ -529,4 +571,13 @@ pub trait Configurer {
fn enable_bracketed_paste(&mut self, enabled: bool) {
self.config_mut().enable_bracketed_paste = enabled;
}

/// Whether Arrow-Up/Down and C-p/n should go through the history line-by-line or perform a
/// reverse-search with the current prefix before the cursor.
///
/// By default, go through the history line-by-line.
fn set_history_search_behaviour(&mut self, history_search_behaviour: HistorySearchBehaviour) {
self.config_mut()
.set_history_search_behaviour(history_search_behaviour);
}
}
141 changes: 117 additions & 24 deletions src/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use unicode_segmentation::UnicodeSegmentation;
use unicode_width::UnicodeWidthChar;

use super::{Context, Helper, Result};
use crate::config::HistorySearchBehaviour;
use crate::highlight::Highlighter;
use crate::hint::Hint;
use crate::history::Direction;
Expand All @@ -34,6 +35,7 @@ pub struct State<'out, 'prompt, H: Helper> {
pub ctx: Context<'out>, // Give access to history for `hinter`
pub hint: Option<Box<dyn Hint>>, // last hint displayed
highlight_char: bool, // `true` if a char has been highlighted
history_search_behaviour: HistorySearchBehaviour, // search history line-by-line or prefix-based
}

enum Info<'m> {
Expand All @@ -48,6 +50,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> {
prompt: &'prompt str,
helper: Option<&'out H>,
ctx: Context<'out>,
history_search_behaviour: HistorySearchBehaviour,
) -> State<'out, 'prompt, H> {
let prompt_size = out.calculate_position(prompt, Position::default());
State {
Expand All @@ -63,6 +66,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> {
ctx,
hint: None,
highlight_char: false,
history_search_behaviour,
}
}

Expand Down Expand Up @@ -228,6 +232,12 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> {
Ok(ValidationResult::Valid(None))
}
}

fn update_line(&mut self, buf: &str, pos: usize) {
self.changes.borrow_mut().begin();
self.line.update(buf, pos);
self.changes.borrow_mut().end();
}
}

impl<'out, 'prompt, H: Helper> Invoke for State<'out, 'prompt, H> {
Expand Down Expand Up @@ -585,25 +595,42 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> {
} else if self.ctx.history_index == 0 && prev {
return Ok(());
}
if prev {
self.ctx.history_index -= 1;
} else {
self.ctx.history_index += 1;
}
if self.ctx.history_index < history.len() {
let buf = history.get(self.ctx.history_index).unwrap();
self.changes.borrow_mut().begin();
self.line.update(buf, buf.len());
self.changes.borrow_mut().end();
} else {
// Restore current edited line
self.restore();

match self.history_search_behaviour {
// only if we have a prefix, do a prefix reverse search
HistorySearchBehaviour::PrefixReverseSearch if self.line.pos() != 0 => {
let direction = if prev {
Direction::Reverse
} else {
Direction::Forward
};
self.edit_history_search(direction, true)?;
}
// if we don't have a prefix, just go forward / backward one history entry
HistorySearchBehaviour::LineByLine | HistorySearchBehaviour::PrefixReverseSearch => {
if prev {
self.ctx.history_index -= 1;
} else {
self.ctx.history_index += 1;
}
if self.ctx.history_index < history.len() {
let buf = history.get(self.ctx.history_index).unwrap();
let pos = match self.history_search_behaviour {
HistorySearchBehaviour::LineByLine => buf.len(),
HistorySearchBehaviour::PrefixReverseSearch => self.line.pos(),
};
self.update_line(buf, pos);
} else {
// Restore current edited line
self.restore();
}
}
}
self.refresh_line()
}

// Non-incremental, anchored search
pub fn edit_history_search(&mut self, dir: Direction) -> Result<()> {
pub fn edit_history_search(&mut self, dir: Direction, keep_cursor_pos: bool) -> Result<()> {
let history = self.ctx.history;
if history.is_empty() {
return self.out.beep();
Expand All @@ -625,13 +652,28 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> {
) {
self.ctx.history_index = history_index;
let buf = history.get(history_index).unwrap();
self.changes.borrow_mut().begin();
self.line.update(buf, buf.len());
self.changes.borrow_mut().end();
self.refresh_line()
} else {
self.out.beep()

// if the history search results in the same command we found last, continue searching
// for a different command
if buf == self.line.as_str() {
return self.edit_history_search(dir, keep_cursor_pos);
}

let pos = if keep_cursor_pos {
self.line.pos()
} else {
buf.len()
};
self.update_line(buf, pos);
} else {
self.ctx.history_index = match dir {
Direction::Forward => history.len(),
Direction::Reverse => 0,
};
self.restore();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this restore breaks the following code:

rustyline/src/lib.rs

Lines 558 to 559 in 88e669c

Cmd::HistorySearchBackward => s.edit_history_search(Direction::Reverse, false)?,
Cmd::HistorySearchForward => s.edit_history_search(Direction::Forward, false)?,

Copy link
Collaborator

@gwenn gwenn Jun 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bash history-search-backward does not seem to loop or skip same history entries.
But you are right: cursor position is preserved.
I guess we should be able to use only a ConditionalEventHandler: if no prefix => PreviousHistory/ NextHistory else => HistorySearchBackward / HistorySearchForward.

self.out.beep()?;
}
self.refresh_line()
}

/// Substitute the currently edited line with the first/last history entry.
Expand All @@ -653,9 +695,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> {
if first {
self.ctx.history_index = 0;
let buf = history.get(self.ctx.history_index).unwrap();
self.changes.borrow_mut().begin();
self.line.update(buf, buf.len());
self.changes.borrow_mut().end();
self.update_line(buf, buf.len());
} else {
self.ctx.history_index = history.len();
// Restore current edited line
Expand All @@ -681,6 +721,7 @@ pub fn init_state<'out, H: Helper>(
pos: usize,
helper: Option<&'out H>,
history: &'out crate::history::History,
history_search_behaviour: HistorySearchBehaviour,
) -> State<'out, 'static, H> {
State {
out,
Expand All @@ -695,12 +736,14 @@ pub fn init_state<'out, H: Helper>(
ctx: Context::new(history),
hint: Some(Box::new("hint".to_owned())),
highlight_char: false,
history_search_behaviour,
}
}

#[cfg(test)]
mod test {
use super::init_state;
use crate::config::HistorySearchBehaviour;
use crate::history::History;
use crate::tty::Sink;

Expand All @@ -712,7 +755,8 @@ mod test {
history.add("line1");
let line = "current edited line";
let helper: Option<()> = None;
let mut s = init_state(&mut out, line, 6, helper.as_ref(), &history);
let hsb = HistorySearchBehaviour::LineByLine;
let mut s = init_state(&mut out, line, 6, helper.as_ref(), &history, hsb);
s.ctx.history_index = history.len();

for _ in 0..2 {
Expand Down Expand Up @@ -742,4 +786,53 @@ mod test {
assert_eq!(2, s.ctx.history_index);
assert_eq!(line, s.line.as_str());
}

#[test]
fn edit_history_prefix() {
let mut out = Sink::new();
let mut history = History::new();
history.add("foo");
history.add("bar");
history.add("cd abcdef");
history.add("ls");
history.add("cd abcdef");
history.add("ls");
history.add("cd ab123");
history.add("ls");
let line = "cd abxyz";
let helper: Option<()> = None;
let hsb = HistorySearchBehaviour::PrefixReverseSearch;
let mut s = init_state(&mut out, line, 5, helper.as_ref(), &history, hsb);
s.ctx.history_index = history.len();

s.edit_history_next(true).unwrap();
assert_eq!(line, s.saved_line_for_history.as_str());
assert_eq!(6, s.ctx.history_index);
assert_eq!("cd ab123", s.line.as_str());

s.edit_history_next(true).unwrap();
assert_eq!(line, s.saved_line_for_history.as_str());
assert_eq!(4, s.ctx.history_index);
assert_eq!("cd abcdef", s.line.as_str());

s.edit_history_next(true).unwrap();
assert_eq!(line, s.saved_line_for_history.as_str());
assert_eq!(0, s.ctx.history_index);
assert_eq!(line, s.line.as_str());

s.edit_history_next(false).unwrap();
assert_eq!(line, s.saved_line_for_history.as_str());
assert_eq!(2, s.ctx.history_index);
assert_eq!("cd abcdef", s.line.as_str());

s.edit_history_next(false).unwrap();
assert_eq!(line, s.saved_line_for_history.as_str());
assert_eq!(6, s.ctx.history_index);
assert_eq!("cd ab123", s.line.as_str());

s.edit_history_next(false).unwrap();
// assert_eq!(line, s.saved_line_for_history.as_str());
assert_eq!(8, s.ctx.history.len());
assert_eq!(line, s.line.as_str());
}
}
8 changes: 7 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,13 @@ fn readline_edit<H: Helper>(

editor.reset_kill_ring(); // TODO recreate a new kill ring vs Arc<Mutex<KillRing>>
let ctx = Context::new(&editor.history);
let mut s = State::new(&mut stdout, prompt, editor.helper.as_ref(), ctx);
let mut s = State::new(
&mut stdout,
prompt,
editor.helper.as_ref(),
ctx,
editor.config.history_search_behaviour(),
);

let mut input_state = InputState::new(&editor.config, Arc::clone(&editor.custom_bindings));

Expand Down
5 changes: 3 additions & 2 deletions src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::vec::IntoIter;
use radix_trie::Trie;

use crate::completion::Completer;
use crate::config::{Config, EditMode};
use crate::config::{Config, EditMode, HistorySearchBehaviour};
use crate::edit::init_state;
use crate::highlight::Highlighter;
use crate::hint::Hinter;
Expand Down Expand Up @@ -57,7 +57,8 @@ fn complete_line() {
let mut out = Sink::new();
let history = crate::history::History::new();
let helper = Some(SimpleCompleter);
let mut s = init_state(&mut out, "rus", 3, helper.as_ref(), &history);
let hsb = HistorySearchBehaviour::LineByLine;
let mut s = init_state(&mut out, "rus", 3, helper.as_ref(), &history, hsb);
let config = Config::default();
let mut input_state = InputState::new(&config, Arc::new(RwLock::new(Trie::new())));
let keys = vec![E::ENTER];
Expand Down