h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-06-09 20:21:16 -03:00
Code Issues Packages Projects Releases Wiki Activity

Allow histories to be created outside of the default path

Browse Source 
Currently history files are written to the "data directory"
(XDG_DATA_HOME). This is awkward in testing since we have to put files
into this directory.

Allow histories to have their own directory, so that they don't
interfere with other files. This will help simplify some tests.

Adopt this in some (but not all) history tests.
This commit is contained in:
Peter Ammon
2026-02-02 14:54:13 -08:00
parent 8d2d50573a
commit f9013ad7a6
1 changed files with 59 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
src/history/history.rs
View File

@@ -295,6 +295,9 @@ enum DeletionScope {
struct HistoryImpl {
/// The name of this list. Used for picking a suitable filename and for switching modes.
name: WString,
/// Optional custom directory for the history file. If None, uses path_get_data().
/// Primarily for testing.
custom_directory: Option<WString>,
/// New items. Note that these are NOT discarded on save. We need to keep these around so we can
/// distinguish between items in our history and items in the history of other shells that were
/// started after we were started.
@@ -333,7 +336,11 @@ fn history_file_path(&self) -> std::io::Result<Option<WString>> {
return Ok(None);
}
let Some(mut path) = path_get_data() else {
let mut path = if let Some(custom_dir) = &self.custom_directory {
custom_dir.clone()
} else if let Some(data_path) = path_get_data() {
data_path
} else {
return Err(std::io::Error::new(
std::io::ErrorKind::NotFound,
"Error obtaining data directory. This is a manually constructed error which does not indicate why this happened.",
@@ -343,7 +350,11 @@ fn history_file_path(&self) -> std::io::Result<Option<WString>> {
path.push('/');
path.push_utfstr(&self.name);
path.push_utfstr(L!("_history"));
if let Some(canonicalized_path) = wrealpath(&path) {
// For custom directories, skip wrealpath since file may not exist yet
if self.custom_directory.is_some() {
Ok(Some(path))
} else if let Some(canonicalized_path) = wrealpath(&path) {
Ok(Some(canonicalized_path))
} else {
Err(std::io::Error::other(format!(
@@ -736,9 +747,10 @@ fn save_unless_disabled(&mut self) {
self.save(vacuum);
}
fn new(name: WString) -> Self {
fn new(name: WString, custom_directory: Option<WString>) -> Self {
Self {
name,
custom_directory,
new_items: vec![],
first_unwritten_new_item_index: 0,
has_pending_item: false,
@@ -1210,18 +1222,26 @@ pub fn add_commandline(&self, s: WString) {
imp.add(item, false, true);
}
pub fn new(name: &wstr) -> Arc<Self> {
Arc::new(Self(Mutex::new(HistoryImpl::new(name.to_owned()))))
/// Creates a new History with a custom directory path.
/// The history file will be stored at `{directory}/{name}_history`.
/// If the directory is None, it will be stored at path_get_data().
pub fn new(name: &wstr, directory: Option<WString>) -> Arc<Self> {
Arc::new(Self(Mutex::new(HistoryImpl::new(
name.to_owned(),
directory,
))))
}
/// Returns history with the given name, creating it if necessary.
/// Returns the history with the given name, creating it if necessary, using the default data directory.
/// This uses the HISTORIES global collection. Note it is possible to create a history without
/// placing it into this collection.
pub fn with_name(name: &wstr) -> Arc<Self> {
let mut histories = HISTORIES.lock().unwrap();
if let Some(hist) = histories.get(name) {
Arc::clone(hist)
} else {
let hist = Self::new(name);
let hist = Self::new(name, None);
histories.insert(name.to_owned(), Arc::clone(&hist));
hist
}
@@ -1786,7 +1806,7 @@ mod tests {
History, HistoryItem, HistorySearch, PathList, PersistenceMode, SearchDirection,
SearchFlags, SearchType, VACUUM_FREQUENCY,
};
use crate::common::{ESCAPE_TEST_CHAR, ScopeGuard, osstr2wcstring, wcs2bytes, wcs2osstring};
use crate::common::{ESCAPE_TEST_CHAR, osstr2wcstring, wcs2bytes, wcs2osstring};
use crate::env::{EnvMode, EnvSetMode, EnvStack};
use crate::fs::{LockedFile, WriteMethod};
use crate::path::path_get_data;
@@ -1799,8 +1819,7 @@ mod tests {
use std::collections::VecDeque;
use std::io::BufReader;
use std::sync::Arc;
use std::time::UNIX_EPOCH;
use std::time::{Duration, SystemTime};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
fn history_contains(history: &History, txt: &wstr) -> bool {
for i in 1.. {
@@ -2010,9 +2029,10 @@ fn generate_history_lines(item_count: usize, idx: usize) -> Vec<WString> {
result
}
fn pound_on_history(item_count: usize, idx: usize) -> Arc<History> {
#[allow(clippy::ref_option)]
fn write_history_entries(dir: &Option<WString>, item_count: usize, idx: usize) -> Arc<History> {
// Called in child thread to modify history.
let hist = History::new(L!("race_test"));
let hist = History::new(L!("race_test"), dir.clone());
let hist_lines = generate_history_lines(item_count, idx);
for line in hist_lines {
hist.add_commandline(line);
@@ -2022,23 +2042,16 @@ fn pound_on_history(item_count: usize, idx: usize) -> Arc<History> {
}
#[test]
#[serial]
fn test_history_races() {
let _cleanup = test_init();
// Place history in a temp directory.
let tmpdir = fish_tempfile::new_dir().unwrap();
let hist_dir = Some(osstr2wcstring(tmpdir.path()));
let tmp_path = std::env::current_dir()
.unwrap()
.join("history-races-test-balloon");
std::fs::write(&tmp_path, []).unwrap();
let _cleanup = ScopeGuard::new((), |()| {
std::fs::remove_file(&tmp_path).unwrap();
});
if LockedFile::new(
crate::fs::LockingMode::Exclusive(WriteMethod::RenameIntoPlace),
&osstr2wcstring(&tmp_path),
)
.is_err()
{
// Skip tests if we can't get an exclusive lock on a file in that directory.
let tmp_balloon = tmpdir.path().join("history-races-test-balloon");
std::fs::write(&tmp_balloon, []).unwrap();
let mode = crate::fs::LockingMode::Exclusive(WriteMethod::RenameIntoPlace);
if LockedFile::new(mode, &osstr2wcstring(&tmp_balloon)).is_err() {
return;
}
@@ -2052,12 +2065,13 @@ fn test_history_races() {
const ITEM_COUNT: usize = 256;
// Ensure history is clear.
History::new(L!("race_test")).clear();
History::new(L!("race_test"), hist_dir.clone()).clear();
let mut children = Vec::with_capacity(RACE_COUNT);
for i in 0..RACE_COUNT {
let hist_dir = hist_dir.clone();
children.push(std::thread::spawn(move || {
pound_on_history(ITEM_COUNT, i);
write_history_entries(&hist_dir, ITEM_COUNT, i);
}));
}
@@ -2074,7 +2088,7 @@ fn test_history_races() {
time_barrier();
// Ensure that we got sane, sorted results.
let hist = History::new(L!("race_test"));
let hist = History::new(L!("race_test"), hist_dir.clone());
// History is enumerated from most recent to least
// Every item should be the last item in some array
@@ -2128,25 +2142,26 @@ fn test_history_races() {
}
#[test]
#[serial]
fn test_history_external_rewrites() {
let _cleanup = test_init();
// Place history in a temp directory.
let tmpdir = fish_tempfile::new_dir().unwrap();
let hist_dir = Some(osstr2wcstring(tmpdir.path()));
// Write some history to disk.
{
let hist = pound_on_history(VACUUM_FREQUENCY / 2, 0);
let hist = write_history_entries(&hist_dir, VACUUM_FREQUENCY / 2, 0);
hist.add_commandline("needle".into());
hist.save();
}
std::thread::sleep(Duration::from_secs(1));
// Read history from disk.
let hist = History::new(L!("race_test"));
let hist = History::new(L!("race_test"), hist_dir.clone());
assert_eq!(hist.item_at_index(1).unwrap().str(), "needle");
// Add items until we rewrite the file.
// In practice this might be done by another shell.
pound_on_history(VACUUM_FREQUENCY, 0);
write_history_entries(&hist_dir, VACUUM_FREQUENCY, 0);
for i in 1.. {
if hist.item_at_index(i).unwrap().str() == "needle" {
@@ -2156,16 +2171,21 @@ fn test_history_external_rewrites() {
}
#[test]
#[serial]
fn test_history_merge() {
let _cleanup = test_init();
let tmpdir = fish_tempfile::new_dir().unwrap();
let hist_dir = Some(osstr2wcstring(tmpdir.path()));
// In a single fish process, only one history is allowed to exist with the given name But it's
// common to have multiple history instances with the same name active in different processes,
// e.g. when you have multiple shells open. We try to get that right and merge all their history
// together. Test that case.
const COUNT: usize = 3;
let name = L!("merge_test");
let hists = [History::new(name), History::new(name), History::new(name)];
let hists = [
History::new(name, hist_dir.clone()),
History::new(name, hist_dir.clone()),
History::new(name, hist_dir.clone()),
];
let texts = [L!("History 1"), L!("History 2"), L!("History 3")];
let alt_texts = [
L!("History Alt 1"),
@@ -2204,7 +2224,7 @@ fn test_history_merge() {
// Make a new history. It should contain everything. The time_barrier() is so that the timestamp
// is newer, since we only pick up items whose timestamp is before the birth stamp.
time_barrier();
let everything = History::new(name);
let everything = History::new(name, hist_dir.clone());
for text in texts {
assert!(history_contains(&everything, text));
}