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.
This commit is contained in:
Thomas Buckley-Houston 2024-11-26 14:05:59 +01:00
parent 4d3283e235
commit 975ebba33f
No known key found for this signature in database
GPG Key ID: 6A8FA0E4BA72A791
9 changed files with 80 additions and 27 deletions

View File

@ -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")? {

View File

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

View File

@ -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<str>) -> 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, ReedlineError> {
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, ReedlineError> {
SqliteBackedHistory::with_file(
self.cfg_dir
self.cfg_and_data_dir
.path()
.join("nushell")
.join(HistoryFileFormat::Sqlite.default_file_name()),

View File

@ -6,9 +6,12 @@ pub fn home_dir() -> Option<AbsolutePathBuf> {
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<AbsolutePathBuf> {
configurable_dir_path("XDG_DATA_HOME", dirs::data_dir)
/// Return the nushell data directory.
pub fn nu_data_dir() -> Option<AbsolutePathBuf> {
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<AbsolutePathBuf> {
/// Return the nushell config directory.
pub fn nu_config_dir() -> Option<AbsolutePathBuf> {
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
})
}

View File

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

View File

@ -47,10 +47,46 @@ pub struct HistoryConfig {
impl HistoryConfig {
pub fn file_path(&self) -> Option<std::path::PathBuf> {
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<std::path::PathBuf> = 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:?}");
}
}
}

View File

@ -1434,6 +1434,21 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"#
span: Option<Span>,
},
/// 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<Span>,
},
/// XDG_CONFIG_HOME was set to an invalid path
#[error("$env.XDG_CONFIG_HOME ({xdg}) is invalid, using default config directory instead: {default}")]
#[diagnostic(

View File

@ -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(

View File

@ -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 {