From 9d24c5efd4d603cc744d7092280e54179ee7e93f Mon Sep 17 00:00:00 2001 From: Piotr Kufel Date: Fri, 26 Jul 2024 18:15:33 -0700 Subject: [PATCH 1/3] Use get_history_path where appropriate --- .../nu-cli/src/commands/history/history_.rs | 151 ++++++++---------- crates/nu-cli/src/config_files.rs | 5 +- crates/nu-cli/src/repl.rs | 19 +-- crates/nu-path/src/helpers.rs | 14 ++ crates/nu-path/src/lib.rs | 5 +- src/config_files.rs | 7 +- src/run.rs | 6 +- 7 files changed, 106 insertions(+), 101 deletions(-) diff --git a/crates/nu-cli/src/commands/history/history_.rs b/crates/nu-cli/src/commands/history/history_.rs index c270cef003..6614635c48 100644 --- a/crates/nu-cli/src/commands/history/history_.rs +++ b/crates/nu-cli/src/commands/history/history_.rs @@ -5,6 +5,8 @@ use reedline::{ SqliteBackedHistory, }; +use crate::config_files::get_history_path; + #[derive(Clone)] pub struct History; @@ -44,89 +46,76 @@ impl Command for History { }; // todo for sqlite history this command should be an alias to `open ~/.config/nushell/history.sqlite3 | get history` - if let Some(config_path) = nu_path::config_dir() { - let clear = call.has_flag(engine_state, stack, "clear")?; - let long = call.has_flag(engine_state, stack, "long")?; - let signals = engine_state.signals().clone(); + let Some(history_path) = get_history_path(history.file_format) else { + return Err(ShellError::ConfigDirNotFound { span: Some(head) }); + }; + let clear = call.has_flag(engine_state, stack, "clear")?; + let long = call.has_flag(engine_state, stack, "long")?; + let signals = engine_state.signals().clone(); - let mut history_path = config_path; - history_path.push("nushell"); - match history.file_format { - HistoryFileFormat::Sqlite => { - history_path.push("history.sqlite3"); - } - HistoryFileFormat::PlainText => { - history_path.push("history.txt"); - } - } - - if clear { - let _ = std::fs::remove_file(history_path); - // TODO: FIXME also clear the auxiliary files when using sqlite - Ok(PipelineData::empty()) - } else { - let history_reader: Option> = match history.file_format { - HistoryFileFormat::Sqlite => { - SqliteBackedHistory::with_file(history_path.clone().into(), None, None) - .map(|inner| { - let boxed: Box = Box::new(inner); - boxed - }) - .ok() - } - - HistoryFileFormat::PlainText => FileBackedHistory::with_file( - history.max_size as usize, - history_path.clone().into(), - ) - .map(|inner| { - let boxed: Box = Box::new(inner); - boxed - }) - .ok(), - }; - - match history.file_format { - HistoryFileFormat::PlainText => Ok(history_reader - .and_then(|h| { - h.search(SearchQuery::everything(SearchDirection::Forward, None)) - .ok() - }) - .map(move |entries| { - entries.into_iter().enumerate().map(move |(idx, entry)| { - Value::record( - record! { - "command" => Value::string(entry.command_line, head), - "index" => Value::int(idx as i64, head), - }, - head, - ) - }) - }) - .ok_or(ShellError::FileNotFound { - file: history_path.display().to_string(), - span: head, - })? - .into_pipeline_data(head, signals)), - HistoryFileFormat::Sqlite => Ok(history_reader - .and_then(|h| { - h.search(SearchQuery::everything(SearchDirection::Forward, None)) - .ok() - }) - .map(move |entries| { - entries.into_iter().enumerate().map(move |(idx, entry)| { - create_history_record(idx, entry, long, head) - }) - }) - .ok_or(ShellError::FileNotFound { - file: history_path.display().to_string(), - span: head, - })? - .into_pipeline_data(head, signals)), - } - } + if clear { + let _ = std::fs::remove_file(history_path); + // TODO: FIXME also clear the auxiliary files when using sqlite + Ok(PipelineData::empty()) } else { - Err(ShellError::ConfigDirNotFound { span: Some(head) }) + let history_reader: Option> = match history.file_format { + HistoryFileFormat::Sqlite => { + SqliteBackedHistory::with_file(history_path.clone(), None, None) + .map(|inner| { + let boxed: Box = Box::new(inner); + boxed + }) + .ok() + } + + HistoryFileFormat::PlainText => { + FileBackedHistory::with_file(history.max_size as usize, history_path.clone()) + .map(|inner| { + let boxed: Box = Box::new(inner); + boxed + }) + .ok() + } + }; + match history.file_format { + HistoryFileFormat::PlainText => Ok(history_reader + .and_then(|h| { + h.search(SearchQuery::everything(SearchDirection::Forward, None)) + .ok() + }) + .map(move |entries| { + entries.into_iter().enumerate().map(move |(idx, entry)| { + Value::record( + record! { + "command" => Value::string(entry.command_line, head), + "index" => Value::int(idx as i64, head), + }, + head, + ) + }) + }) + .ok_or(ShellError::FileNotFound { + file: history_path.display().to_string(), + span: head, + })? + .into_pipeline_data(head, signals)), + HistoryFileFormat::Sqlite => Ok(history_reader + .and_then(|h| { + h.search(SearchQuery::everything(SearchDirection::Forward, None)) + .ok() + }) + .map(move |entries| { + entries + .into_iter() + .enumerate() + .map(move |(idx, entry)| create_history_record(idx, entry, long, head)) + }) + .ok_or(ShellError::FileNotFound { + file: history_path.display().to_string(), + span: head, + })? + .into_pipeline_data(head, signals)), + } } } diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 0a1226c304..5842b6b0ce 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -241,9 +241,8 @@ pub fn eval_config_contents( } } -pub(crate) fn get_history_path(storage_path: &str, mode: HistoryFileFormat) -> Option { - nu_path::config_dir().map(|mut history_path| { - history_path.push(storage_path); +pub(crate) fn get_history_path(mode: HistoryFileFormat) -> Option { + nu_path::nu_config_dir().map(|mut history_path| { history_path.push(match mode { HistoryFileFormat::PlainText => HISTORY_FILE_TXT, HistoryFileFormat::Sqlite => HISTORY_FILE_SQLITE, diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 28a3e2cec7..bc8df2d552 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -52,7 +52,6 @@ use sysinfo::System; pub fn evaluate_repl( engine_state: &mut EngineState, stack: Stack, - nushell_path: &str, prerun_command: Option>, load_std_lib: Option>, entire_start_time: Instant, @@ -99,7 +98,7 @@ pub fn evaluate_repl( unique_stack.add_env_var("LAST_EXIT_CODE".into(), Value::int(0, Span::unknown())); - let mut line_editor = get_line_editor(engine_state, nushell_path, use_color)?; + let mut line_editor = get_line_editor(engine_state, use_color)?; let temp_file = temp_dir().join(format!("{}.nu", uuid::Uuid::new_v4())); if let Some(s) = prerun_command { @@ -200,7 +199,7 @@ pub fn evaluate_repl( } Err(_) => { // line_editor is lost in the error case so reconstruct a new one - line_editor = get_line_editor(engine_state, nushell_path, use_color)?; + line_editor = get_line_editor(engine_state, use_color)?; } } } @@ -208,11 +207,7 @@ pub fn evaluate_repl( Ok(()) } -fn get_line_editor( - engine_state: &mut EngineState, - nushell_path: &str, - use_color: bool, -) -> Result { +fn get_line_editor(engine_state: &mut EngineState, use_color: bool) -> Result { let mut start_time = std::time::Instant::now(); let mut line_editor = Reedline::create(); @@ -223,7 +218,7 @@ fn get_line_editor( if let Some(history) = engine_state.history_config() { start_time = std::time::Instant::now(); - line_editor = setup_history(nushell_path, engine_state, line_editor, history)?; + line_editor = setup_history(engine_state, line_editor, history)?; perf!("setup history", start_time, use_color); } @@ -1037,7 +1032,6 @@ fn flush_engine_state_repl_buffer(engine_state: &mut EngineState, line_editor: & /// Setup history management for Reedline /// fn setup_history( - nushell_path: &str, engine_state: &mut EngineState, line_editor: Reedline, history: HistoryConfig, @@ -1049,7 +1043,7 @@ fn setup_history( None }; - if let Some(path) = crate::config_files::get_history_path(nushell_path, history.file_format) { + if let Some(path) = crate::config_files::get_history_path(history.file_format) { return update_line_editor_history( engine_state, path, @@ -1317,8 +1311,7 @@ fn trailing_slash_looks_like_path() { fn are_session_ids_in_sync() { let engine_state = &mut EngineState::new(); let history = engine_state.history_config().unwrap(); - let history_path = - crate::config_files::get_history_path("nushell", history.file_format).unwrap(); + let history_path = crate::config_files::get_history_path(history.file_format).unwrap(); let line_editor = reedline::Reedline::create(); let history_session_id = reedline::Reedline::create_history_session_id(); let line_editor = update_line_editor_history( diff --git a/crates/nu-path/src/helpers.rs b/crates/nu-path/src/helpers.rs index b36d59971e..5a0e3aadd6 100644 --- a/crates/nu-path/src/helpers.rs +++ b/crates/nu-path/src/helpers.rs @@ -1,5 +1,10 @@ use crate::AbsolutePathBuf; +#[deprecated( + note = "prefer using nu_config_dir() instead of config_dir() joined with NUSHELL_FOLDER" +)] +pub const NUSHELL_FOLDER: &str = "nushell"; + pub fn home_dir() -> Option { dirs::home_dir().and_then(|home| AbsolutePathBuf::try_from(home).ok()) } @@ -30,3 +35,12 @@ pub fn config_dir() -> Option { .or_else(|| dirs::config_dir().and_then(|path| AbsolutePathBuf::try_from(path).ok())) .map(|path| path.canonicalize().map(Into::into).unwrap_or(path)) } + +/// Return the nushell config directory. +pub fn nu_config_dir() -> Option { + config_dir().map(|mut p| { + #[allow(deprecated)] + p.push(NUSHELL_FOLDER); + p + }) +} diff --git a/crates/nu-path/src/lib.rs b/crates/nu-path/src/lib.rs index 524a6aed4e..51ca5ae812 100644 --- a/crates/nu-path/src/lib.rs +++ b/crates/nu-path/src/lib.rs @@ -11,7 +11,10 @@ mod trailing_slash; pub use components::components; pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path, locate_in_dirs}; -pub use helpers::{cache_dir, config_dir, data_dir, home_dir}; +pub use helpers::{cache_dir, config_dir, data_dir, home_dir, nu_config_dir}; pub use path::*; pub use tilde::expand_tilde; pub use trailing_slash::{has_trailing_slash, strip_trailing_slash}; + +#[allow(deprecated)] +pub use helpers::NUSHELL_FOLDER; diff --git a/src/config_files.rs b/src/config_files.rs index 1f9e4e9433..dcc4c5da76 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -3,6 +3,8 @@ use log::warn; use nu_cli::read_plugin_file; use nu_cli::{eval_config_contents, eval_source}; use nu_path::canonicalize_with; +#[allow(deprecated)] +use nu_path::NUSHELL_FOLDER; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, report_error, report_error_new, Config, ParseError, PipelineData, Spanned, @@ -17,7 +19,6 @@ use std::{ sync::Arc, }; -pub(crate) const NUSHELL_FOLDER: &str = "nushell"; const CONFIG_FILE: &str = "config.nu"; const ENV_FILE: &str = "env.nu"; const LOGINSHELL_FILE: &str = "login.nu"; @@ -50,6 +51,7 @@ pub(crate) fn read_config_file( } } } else if let Some(mut config_path) = nu_path::config_dir() { + #[allow(deprecated)] config_path.push(NUSHELL_FOLDER); // Create config directory if it does not exist @@ -135,6 +137,7 @@ pub(crate) fn read_loginshell_file(engine_state: &mut EngineState, stack: &mut S // read and execute loginshell file if exists if let Some(mut config_path) = nu_path::config_dir() { + #[allow(deprecated)] config_path.push(NUSHELL_FOLDER); config_path.push(LOGINSHELL_FILE); @@ -268,6 +271,7 @@ pub(crate) fn setup_config( ); let result = catch_unwind(AssertUnwindSafe(|| { #[cfg(feature = "plugin")] + #[allow(deprecated)] read_plugin_file(engine_state, plugin_file, NUSHELL_FOLDER); read_config_file(engine_state, stack, env_file, true); @@ -301,6 +305,7 @@ pub(crate) fn set_config_path( let config_path = match config_file { Some(s) => canonicalize_with(&s.item, cwd).ok(), None => nu_path::config_dir().map(|mut p| { + #[allow(deprecated)] p.push(NUSHELL_FOLDER); let mut p = canonicalize_with(&p, cwd).unwrap_or(p.into()); p.push(default_config_name); diff --git a/src/run.rs b/src/run.rs index 10a5043b25..652cc05b44 100644 --- a/src/run.rs +++ b/src/run.rs @@ -1,5 +1,4 @@ #[cfg(feature = "plugin")] -use crate::config_files::NUSHELL_FOLDER; use crate::{ command, config_files::{self, setup_config}, @@ -8,6 +7,8 @@ use log::trace; #[cfg(feature = "plugin")] use nu_cli::read_plugin_file; use nu_cli::{evaluate_commands, evaluate_file, evaluate_repl, EvaluateCommandsOpts}; +#[allow(deprecated)] +use nu_path::NUSHELL_FOLDER; use nu_protocol::{ engine::{EngineState, Stack}, report_error_new, PipelineData, Spanned, @@ -37,6 +38,7 @@ pub(crate) fn run_commands( // if the --no-config-file(-n) flag is passed, do not load plugin, env, or config files if parsed_nu_cli_args.no_config_file.is_none() { #[cfg(feature = "plugin")] + #[allow(deprecated)] read_plugin_file(engine_state, parsed_nu_cli_args.plugin_file, NUSHELL_FOLDER); perf!("read plugins", start_time, use_color); @@ -125,6 +127,7 @@ pub(crate) fn run_file( if parsed_nu_cli_args.no_config_file.is_none() { let start_time = std::time::Instant::now(); #[cfg(feature = "plugin")] + #[allow(deprecated)] read_plugin_file(engine_state, parsed_nu_cli_args.plugin_file, NUSHELL_FOLDER); perf!("read plugins", start_time, use_color); @@ -216,7 +219,6 @@ pub(crate) fn run_repl( let ret_val = evaluate_repl( engine_state, stack, - config_files::NUSHELL_FOLDER, parsed_nu_cli_args.execute, parsed_nu_cli_args.no_std_lib, entire_start_time, From 75dc8a6ed1536979b9d3198e0252c3fec99e54af Mon Sep 17 00:00:00 2001 From: Piotr Kufel Date: Sat, 27 Jul 2024 17:44:12 -0700 Subject: [PATCH 2/3] Remove uses of NUSHELL_FOLDER --- crates/nu-cli/src/config_files.rs | 37 ++++++----------------- crates/nu-path/src/helpers.rs | 8 +---- crates/nu-path/src/lib.rs | 3 -- crates/nu-protocol/src/config/reedline.rs | 10 ++++++ crates/nu-protocol/src/eval_const.rs | 7 ++--- src/config_files.rs | 18 +++-------- src/run.rs | 8 ++--- 7 files changed, 29 insertions(+), 62 deletions(-) diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 5842b6b0ce..a74fcc2502 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -16,15 +16,8 @@ const PLUGIN_FILE: &str = "plugin.msgpackz"; #[cfg(feature = "plugin")] const OLD_PLUGIN_FILE: &str = "plugin.nu"; -const HISTORY_FILE_TXT: &str = "history.txt"; -const HISTORY_FILE_SQLITE: &str = "history.sqlite3"; - #[cfg(feature = "plugin")] -pub fn read_plugin_file( - engine_state: &mut EngineState, - plugin_file: Option>, - storage_path: &str, -) { +pub fn read_plugin_file(engine_state: &mut EngineState, plugin_file: Option>) { use nu_protocol::ShellError; use std::path::Path; @@ -52,7 +45,7 @@ pub fn read_plugin_file( let mut start_time = std::time::Instant::now(); // Reading signatures from plugin registry file // The plugin.msgpackz file stores the parsed signature collected from each registered plugin - add_plugin_file(engine_state, plugin_file.clone(), storage_path); + add_plugin_file(engine_state, plugin_file.clone()); perf!( "add plugin file to engine_state", start_time, @@ -70,8 +63,7 @@ pub fn read_plugin_file( log::warn!("Plugin file not found: {}", plugin_path.display()); // Try migration of an old plugin file if this wasn't a custom plugin file - if plugin_file.is_none() && migrate_old_plugin_file(engine_state, storage_path) - { + if plugin_file.is_none() && migrate_old_plugin_file(engine_state) { let Ok(file) = std::fs::File::open(&plugin_path) else { log::warn!("Failed to load newly migrated plugin file"); return; @@ -159,11 +151,7 @@ pub fn read_plugin_file( } #[cfg(feature = "plugin")] -pub fn add_plugin_file( - engine_state: &mut EngineState, - plugin_file: Option>, - storage_path: &str, -) { +pub fn add_plugin_file(engine_state: &mut EngineState, plugin_file: Option>) { use std::path::Path; let working_set = StateWorkingSet::new(engine_state); @@ -189,9 +177,8 @@ pub fn add_plugin_file( ), ); } - } else if let Some(mut plugin_path) = nu_path::config_dir() { + } else if let Some(plugin_path) = nu_path::nu_config_dir() { // Path to store plugins signatures - plugin_path.push(storage_path); let mut plugin_path = canonicalize_with(&plugin_path, &cwd).unwrap_or(plugin_path.into()); plugin_path.push(PLUGIN_FILE); @@ -243,16 +230,13 @@ pub fn eval_config_contents( pub(crate) fn get_history_path(mode: HistoryFileFormat) -> Option { nu_path::nu_config_dir().map(|mut history_path| { - history_path.push(match mode { - HistoryFileFormat::PlainText => HISTORY_FILE_TXT, - HistoryFileFormat::Sqlite => HISTORY_FILE_SQLITE, - }); + history_path.push(mode.default_file_name()); history_path.into() }) } #[cfg(feature = "plugin")] -pub fn migrate_old_plugin_file(engine_state: &EngineState, storage_path: &str) -> bool { +pub fn migrate_old_plugin_file(engine_state: &EngineState) -> bool { use nu_protocol::{ PluginExample, PluginIdentity, PluginRegistryItem, PluginRegistryItemData, PluginSignature, ShellError, @@ -265,10 +249,9 @@ pub fn migrate_old_plugin_file(engine_state: &EngineState, storage_path: &str) - return false; }; - let Some(config_dir) = nu_path::config_dir().and_then(|mut dir| { - dir.push(storage_path); - nu_path::canonicalize_with(dir, &cwd).ok() - }) else { + let Some(config_dir) = + nu_path::nu_config_dir().and_then(|dir| nu_path::canonicalize_with(dir, &cwd).ok()) + else { return false; }; diff --git a/crates/nu-path/src/helpers.rs b/crates/nu-path/src/helpers.rs index 5a0e3aadd6..fc3113a32f 100644 --- a/crates/nu-path/src/helpers.rs +++ b/crates/nu-path/src/helpers.rs @@ -1,10 +1,5 @@ use crate::AbsolutePathBuf; -#[deprecated( - note = "prefer using nu_config_dir() instead of config_dir() joined with NUSHELL_FOLDER" -)] -pub const NUSHELL_FOLDER: &str = "nushell"; - pub fn home_dir() -> Option { dirs::home_dir().and_then(|home| AbsolutePathBuf::try_from(home).ok()) } @@ -39,8 +34,7 @@ pub fn config_dir() -> Option { /// Return the nushell config directory. pub fn nu_config_dir() -> Option { config_dir().map(|mut p| { - #[allow(deprecated)] - p.push(NUSHELL_FOLDER); + p.push("nushell"); p }) } diff --git a/crates/nu-path/src/lib.rs b/crates/nu-path/src/lib.rs index 51ca5ae812..d85f7bebd4 100644 --- a/crates/nu-path/src/lib.rs +++ b/crates/nu-path/src/lib.rs @@ -15,6 +15,3 @@ pub use helpers::{cache_dir, config_dir, data_dir, home_dir, nu_config_dir}; pub use path::*; pub use tilde::expand_tilde; pub use trailing_slash::{has_trailing_slash, strip_trailing_slash}; - -#[allow(deprecated)] -pub use helpers::NUSHELL_FOLDER; diff --git a/crates/nu-protocol/src/config/reedline.rs b/crates/nu-protocol/src/config/reedline.rs index 7032e74087..86177812cb 100644 --- a/crates/nu-protocol/src/config/reedline.rs +++ b/crates/nu-protocol/src/config/reedline.rs @@ -79,6 +79,16 @@ pub enum HistoryFileFormat { PlainText, } +impl HistoryFileFormat { + pub fn default_file_name(self) -> std::path::PathBuf { + match self { + HistoryFileFormat::PlainText => "history.txt", + HistoryFileFormat::Sqlite => "history.sqlite3", + } + .into() + } +} + impl FromStr for HistoryFileFormat { type Err = &'static str; diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 1ee7802799..106bd00c7f 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -33,11 +33,8 @@ pub(crate) fn create_nu_constant(engine_state: &EngineState, span: Span) -> Valu let mut record = Record::new(); - let config_path = match nu_path::config_dir() { - Some(mut path) => { - path.push("nushell"); - Ok(canonicalize_path(engine_state, path.as_ref())) - } + let config_path = match nu_path::nu_config_dir() { + Some(path) => Ok(canonicalize_path(engine_state, path.as_ref())), None => Err(Value::error( ShellError::ConfigDirNotFound { span: Some(span) }, span, diff --git a/src/config_files.rs b/src/config_files.rs index dcc4c5da76..d36c18d501 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -3,8 +3,6 @@ use log::warn; use nu_cli::read_plugin_file; use nu_cli::{eval_config_contents, eval_source}; use nu_path::canonicalize_with; -#[allow(deprecated)] -use nu_path::NUSHELL_FOLDER; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, report_error, report_error_new, Config, ParseError, PipelineData, Spanned, @@ -50,10 +48,7 @@ pub(crate) fn read_config_file( report_error(&working_set, &e); } } - } else if let Some(mut config_path) = nu_path::config_dir() { - #[allow(deprecated)] - config_path.push(NUSHELL_FOLDER); - + } else if let Some(mut config_path) = nu_path::nu_config_dir() { // Create config directory if it does not exist if !config_path.exists() { if let Err(err) = std::fs::create_dir_all(&config_path) { @@ -136,9 +131,7 @@ pub(crate) fn read_loginshell_file(engine_state: &mut EngineState, stack: &mut S ); // read and execute loginshell file if exists - if let Some(mut config_path) = nu_path::config_dir() { - #[allow(deprecated)] - config_path.push(NUSHELL_FOLDER); + if let Some(mut config_path) = nu_path::nu_config_dir() { config_path.push(LOGINSHELL_FILE); warn!("loginshell_file: {}", config_path.display()); @@ -271,8 +264,7 @@ pub(crate) fn setup_config( ); let result = catch_unwind(AssertUnwindSafe(|| { #[cfg(feature = "plugin")] - #[allow(deprecated)] - read_plugin_file(engine_state, plugin_file, NUSHELL_FOLDER); + read_plugin_file(engine_state, plugin_file); read_config_file(engine_state, stack, env_file, true); read_config_file(engine_state, stack, config_file, false); @@ -304,9 +296,7 @@ pub(crate) fn set_config_path( ); let config_path = match config_file { Some(s) => canonicalize_with(&s.item, cwd).ok(), - None => nu_path::config_dir().map(|mut p| { - #[allow(deprecated)] - p.push(NUSHELL_FOLDER); + None => nu_path::nu_config_dir().map(|p| { let mut p = canonicalize_with(&p, cwd).unwrap_or(p.into()); p.push(default_config_name); canonicalize_with(&p, cwd).unwrap_or(p) diff --git a/src/run.rs b/src/run.rs index 652cc05b44..1ec0c3eb7e 100644 --- a/src/run.rs +++ b/src/run.rs @@ -7,8 +7,6 @@ use log::trace; #[cfg(feature = "plugin")] use nu_cli::read_plugin_file; use nu_cli::{evaluate_commands, evaluate_file, evaluate_repl, EvaluateCommandsOpts}; -#[allow(deprecated)] -use nu_path::NUSHELL_FOLDER; use nu_protocol::{ engine::{EngineState, Stack}, report_error_new, PipelineData, Spanned, @@ -38,8 +36,7 @@ pub(crate) fn run_commands( // if the --no-config-file(-n) flag is passed, do not load plugin, env, or config files if parsed_nu_cli_args.no_config_file.is_none() { #[cfg(feature = "plugin")] - #[allow(deprecated)] - read_plugin_file(engine_state, parsed_nu_cli_args.plugin_file, NUSHELL_FOLDER); + read_plugin_file(engine_state, parsed_nu_cli_args.plugin_file); perf!("read plugins", start_time, use_color); @@ -127,8 +124,7 @@ pub(crate) fn run_file( if parsed_nu_cli_args.no_config_file.is_none() { let start_time = std::time::Instant::now(); #[cfg(feature = "plugin")] - #[allow(deprecated)] - read_plugin_file(engine_state, parsed_nu_cli_args.plugin_file, NUSHELL_FOLDER); + read_plugin_file(engine_state, parsed_nu_cli_args.plugin_file); perf!("read plugins", start_time, use_color); let start_time = std::time::Instant::now(); From 846dd480ecd3616979680ede78008128b1c30389 Mon Sep 17 00:00:00 2001 From: Piotr Kufel Date: Sat, 27 Jul 2024 18:08:43 -0700 Subject: [PATCH 3/3] Plumb history path to $nu --- crates/nu-protocol/src/config/helper.rs | 30 +++++++- crates/nu-protocol/src/config/mod.rs | 71 ++++++++++++++++++- crates/nu-protocol/src/engine/engine_state.rs | 6 +- crates/nu-protocol/src/eval_const.rs | 48 +++++++------ src/run.rs | 1 - tests/repl/test_config_path.rs | 1 + 6 files changed, 133 insertions(+), 24 deletions(-) diff --git a/crates/nu-protocol/src/config/helper.rs b/crates/nu-protocol/src/config/helper.rs index 7033a1ac11..c226da23b4 100644 --- a/crates/nu-protocol/src/config/helper.rs +++ b/crates/nu-protocol/src/config/helper.rs @@ -1,5 +1,5 @@ use crate::{Record, ShellError, Span, Value}; -use std::{collections::HashMap, fmt::Display, str::FromStr}; +use std::{collections::HashMap, fmt::Display, str::FromStr, sync::Arc}; pub(super) trait ReconstructVal { fn reconstruct_value(&self, span: Span) -> Value; @@ -88,6 +88,34 @@ pub(super) fn process_int_config( } } +pub(super) fn process_opt_str_config( + value: &mut Value, + errors: &mut Vec, + config_point: &mut Option>, +) { + if value.is_nothing() { + *config_point = None; + return; + } + match value.coerce_str() { + Ok(s) => *config_point = Some(s.into()), + Err(e) => { + errors.push(ShellError::GenericError { + error: "Error while applying config changes".into(), + msg: e.to_string(), + span: Some(value.span()), + help: Some("This value will be ignored.".into()), + inner: vec![], + }); + // Reconstruct + *value = match config_point { + Some(s) => Value::string(s.as_ref(), value.span()), + None => Value::nothing(value.span()), + } + } + } +} + pub(super) fn report_invalid_key(keys: &[&str], span: Span, errors: &mut Vec) { // Because Value::Record discards all of the spans of its // column names (by storing them as Strings), the key name cannot be provided diff --git a/crates/nu-protocol/src/config/mod.rs b/crates/nu-protocol/src/config/mod.rs index eda3d9a15e..45a484d31d 100644 --- a/crates/nu-protocol/src/config/mod.rs +++ b/crates/nu-protocol/src/config/mod.rs @@ -8,8 +8,12 @@ use self::table::*; use crate::engine::Closure; use crate::{record, ShellError, Span, Value}; +use serde::Deserializer; +use serde::Serializer; use serde::{Deserialize, Serialize}; use std::collections::HashMap; +use std::path::MAIN_SEPARATOR; +use std::sync::Arc; pub use self::completer::CompletionAlgorithm; pub use self::helper::extract_value; @@ -29,12 +33,63 @@ mod plugin_gc; mod reedline; mod table; -#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +fn deserialize_config_path<'de, D>(deserializer: D) -> Result>, D::Error> +where + D: Deserializer<'de>, +{ + let s: Option = Deserialize::deserialize(deserializer)?; + Ok(s.map(|s| s.into())) +} + +fn serialize_config_path(v: &Option>, serializer: S) -> Result +where + S: Serializer, +{ + match v { + None => serializer.serialize_none(), + Some(s) => serializer.serialize_str(s), + } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct HistoryConfig { pub max_size: i64, pub sync_on_enter: bool, pub file_format: HistoryFileFormat, pub isolation: bool, + + // History path. Can be either a directory (ending with a path separator), in which case a + // default file name will be appended - or a full file path. + // + // PathBuf would be more correct, but it causes conversion issues down the line. String is more + // strict in terms of characters, but less strict in terms of semantics. We use Arc for + // cheap clone. + #[serde( + serialize_with = "serialize_config_path", + deserialize_with = "deserialize_config_path" + )] + path: Option>, +} + +impl HistoryConfig { + #[allow(clippy::question_mark)] + pub fn file_path(&self) -> Option { + let Some(path) = &self.path else { + return None; + }; + if path.ends_with(MAIN_SEPARATOR) { + let mut path = path.to_string(); + path.push_str(match self.file_format { + HistoryFileFormat::PlainText => "history.txt", + HistoryFileFormat::Sqlite => "history.sqlite3", + }); + Some(path) + } else if !path.is_empty() { + Some(path.to_string()) + } else { + None + } + } } impl Default for HistoryConfig { @@ -44,6 +99,17 @@ impl Default for HistoryConfig { sync_on_enter: true, file_format: HistoryFileFormat::PlainText, isolation: false, + // TODO: This reading an env var is not great. Ideally we'd plumb config location + // explicitly, but there's already lots of uses of Config::default and converting them + // all is not trivial. + path: nu_path::nu_config_dir().and_then(|dir| { + let s = dir.into_os_string().into_string(); + s.map(|mut s| { + s.push(MAIN_SEPARATOR); + Arc::from(s) + }) + .ok() + }), } } } @@ -299,6 +365,9 @@ impl Value { value, &mut errors); } + "path" => { + process_opt_str_config(value, &mut errors, &mut history.path); + } _ => { report_invalid_key(&[key, key2], span, &mut errors); return false; diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index bd049bbb2d..51f8879ce5 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -762,7 +762,11 @@ impl EngineState { /// Returns the configuration settings for command history or `None` if history is disabled pub fn history_config(&self) -> Option { - self.history_enabled.then(|| self.config.history) + self.history_enabled.then(|| self.config.history.clone()) + } + + pub fn history_file_path(&self) -> Option { + self.config.history.file_path() } pub fn get_var(&self, var_id: VarId) -> &Variable { diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 106bd00c7f..f48a5d70c2 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -7,30 +7,46 @@ use crate::{ debugger::{DebugContext, WithoutDebug}, engine::{EngineState, StateWorkingSet}, eval_base::Eval, - record, Config, HistoryFileFormat, PipelineData, Record, ShellError, Span, Value, VarId, + record, Config, PipelineData, Record, ShellError, Span, Value, VarId, }; use nu_system::os_info::{get_kernel_version, get_os_arch, get_os_family, get_os_name}; use std::{ + io, path::{Path, PathBuf}, sync::Arc, }; /// Create a Value for `$nu`. pub(crate) fn create_nu_constant(engine_state: &EngineState, span: Span) -> Value { - fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf { + fn canonicalize_path_fallible(engine_state: &EngineState, path: &Path) -> io::Result { #[allow(deprecated)] let cwd = engine_state.current_work_dir(); - if path.exists() { - match nu_path::canonicalize_with(path, cwd) { - Ok(canon_path) => canon_path, - Err(_) => path.to_owned(), + match nu_path::canonicalize_with(path, &cwd) { + Err(e) if e.kind() == io::ErrorKind::NotFound => { + // If the path does not exist, try to canonicalize the parent (directory) alone. + // Some tests (and perhaps, users) rely on path to non-existent files being + // canonicalized to some extent. This works for the most part due to how those + // paths happen to be constructed, but in some cases there is no intermediate + // canonicalization and we have to do it here. + match (path.parent(), path.file_name()) { + (Some(dir), Some(file)) => { + let mut path = nu_path::canonicalize_with(dir, cwd)?; + path.push(file); + Ok(path) + } + _ => Err(e), + } } - } else { - path.to_owned() + Err(e) => Err(e), + Ok(p) => Ok(p), } } + fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf { + canonicalize_path_fallible(engine_state, path).unwrap_or_else(|_| path.to_owned()) + } + let mut record = Record::new(); let config_path = match nu_path::nu_config_dir() { @@ -85,18 +101,10 @@ pub(crate) fn create_nu_constant(engine_state: &EngineState, span: Span) -> Valu record.push( "history-path", - config_path.clone().map_or_else( - |e| e, - |mut path| { - match engine_state.config.history.file_format { - HistoryFileFormat::Sqlite => { - path.push("history.sqlite3"); - } - HistoryFileFormat::PlainText => { - path.push("history.txt"); - } - } - let canon_hist_path = canonicalize_path(engine_state, &path); + engine_state.history_file_path().map_or_else( + || Value::error(ShellError::ConfigDirNotFound { span: Some(span) }, span), + |path| { + let canon_hist_path = canonicalize_path(engine_state, Path::new(&path)); Value::string(canon_hist_path.to_string_lossy(), span) }, ), diff --git a/src/run.rs b/src/run.rs index 1ec0c3eb7e..2dab3295da 100644 --- a/src/run.rs +++ b/src/run.rs @@ -1,4 +1,3 @@ -#[cfg(feature = "plugin")] use crate::{ command, config_files::{self, setup_config}, diff --git a/tests/repl/test_config_path.rs b/tests/repl/test_config_path.rs index 6bee043765..49038fc88a 100644 --- a/tests/repl/test_config_path.rs +++ b/tests/repl/test_config_path.rs @@ -151,6 +151,7 @@ fn test_default_config_path() { fn test_default_symlinked_config_path_empty() { Playground::setup("symlinked_empty_config_dir", |_, playground| { let config_dir_nushell = setup_fake_config(playground); + println!("config_dir_nushell = {}", config_dir_nushell.display()); test_config_path_helper(playground, config_dir_nushell); }); }