From 0986c61a5d95ada321c345434b0e489487336767 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Thu, 31 Mar 2022 23:25:48 +0200 Subject: [PATCH] Lift line editor construction out of loop (#5041) Enables the use of some features on reedline - Keeping the line when clearing the screen with `Ctrl-L` - Using the internal cut buffer between lines - Submitting external commands via keybinding and keeping the line Additional effect: Keep the history around and do basic syncs (performance improvement minimal as session changes have to be read and written) Additional change: Give the option to defer writing/rereading the history file to the closing of the session ($config.sync_history_on_enter) --- Cargo.lock | 2 +- crates/nu-cli/src/repl.rs | 90 ++++++++++++++++------------ crates/nu-protocol/src/config.rs | 9 +++ docs/sample_config/default_config.nu | 3 +- 4 files changed, 63 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4172e3852..0e5c69e09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3428,7 +3428,7 @@ dependencies = [ [[package]] name = "reedline" version = "0.3.1" -source = "git+https://github.com/nushell/reedline?branch=main#bc18ba3260604172700bf5996f66635ee428d625" +source = "git+https://github.com/nushell/reedline?branch=main#0f92985c69efca2567ad40ebc0381864e91aff8c" dependencies = [ "chrono", "crossterm", diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index c463277c3..3b894f5db 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -104,6 +104,45 @@ pub fn evaluate_repl( }, ); + if is_perf_true { + info!( + "load config initially {}:{}:{}", + file!(), + line!(), + column!() + ); + } + + // Get the config once for the history `max_history_size` + // Updating that will not be possible in one session + let mut config = match stack.get_config() { + Ok(config) => config, + Err(e) => { + let working_set = StateWorkingSet::new(engine_state); + + report_error(&working_set, &e); + Config::default() + } + }; + + if is_perf_true { + info!("setup reedline {}:{}:{}", file!(), line!(), column!()); + } + let mut line_editor = Reedline::create().into_diagnostic()?; + if let Some(history_path) = history_path.as_deref() { + if is_perf_true { + info!("setup history {}:{}:{}", file!(), line!(), column!()); + } + let history = Box::new( + FileBackedHistory::with_file( + config.max_history_size as usize, + history_path.to_path_buf(), + ) + .into_diagnostic()?, + ); + line_editor = line_editor.with_history(history).into_diagnostic()?; + }; + loop { if is_perf_true { info!( @@ -114,7 +153,7 @@ pub fn evaluate_repl( ); } - let config = match stack.get_config() { + config = match stack.get_config() { Ok(config) => config, Err(e) => { let working_set = StateWorkingSet::new(engine_state); @@ -123,18 +162,14 @@ pub fn evaluate_repl( Config::default() } }; + let color_hm = get_color_config(&config); //Reset the ctrl-c handler if let Some(ctrlc) = &mut engine_state.ctrlc { ctrlc.store(false, Ordering::SeqCst); } - if is_perf_true { - info!("setup line editor {}:{}:{}", file!(), line!(), column!()); - } - - let mut line_editor = Reedline::create() - .into_diagnostic()? + line_editor = line_editor .with_highlighter(Box::new(NuHighlighter { engine_state: engine_state.clone(), config: config.clone(), @@ -143,6 +178,9 @@ pub fn evaluate_repl( .with_validator(Box::new(NuValidator { engine_state: engine_state.clone(), })) + .with_hinter(Box::new( + DefaultHinter::default().with_style(color_hm["hints"]), + )) .with_completer(Box::new(NuCompleter::new( engine_state.clone(), stack.clone(), @@ -153,7 +191,7 @@ pub fn evaluate_repl( .with_ansi_colors(config.use_ansi_coloring); if is_perf_true { - info!("setup reedline {}:{}:{}", file!(), line!(), column!()); + info!("update reedline {}:{}:{}", file!(), line!(), column!()); } line_editor = add_completion_menu(line_editor, &config); @@ -168,38 +206,12 @@ pub fn evaluate_repl( //FIXME: if config.use_ansi_coloring is false then we should // turn off the hinter but I don't see any way to do that yet. - let color_hm = get_color_config(&config); - - if is_perf_true { - info!( - "setup history and hinter {}:{}:{}", - file!(), - line!(), - column!() - ); - } - - line_editor = if let Some(history_path) = history_path.clone() { - let history = std::fs::read_to_string(&history_path); - if history.is_ok() { - line_editor - .with_hinter(Box::new( - DefaultHinter::default().with_style(color_hm["hints"]), - )) - .with_history(Box::new( - FileBackedHistory::with_file( - config.max_history_size as usize, - history_path.clone(), - ) - .into_diagnostic()?, - )) - .into_diagnostic()? - } else { - line_editor + if config.sync_history_on_enter { + if is_perf_true { + info!("sync history {}:{}:{}", file!(), line!(), column!()); } - } else { - line_editor - }; + line_editor.sync_history().into_diagnostic()?; + } if is_perf_true { info!("setup keybindings {}:{}:{}", file!(), line!(), column!()); diff --git a/crates/nu-protocol/src/config.rs b/crates/nu-protocol/src/config.rs index b74e98d63..d237cbc02 100644 --- a/crates/nu-protocol/src/config.rs +++ b/crates/nu-protocol/src/config.rs @@ -29,6 +29,7 @@ pub struct Config { pub partial_completions: bool, pub edit_mode: String, pub max_history_size: i64, + pub sync_history_on_enter: bool, pub log_level: String, pub menu_config: HashMap, pub keybindings: Vec, @@ -54,6 +55,7 @@ impl Default for Config { partial_completions: true, edit_mode: "emacs".into(), max_history_size: 1000, + sync_history_on_enter: true, log_level: String::new(), menu_config: HashMap::new(), history_config: HashMap::new(), @@ -199,6 +201,13 @@ impl Value { eprintln!("$config.max_history_size is not an integer") } } + "sync_history_on_enter" => { + if let Ok(b) = value.as_bool() { + config.sync_history_on_enter = b; + } else { + eprintln!("$config.sync_history_on_enter is not a bool") + } + } "log_level" => { if let Ok(v) = value.as_string() { config.log_level = v.to_lowercase(); diff --git a/docs/sample_config/default_config.nu b/docs/sample_config/default_config.nu index 737302e84..1f117251f 100644 --- a/docs/sample_config/default_config.nu +++ b/docs/sample_config/default_config.nu @@ -196,7 +196,8 @@ let $config = { use_ansi_coloring: true filesize_format: "auto" # b, kb, kib, mb, mib, gb, gib, tb, tib, pb, pib, eb, eib, zb, zib, auto edit_mode: emacs # emacs, vi - max_history_size: 10000 + max_history_size: 10000 # Session has to be reloaded for this to take effect + sync_history_on_enter: true # Enable to share the history between multiple sessions, else you have to close the session to persist history to file menu_config: { columns: 4 col_width: 20 # Optional value. If missing all the screen width is used to calculate column width