Plumb history path to $nu

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<ShellError>,
+    config_point: &mut Option<Arc<str>>,
+) {
+    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<ShellError>) {
     // Because Value::Record discards all of the spans of its
     // column names (by storing them as Strings), the key name cannot be provided

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<Option<Arc<str>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let s: Option<String> = Deserialize::deserialize(deserializer)?;
+    Ok(s.map(|s| s.into()))
+}
+
+fn serialize_config_path<S>(v: &Option<Arc<str>>, serializer: S) -> Result<S::Ok, S::Error>
+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<str> for
+    // cheap clone.
+    #[serde(
+        serialize_with = "serialize_config_path",
+        deserialize_with = "deserialize_config_path"
+    )]
+    path: Option<Arc<str>>,
+}
+
+impl HistoryConfig {
+    #[allow(clippy::question_mark)]
+    pub fn file_path(&self) -> Option<String> {
+        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;

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<HistoryConfig> {
-        self.history_enabled.then(|| self.config.history)
+        self.history_enabled.then(|| self.config.history.clone())
+    }
+
+    pub fn history_file_path(&self) -> Option<String> {
+        self.config.history.file_path()
     }
 
     pub fn get_var(&self, var_id: VarId) -> &Variable {

crates/nu-protocol/src/eval_const.rs

@@ -7,29 +7,45 @@ 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<PathBuf> {
         #[allow(deprecated)]
         let cwd = engine_state.current_work_dir();
 
-        if path.exists() {
+        match nu_path::canonicalize_with(path, &cwd) {
-            match nu_path::canonicalize_with(path, cwd) {
+            Err(e) if e.kind() == io::ErrorKind::NotFound => {
-                Ok(canon_path) => canon_path,
+                // If the path does not exist, try to canonicalize the parent (directory) alone.
-                Err(_) => path.to_owned(),
+                // 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)
                     }
-        } else {
+                    _ => Err(e),
-            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();
 
@@ -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(
+        engine_state.history_file_path().map_or_else(
-            |e| e,
+            || Value::error(ShellError::ConfigDirNotFound { span: Some(span) }, span),
-            |mut path| {
+            |path| {
-                match engine_state.config.history.file_format {
+                let canon_hist_path = canonicalize_path(engine_state, Path::new(&path));
-                    HistoryFileFormat::Sqlite => {
-                        path.push("history.sqlite3");
-                    }
-                    HistoryFileFormat::PlainText => {
-                        path.push("history.txt");
-                    }
-                }
-                let canon_hist_path = canonicalize_path(engine_state, &path);
                 Value::string(canon_hist_path.to_string_lossy(), span)
             },
         ),

src/run.rs

@@ -1,4 +1,3 @@
-#[cfg(feature = "plugin")]
 use crate::{
     command,
     config_files::{self, setup_config},

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