From 975ebba33fcc0bb420ab686ec6f6544379ec30ce Mon Sep 17 00:00:00 2001 From: Thomas Buckley-Houston Date: Tue, 26 Nov 2024 14:05:59 +0100 Subject: [PATCH] Use $XDG_DATA_DIR as default history path Fixes #10100 Consensus was that the history file was not config and so should not live in the config directory by default. Also includes an automated "migration" that moves the old history file to the new path: only if the old file exists and there is no file in the new path. Notes: * Changes `nu_path::data_dir()` to `nu_path::nu_data_dir()` so that it returns the data dir with `/nushell` appended. This was already being doing by callers of `nu_path::data_dir()` so I just refactored it. * `history_import.rs` tests now set `XDG_CONFIG_HOME` _and_ `XDG_DATA_HOME`, there may be other tests that will come to need both. But currently all tests pass. --- .../nu-cli/src/commands/history/history_.rs | 2 +- .../src/commands/history/history_import.rs | 2 +- .../nu-cli/tests/commands/history_import.rs | 23 ++++++----- crates/nu-path/src/helpers.rs | 15 ++++--- crates/nu-path/src/lib.rs | 2 +- crates/nu-protocol/src/config/history.rs | 40 ++++++++++++++++++- crates/nu-protocol/src/errors/shell_error.rs | 15 +++++++ crates/nu-protocol/src/eval_const.rs | 5 +-- src/main.rs | 3 +- 9 files changed, 80 insertions(+), 27 deletions(-) diff --git a/crates/nu-cli/src/commands/history/history_.rs b/crates/nu-cli/src/commands/history/history_.rs index 40d951966e..4f0b4e102e 100644 --- a/crates/nu-cli/src/commands/history/history_.rs +++ b/crates/nu-cli/src/commands/history/history_.rs @@ -46,7 +46,7 @@ impl Command for History { }; // todo for sqlite history this command should be an alias to `open ~/.config/nushell/history.sqlite3 | get history` let Some(history_path) = history.file_path() else { - return Err(ShellError::ConfigDirNotFound { span: Some(head) }); + return Err(ShellError::DataDirNotFound { span: Some(head) }); }; if call.has_flag(engine_state, stack, "clear")? { diff --git a/crates/nu-cli/src/commands/history/history_import.rs b/crates/nu-cli/src/commands/history/history_import.rs index d56bc1da03..dc1ca68cfc 100644 --- a/crates/nu-cli/src/commands/history/history_import.rs +++ b/crates/nu-cli/src/commands/history/history_import.rs @@ -74,7 +74,7 @@ Note that history item IDs are ignored when importing from file."# return ok; }; let Some(current_history_path) = history.file_path() else { - return Err(ShellError::ConfigDirNotFound { + return Err(ShellError::DataDirNotFound { span: Some(call.head), }); }; diff --git a/crates/nu-cli/tests/commands/history_import.rs b/crates/nu-cli/tests/commands/history_import.rs index 79ebdc1cfc..03b900406e 100644 --- a/crates/nu-cli/tests/commands/history_import.rs +++ b/crates/nu-cli/tests/commands/history_import.rs @@ -8,38 +8,39 @@ use rstest::rstest; use tempfile::TempDir; struct Test { - cfg_dir: TempDir, + cfg_and_data_dir: TempDir, } impl Test { fn new(history_format: &'static str) -> Self { - let cfg_dir = tempfile::Builder::new() + let cfg_and_data_dir = tempfile::Builder::new() .prefix("history_import_test") .tempdir() .unwrap(); // Assigning to $env.config.history.file_format seems to work only in startup // configuration. std::fs::write( - cfg_dir.path().join("env.nu"), + cfg_and_data_dir.path().join("env.nu"), format!("$env.config.history.file_format = {history_format:?}"), ) .unwrap(); - Self { cfg_dir } + Self { cfg_and_data_dir } } fn nu(&self, cmd: impl AsRef) -> Outcome { - let env = [( - "XDG_CONFIG_HOME".to_string(), - self.cfg_dir.path().to_str().unwrap().to_string(), - )]; - let env_config = self.cfg_dir.path().join("env.nu"); + let config_and_data_dir = self.cfg_and_data_dir.path().to_str().unwrap().to_string(); + let env = [ + ("XDG_CONFIG_HOME".to_string(), config_and_data_dir.clone()), + ("XDG_DATA_HOME".to_string(), config_and_data_dir), + ]; + let env_config = self.cfg_and_data_dir.path().join("env.nu"); nu!(envs: env, env_config: env_config, cmd.as_ref()) } fn open_plaintext(&self) -> Result { FileBackedHistory::with_file( 100, - self.cfg_dir + self.cfg_and_data_dir .path() .join("nushell") .join(HistoryFileFormat::Plaintext.default_file_name()), @@ -48,7 +49,7 @@ impl Test { fn open_sqlite(&self) -> Result { SqliteBackedHistory::with_file( - self.cfg_dir + self.cfg_and_data_dir .path() .join("nushell") .join(HistoryFileFormat::Sqlite.default_file_name()), diff --git a/crates/nu-path/src/helpers.rs b/crates/nu-path/src/helpers.rs index 931b3f9d07..90d30c910e 100644 --- a/crates/nu-path/src/helpers.rs +++ b/crates/nu-path/src/helpers.rs @@ -6,9 +6,12 @@ pub fn home_dir() -> Option { dirs::home_dir().and_then(|home| AbsolutePathBuf::try_from(home).ok()) } -/// Return the data directory for the current platform or XDG_DATA_HOME if specified. -pub fn data_dir() -> Option { - configurable_dir_path("XDG_DATA_HOME", dirs::data_dir) +/// Return the nushell data directory. +pub fn nu_data_dir() -> Option { + configurable_dir_path("XDG_DATA_HOME", dirs::data_dir).map(|mut path| { + path.push("nushell"); + path + }) } /// Return the cache directory for the current platform or XDG_CACHE_HOME if specified. @@ -18,9 +21,9 @@ pub fn cache_dir() -> Option { /// Return the nushell config directory. pub fn nu_config_dir() -> Option { - configurable_dir_path("XDG_CONFIG_HOME", dirs::config_dir).map(|mut p| { - p.push("nushell"); - p + configurable_dir_path("XDG_CONFIG_HOME", dirs::config_dir).map(|mut path| { + path.push("nushell"); + path }) } diff --git a/crates/nu-path/src/lib.rs b/crates/nu-path/src/lib.rs index cf31a5789f..7a1ab962ec 100644 --- a/crates/nu-path/src/lib.rs +++ b/crates/nu-path/src/lib.rs @@ -11,7 +11,7 @@ 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, data_dir, home_dir, nu_config_dir}; +pub use helpers::{cache_dir, home_dir, nu_config_dir, nu_data_dir}; pub use path::*; pub use tilde::expand_tilde; pub use trailing_slash::{has_trailing_slash, strip_trailing_slash}; diff --git a/crates/nu-protocol/src/config/history.rs b/crates/nu-protocol/src/config/history.rs index 8f7af7c4a5..ed19f2cfcb 100644 --- a/crates/nu-protocol/src/config/history.rs +++ b/crates/nu-protocol/src/config/history.rs @@ -47,10 +47,46 @@ pub struct HistoryConfig { impl HistoryConfig { pub fn file_path(&self) -> Option { - nu_path::nu_config_dir().map(|mut history_path| { + let history_path: std::path::PathBuf = nu_path::nu_data_dir().map(|mut history_path| { history_path.push(self.file_format.default_file_name()); history_path.into() - }) + })?; + + self.maybe_migrate_history_file_path(history_path.clone()); + + Some(history_path) + } + + fn maybe_migrate_history_file_path(&self, modern_history_path: std::path::PathBuf) { + let maybe_pre_0_99_1_history_path: Option = nu_path::nu_config_dir() + .map(|mut path| { + path.push(self.file_format.default_file_name()); + path.into() + }); + + let Some(pre_0_99_1_history_path) = maybe_pre_0_99_1_history_path else { + return; + }; + + if modern_history_path == pre_0_99_1_history_path { + return; + } + + if !pre_0_99_1_history_path.exists() || modern_history_path.exists() { + return; + } + + // TODO: Create the base directory? `std::fs::create_dir(modern_history_path.parent())` + log::info!("Moving {pre_0_99_1_history_path:?} to {modern_history_path:?}"); + let result = std::fs::rename(pre_0_99_1_history_path.clone(), modern_history_path.clone()); + + if result.is_err() { + // TODO: Report an error. + // It seems a shame to create a whole new error for something that isn't going to + // be relevant for the lifetime of Nushell. But panicking seems a bit overkill for an + // innocent migration. + log::warn!("Couldn't migrate {pre_0_99_1_history_path:?} to {modern_history_path:?}. Error: {result:?}"); + } } } diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index b4b6c29cee..0959cae236 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -1434,6 +1434,21 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"# span: Option, }, + /// The data directory could not be found + #[error("The data directory could not be found")] + #[diagnostic( + code(nu::shell::data_dir_not_found), + help( + r#"On Linux, this would be $XDG_DATA_HOME or $HOME/.local/share. +On MacOS, this would be `$HOME/Library/Application Support` +On Windows, this would be {{FOLDERID_RoamingAppData}}"# + ) + )] + DataDirNotFound { + #[label = "Could not find state directory"] + span: Option, + }, + /// XDG_CONFIG_HOME was set to an invalid path #[error("$env.XDG_CONFIG_HOME ({xdg}) is invalid, using default config directory instead: {default}")] #[diagnostic( diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 695c044a79..e540b9c416 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -153,9 +153,8 @@ pub(crate) fn create_nu_constant(engine_state: &EngineState, span: Span) -> Valu record.push( "data-dir", - if let Some(path) = nu_path::data_dir() { - let mut canon_data_path = canonicalize_path(engine_state, path.as_ref()); - canon_data_path.push("nushell"); + if let Some(path) = nu_path::nu_data_dir() { + let canon_data_path = canonicalize_path(engine_state, path.as_ref()); Value::string(canon_data_path.to_string_lossy(), span) } else { Value::error( diff --git a/src/main.rs b/src/main.rs index 2c7b94b628..0147e9db7e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -137,8 +137,7 @@ fn main() -> Result<()> { } } - let default_nushell_completions_path = if let Some(mut path) = nu_path::data_dir() { - path.push("nushell"); + let default_nushell_completions_path = if let Some(mut path) = nu_path::nu_data_dir() { path.push("completions"); path.into() } else {