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); }); }