From c34ef39bb962f7f826cebdcb1b12565868d56288 Mon Sep 17 00:00:00 2001 From: gwenn Date: Mon, 7 Oct 2024 14:28:29 +0200 Subject: [PATCH] Fix Highlighter::highlight_char Introduce `CmdKind` parameter to make the distinction between edits vs cursor moves vs final refresh --- examples/example.rs | 6 ++--- examples/read_password.rs | 10 ++++--- rustyline-derive/src/lib.rs | 4 +-- src/command.rs | 7 +++-- src/edit.rs | 52 +++++++++++++++++-------------------- src/highlight.rs | 26 +++++++++++++------ src/keymap.rs | 3 ++- src/lib.rs | 6 ++--- src/tty/unix.rs | 2 +- 9 files changed, 62 insertions(+), 54 deletions(-) diff --git a/examples/example.rs b/examples/example.rs index cc3e2a0cc..d258064c6 100644 --- a/examples/example.rs +++ b/examples/example.rs @@ -2,7 +2,7 @@ use std::borrow::Cow::{self, Borrowed, Owned}; use rustyline::completion::FilenameCompleter; use rustyline::error::ReadlineError; -use rustyline::highlight::{Highlighter, MatchingBracketHighlighter}; +use rustyline::highlight::{CmdKind, Highlighter, MatchingBracketHighlighter}; use rustyline::hint::HistoryHinter; use rustyline::validate::MatchingBracketValidator; use rustyline::{Cmd, CompletionType, Config, EditMode, Editor, KeyEvent}; @@ -41,8 +41,8 @@ impl Highlighter for MyHelper { self.highlighter.highlight(line, pos) } - fn highlight_char(&self, line: &str, pos: usize, forced: bool) -> bool { - self.highlighter.highlight_char(line, pos, forced) + fn highlight_char(&self, line: &str, pos: usize, kind: CmdKind) -> bool { + self.highlighter.highlight_char(line, pos, kind) } } diff --git a/examples/read_password.rs b/examples/read_password.rs index d669d511b..d93418ae2 100644 --- a/examples/read_password.rs +++ b/examples/read_password.rs @@ -1,7 +1,7 @@ use std::borrow::Cow::{self, Borrowed, Owned}; use rustyline::config::Configurer; -use rustyline::highlight::Highlighter; +use rustyline::highlight::{CmdKind, Highlighter}; use rustyline::{ColorMode, Editor, Result}; use rustyline::{Completer, Helper, Hinter, Validator}; @@ -20,12 +20,16 @@ impl Highlighter for MaskingHighlighter { } } - fn highlight_char(&self, _line: &str, _pos: usize, _forced: bool) -> bool { - self.masking + fn highlight_char(&self, _line: &str, _pos: usize, kind: CmdKind) -> bool { + match kind { + CmdKind::MoveCursor => false, + _ => self.masking, + } } } fn main() -> Result<()> { + env_logger::init(); println!("This is just a hack. Reading passwords securely requires more than that."); let h = MaskingHighlighter { masking: false }; let mut rl = Editor::new()?; diff --git a/rustyline-derive/src/lib.rs b/rustyline-derive/src/lib.rs index c25e6165b..e23908c84 100644 --- a/rustyline-derive/src/lib.rs +++ b/rustyline-derive/src/lib.rs @@ -126,8 +126,8 @@ pub fn highlighter_macro_derive(input: TokenStream) -> TokenStream { ::rustyline::highlight::Highlighter::highlight_candidate(&self.#field_name_or_index, candidate, completion) } - fn highlight_char(&self, line: &str, pos: usize, forced: bool) -> bool { - ::rustyline::highlight::Highlighter::highlight_char(&self.#field_name_or_index, line, pos, forced) + fn highlight_char(&self, line: &str, pos: usize, kind: ::rustyline::highlight::CmdKind) -> bool { + ::rustyline::highlight::Highlighter::highlight_char(&self.#field_name_or_index, line, pos, kind) } } } diff --git a/src/command.rs b/src/command.rs index f0185e2cb..4eb215134 100644 --- a/src/command.rs +++ b/src/command.rs @@ -2,6 +2,7 @@ use crate::complete_hint_line; use crate::config::Config; use crate::edit::State; use crate::error; +use crate::highlight::CmdKind; use crate::history::SearchDirection; use crate::keymap::{Anchor, At, Cmd, Movement, Word}; use crate::keymap::{InputState, Refresher}; @@ -28,9 +29,7 @@ pub fn execute( if s.has_hint() || !s.is_default_prompt() || s.highlight_char { // Force a refresh without hints to leave the previous // line as the user typed it after a newline. - s.forced_refresh = true; - s.refresh_line_with_msg(None)?; - s.forced_refresh = false; + s.refresh_line_with_msg(None, CmdKind::ForcedRefresh)?; } } _ => {} @@ -190,7 +189,7 @@ pub fn execute( } Cmd::Move(Movement::EndOfBuffer) => { // Move to the end of the buffer. - s.edit_move_buffer_end()?; + s.edit_move_buffer_end(CmdKind::MoveCursor)?; } Cmd::DowncaseWord => { // lowercase word after point diff --git a/src/edit.rs b/src/edit.rs index c231e4a42..af75622ec 100644 --- a/src/edit.rs +++ b/src/edit.rs @@ -7,7 +7,7 @@ use unicode_width::UnicodeWidthChar; use super::{Context, Helper, Result}; use crate::error::ReadlineError; -use crate::highlight::Highlighter; +use crate::highlight::{CmdKind, Highlighter}; use crate::hint::Hint; use crate::history::SearchDirection; use crate::keymap::{Anchor, At, CharSearch, Cmd, Movement, RepeatCount, Word}; @@ -36,7 +36,6 @@ pub struct State<'out, 'prompt, H: Helper> { pub ctx: Context<'out>, // Give access to history for `hinter` pub hint: Option>, // last hint displayed pub highlight_char: bool, // `true` if a char has been highlighted - pub forced_refresh: bool, // `true` if line is redraw without hint or highlight_char } enum Info<'m> { @@ -66,7 +65,6 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { ctx, hint: None, highlight_char: false, - forced_refresh: false, } } @@ -122,7 +120,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { ); } - pub fn move_cursor(&mut self) -> Result<()> { + pub fn move_cursor(&mut self, kind: CmdKind) -> Result<()> { // calculate the desired position of the cursor let cursor = self .out @@ -130,7 +128,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { if self.layout.cursor == cursor { return Ok(()); } - if self.highlight_char() { + if self.highlight_char(kind) { let prompt_size = self.prompt_size; self.refresh(self.prompt, prompt_size, true, Info::NoHint)?; } else { @@ -205,10 +203,9 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { } } - fn highlight_char(&mut self) -> bool { + fn highlight_char(&mut self, kind: CmdKind) -> bool { if let Some(highlighter) = self.highlighter() { - let highlight_char = - highlighter.highlight_char(&self.line, self.line.pos(), self.forced_refresh); + let highlight_char = highlighter.highlight_char(&self.line, self.line.pos(), kind); if highlight_char { self.highlight_char = true; true @@ -240,12 +237,12 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { if corrected || self.has_hint() || msg.is_some() { // Force a refresh without hints to leave the previous // line as the user typed it after a newline. - self.refresh_line_with_msg(msg.as_deref())?; + self.refresh_line_with_msg(msg.as_deref(), CmdKind::ForcedRefresh)?; } } ValidationResult::Invalid(ref msg) => { if corrected || self.has_hint() || msg.is_some() { - self.refresh_line_with_msg(msg.as_deref())?; + self.refresh_line_with_msg(msg.as_deref(), CmdKind::Other)?; } } } @@ -266,21 +263,21 @@ impl<'out, 'prompt, H: Helper> Refresher for State<'out, 'prompt, H> { fn refresh_line(&mut self) -> Result<()> { let prompt_size = self.prompt_size; self.hint(); - self.highlight_char(); + self.highlight_char(CmdKind::Other); self.refresh(self.prompt, prompt_size, true, Info::Hint) } - fn refresh_line_with_msg(&mut self, msg: Option<&str>) -> Result<()> { + fn refresh_line_with_msg(&mut self, msg: Option<&str>, kind: CmdKind) -> Result<()> { let prompt_size = self.prompt_size; self.hint = None; - self.highlight_char(); + self.highlight_char(kind); self.refresh(self.prompt, prompt_size, true, Info::Msg(msg)) } fn refresh_prompt_and_line(&mut self, prompt: &str) -> Result<()> { let prompt_size = self.out.calculate_position(prompt, Position::default()); self.hint(); - self.highlight_char(); + self.highlight_char(CmdKind::Other); self.refresh(prompt, prompt_size, false, Info::Hint) } @@ -361,7 +358,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { && width != 0 // Ctrl-V + \t or \n ... && self.layout.cursor.col + width < self.out.get_columns() && (self.hint.is_none() && no_previous_hint) // TODO refresh only current line - && !self.highlight_char() + && !self.highlight_char(CmdKind::Other) { // Avoid a full update of the line in the trivial case. self.layout.cursor.col += width; @@ -454,7 +451,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { /// Move cursor on the left. pub fn edit_move_backward(&mut self, n: RepeatCount) -> Result<()> { if self.line.move_backward(n) { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } @@ -463,7 +460,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { /// Move cursor on the right. pub fn edit_move_forward(&mut self, n: RepeatCount) -> Result<()> { if self.line.move_forward(n) { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } @@ -472,7 +469,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { /// Move cursor to the start of the line. pub fn edit_move_home(&mut self) -> Result<()> { if self.line.move_home() { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } @@ -481,7 +478,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { /// Move cursor to the end of the line. pub fn edit_move_end(&mut self) -> Result<()> { if self.line.move_end() { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } @@ -490,16 +487,16 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { /// Move cursor to the start of the buffer. pub fn edit_move_buffer_start(&mut self) -> Result<()> { if self.line.move_buffer_start() { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } } /// Move cursor to the end of the buffer. - pub fn edit_move_buffer_end(&mut self) -> Result<()> { + pub fn edit_move_buffer_end(&mut self, kind: CmdKind) -> Result<()> { if self.line.move_buffer_end() { - self.move_cursor() + self.move_cursor(kind) } else { Ok(()) } @@ -565,7 +562,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { pub fn edit_move_to_prev_word(&mut self, word_def: Word, n: RepeatCount) -> Result<()> { if self.line.move_to_prev_word(word_def, n) { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } @@ -573,7 +570,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { pub fn edit_move_to_next_word(&mut self, at: At, word_def: Word, n: RepeatCount) -> Result<()> { if self.line.move_to_next_word(at, word_def, n) { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } @@ -582,7 +579,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { /// Moves the cursor to the same column in the line above pub fn edit_move_line_up(&mut self, n: RepeatCount) -> Result { if self.line.move_to_line_up(n) { - self.move_cursor()?; + self.move_cursor(CmdKind::MoveCursor)?; Ok(true) } else { Ok(false) @@ -592,7 +589,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { /// Moves the cursor to the same column in the line above pub fn edit_move_line_down(&mut self, n: RepeatCount) -> Result { if self.line.move_to_line_down(n) { - self.move_cursor()?; + self.move_cursor(CmdKind::MoveCursor)?; Ok(true) } else { Ok(false) @@ -601,7 +598,7 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> { pub fn edit_move_to(&mut self, cs: CharSearch, n: RepeatCount) -> Result<()> { if self.line.move_to(cs, n) { - self.move_cursor() + self.move_cursor(CmdKind::MoveCursor) } else { Ok(()) } @@ -765,7 +762,6 @@ pub fn init_state<'out, H: Helper>( ctx: Context::new(history), hint: Some(Box::new("hint".to_owned())), highlight_char: false, - forced_refresh: false, } } diff --git a/src/highlight.rs b/src/highlight.rs index a21459e0a..ffee71e76 100644 --- a/src/highlight.rs +++ b/src/highlight.rs @@ -4,6 +4,18 @@ use crate::config::CompletionType; use std::borrow::Cow::{self, Borrowed, Owned}; use std::cell::Cell; +/// Describe which kind of action has been triggering the call to `Highlighter`. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum CmdKind { + /// Cursor moved + MoveCursor, + /// Other action + Other, + /// Forced / final refresh (no auto-suggestion / hint, no matching bracket + /// highlighted, ...) + ForcedRefresh, +} + /// Syntax highlighter with [ANSI color](https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_(Select_Graphic_Rendition)_parameters). /// /// Currently, the highlighted version *must* have the same display width as @@ -47,13 +59,11 @@ pub trait Highlighter { } /// Tells if `line` needs to be highlighted when a specific char is typed or /// when cursor is moved under a specific char. - /// `forced` flag is `true` mainly when user presses Enter (i.e. transient - /// vs final highlight). /// /// Used to optimize refresh when a character is inserted or the cursor is /// moved. - fn highlight_char(&self, line: &str, pos: usize, forced: bool) -> bool { - let _ = (line, pos, forced); + fn highlight_char(&self, line: &str, pos: usize, kind: CmdKind) -> bool { + let _ = (line, pos, kind); false } } @@ -85,8 +95,8 @@ impl<'r, H: ?Sized + Highlighter> Highlighter for &'r H { (**self).highlight_candidate(candidate, completion) } - fn highlight_char(&self, line: &str, pos: usize, forced: bool) -> bool { - (**self).highlight_char(line, pos, forced) + fn highlight_char(&self, line: &str, pos: usize, kind: CmdKind) -> bool { + (**self).highlight_char(line, pos, kind) } } @@ -124,8 +134,8 @@ impl Highlighter for MatchingBracketHighlighter { Borrowed(line) } - fn highlight_char(&self, line: &str, pos: usize, forced: bool) -> bool { - if forced { + fn highlight_char(&self, line: &str, pos: usize, kind: CmdKind) -> bool { + if kind == CmdKind::ForcedRefresh { self.bracket.set(None); return false; } diff --git a/src/keymap.rs b/src/keymap.rs index 8564c163a..9bef72798 100644 --- a/src/keymap.rs +++ b/src/keymap.rs @@ -2,6 +2,7 @@ use log::debug; use super::Result; +use crate::highlight::CmdKind; use crate::keys::{KeyCode as K, KeyEvent, KeyEvent as E, Modifiers as M}; use crate::tty::{self, RawReader, Term, Terminal}; use crate::{Config, EditMode}; @@ -374,7 +375,7 @@ pub trait Refresher { /// cursor position, and number of columns of the terminal. fn refresh_line(&mut self) -> Result<()>; /// Same as [`refresh_line`] with a specific message instead of hint - fn refresh_line_with_msg(&mut self, msg: Option<&str>) -> Result<()>; + fn refresh_line_with_msg(&mut self, msg: Option<&str>, kind: CmdKind) -> Result<()>; /// Same as `refresh_line` but with a dynamic prompt. fn refresh_prompt_and_line(&mut self, prompt: &str) -> Result<()>; /// Vi only, switch to insert mode. diff --git a/src/lib.rs b/src/lib.rs index a40db34db..41675730a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,7 +59,7 @@ use crate::completion::{longest_common_prefix, Candidate, Completer}; pub use crate::config::{Behavior, ColorMode, CompletionType, Config, EditMode, HistoryDuplicates}; use crate::edit::State; use crate::error::ReadlineError; -use crate::highlight::Highlighter; +use crate::highlight::{CmdKind, Highlighter}; use crate::hint::Hinter; use crate::history::{DefaultHistory, History, SearchDirection}; pub use crate::keymap::{Anchor, At, CharSearch, Cmd, InputMode, Movement, RepeatCount, Word}; @@ -790,9 +790,7 @@ impl Editor { // Move to end, in case cursor was in the middle of the line, so that // next thing application prints goes after the input - s.forced_refresh = true; - s.edit_move_buffer_end()?; - s.forced_refresh = false; + s.edit_move_buffer_end(CmdKind::ForcedRefresh)?; if cfg!(windows) { let _ = original_mode; // silent warning diff --git a/src/tty/unix.rs b/src/tty/unix.rs index 2800af617..054b4f118 100644 --- a/src/tty/unix.rs +++ b/src/tty/unix.rs @@ -882,7 +882,7 @@ impl Receiver for Utf8 { self.valid = true; } - /// Called when an invalid_sequence is detected + /// Called when an invalid sequence is detected fn invalid_sequence(&mut self) { self.c = None; self.valid = false;