Distinguish builtin read history session ID from private mode

Fixes #12662
This commit is contained in:
Johannes Altmanninger
2026-04-29 00:28:31 +08:00
parent e5f57b1daf
commit 281399561b
11 changed files with 96 additions and 41 deletions

View File

@@ -2,7 +2,7 @@
use crate::builtins::error::Error;
use crate::history::in_private_mode;
use crate::history::{self, History, history_session_id};
use crate::history::{self, History, history_id};
use crate::reader::commandline_get_state;
use crate::{err_fmt, err_str};
@@ -256,7 +256,7 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) ->
// from webconfig.py.
let history = commandline_get_state(true)
.history
.unwrap_or_else(|| History::new(&history_session_id(parser.vars())));
.unwrap_or_else(|| History::new(history_id(parser.vars())));
// If a history command hasn't already been specified via a flag check the first word.
// Note that this can be simplified after we eliminate allowing subcommands as flags.

View File

@@ -6,6 +6,7 @@
common::valid_var_name,
env::{EnvMode, EnvVar, EnvVarFlags, Environment as _, READ_BYTE_LIMIT},
err_fmt, err_str,
history::{HistoryId, MemoryHistoryId},
input_common::{DecodeState, InvalidPolicy, decode_utf8},
nix::isatty,
parse_execution::varname_error,
@@ -261,7 +262,11 @@ fn read_interactive(
let old_modes = set_shell_modes_temporarily(inputfd);
// Keep in-memory history only.
reader_push(parser, L!(""), conf);
reader_push(
parser,
HistoryId::Memory(MemoryHistoryId::BuiltinRead),
conf,
);
let _modifiable_commandline = parser.scope().readonly_commandline.then(|| {
parser.push_scope(|s| {
s.readonly_commandline = false;

View File

@@ -6,7 +6,7 @@
err_fmt, err_str,
event::{self, Event},
expand::{expand_escape_string, expand_escape_variable},
history::{History, history_session_id},
history::{History, history_id},
parse_execution::varname_error,
parser::ParserEnvSetMode,
wutil::wcstoi::wcstoi_partial,
@@ -567,7 +567,7 @@ fn list(opts: &Options, parser: &Parser, streams: &mut IoStreams) -> BuiltinResu
if !names_only {
let mut val = WString::new();
if opts.shorten_ok && key == "history" {
let history = History::new(&history_session_id(parser.vars()));
let history = History::new(history_id(parser.vars()));
for i in 1..history.size() {
if val.len() >= 64 {
break;

View File

@@ -12,7 +12,7 @@
},
flog::{flog, flogf},
function,
history::{History, history_session_id},
history::{History, history_id},
localization::{LocalizableString, localizable_string},
operation_context::OperationContext,
parse_constants::SourceRange,
@@ -1675,7 +1675,7 @@ fn complete_variable(&mut self, s: &wstr, start_offset: usize) -> bool {
// $history can be huge, don't put all of it in the completion description; see
// #6288.
if env_name == "history" {
let history = History::new(&history_session_id(self.ctx.vars()));
let history = History::new(history_id(self.ctx.vars()));
for i in 1..std::cmp::min(history.size(), 64) {
if i > 1 {
desc.push(' ');

View File

@@ -5,7 +5,7 @@
use crate::env_universal_common::EnvUniversal;
use crate::flog::flog;
use crate::global_safety::RelaxedAtomicBool;
use crate::history::{History, history_session_id_from_var};
use crate::history::{History, history_id_from_var};
use crate::kill::kill_entries;
use crate::null_terminated_array::OwningNullTerminatedArray;
use crate::portable_atomic::AtomicU64;
@@ -373,8 +373,8 @@ fn try_get_computed(&self, key: &wstr) -> Option<EnvVar> {
}
let history = commandline_get_state(true).history.unwrap_or_else(|| {
let fish_history_var = self.getf(L!("fish_history"), EnvMode::default());
let session_id = history_session_id_from_var(fish_history_var);
History::new(&session_id)
let history_id = history_id_from_var(fish_history_var);
History::new(history_id)
});
Some(EnvVar::new_from_name_vec(
L!("history"),

View File

@@ -231,8 +231,8 @@ fn handle_term_size_change(vars: &EnvStack) {
}
fn handle_fish_history_change(vars: &EnvStack) {
let session_id = crate::history::history_session_id(vars);
reader_change_history(&session_id);
let history_id = crate::history::history_id(vars);
reader_change_history(history_id);
}
fn handle_fish_cursor_selection_mode_change(vars: &EnvStack) {

View File

@@ -13,7 +13,7 @@
complete::{CompleteFlags, Completion, CompletionList, CompletionReceiver},
env::{EnvVar, Environment},
exec::exec_subshell_for_expand,
history::{History, history_session_id},
history::{History, history_id},
operation_context::OperationContext,
parse_constants::{ParseError, ParseErrorCode, ParseErrorList, SOURCE_LOCATION_UNKNOWN},
parse_util::{MaybeParentheses, expand_variable_error, locate_cmdsubst_range},
@@ -613,7 +613,7 @@ fn expand_variables(
let mut history = None;
let mut var = None;
if var_name == "history" {
history = Some(History::new(&history_session_id(vars)));
history = Some(History::new(history_id(vars)));
} else if var_name.as_char_slice() != [VARIABLE_EXPAND_EMPTY] {
var = vars.get(var_name);
}

View File

@@ -276,7 +276,19 @@ fn merge(&mut self, item: &HistoryItem) -> bool {
}
}
static HISTORIES: Mutex<BTreeMap<WString, Arc<History>>> = Mutex::new(BTreeMap::new());
#[derive(Clone, Eq, Ord, PartialEq, PartialOrd)]
pub enum MemoryHistoryId {
PrivateMode,
BuiltinRead,
}
#[derive(Clone, Eq, Ord, PartialEq, PartialOrd)]
pub enum HistoryId {
Memory(MemoryHistoryId),
Disk { session_id: WString },
}
static HISTORIES: Mutex<BTreeMap<HistoryId, Arc<History>>> = Mutex::new(BTreeMap::new());
/// When deleting, whether the deletion should be only for this session or for all sessions.
#[derive(Clone, Copy, PartialEq, Eq)]
@@ -1218,9 +1230,14 @@ pub fn add_commandline(&self, s: WString) {
/// 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().
fn new_with_directory(name: &wstr, directory: Option<WString>) -> Arc<Self> {
fn new_with_directory(id: HistoryId, directory: Option<WString>) -> Arc<Self> {
Arc::new(Self(Mutex::new(HistoryImpl::new(
name.to_owned(),
match id {
HistoryId::Memory(_) => WString::new(),
HistoryId::Disk {
session_id: filename,
} => filename,
},
directory,
))))
}
@@ -1228,14 +1245,14 @@ fn new_with_directory(name: &wstr, directory: Option<WString>) -> Arc<Self> {
/// 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 new(name: &wstr) -> Arc<Self> {
pub fn new(id: HistoryId) -> Arc<Self> {
let mut histories = HISTORIES.lock().unwrap();
if let Some(hist) = histories.get(name) {
if let Some(hist) = histories.get(&id) {
Arc::clone(hist)
} else {
let hist = Self::new_with_directory(name, None);
histories.insert(name.to_owned(), Arc::clone(&hist));
let hist = Self::new_with_directory(id.clone(), None);
histories.insert(id, Arc::clone(&hist));
hist
}
}
@@ -1693,17 +1710,23 @@ pub fn save_all() {
}
/// Return the prefix for the files to be used for command and read history.
pub fn history_session_id(vars: &dyn Environment) -> WString {
history_session_id_from_var(vars.get(L!("fish_history")))
pub fn history_id(vars: &dyn Environment) -> HistoryId {
history_id_from_var(vars.get(L!("fish_history")))
}
pub fn history_session_id_from_var(history_name_var: Option<EnvVar>) -> WString {
pub fn history_id_from_var(history_name_var: Option<EnvVar>) -> HistoryId {
use HistoryId::*;
let default = || Disk {
session_id: DFLT_FISH_HISTORY_SESSION_ID.to_owned(),
};
let Some(var) = history_name_var else {
return DFLT_FISH_HISTORY_SESSION_ID.to_owned();
return default();
};
let session_id = var.as_string();
if session_id.is_empty() || valid_var_name(&session_id) {
session_id
if session_id.is_empty() {
Memory(MemoryHistoryId::PrivateMode)
} else if valid_var_name(&session_id) {
Disk { session_id }
} else {
flog!(
error,
@@ -1713,7 +1736,7 @@ pub fn history_session_id_from_var(history_name_var: Option<EnvVar>) -> WString
DFLT_FISH_HISTORY_SESSION_ID
),
);
DFLT_FISH_HISTORY_SESSION_ID.to_owned()
default()
}
}
@@ -1803,6 +1826,7 @@ mod tests {
common::ESCAPE_TEST_CHAR,
env::{EnvMode, EnvSetMode, EnvStack},
fs::{LockedFile, WriteMethod},
history::HistoryId,
prelude::*,
tests::prelude::test_init,
};
@@ -1834,7 +1858,12 @@ fn history_contains(history: &History, txt: &wstr) -> bool {
// Helper to create a history with a custom directory, for testing.
fn create_test_history(name: &wstr, custom_dir: &wstr) -> Arc<History> {
History::new_with_directory(name, Some(custom_dir.to_owned()))
History::new_with_directory(
HistoryId::Disk {
session_id: name.to_owned(),
},
Some(custom_dir.to_owned()),
)
}
fn random_string(rng: &mut ThreadRng) -> WString {
@@ -2292,7 +2321,6 @@ fn test_history_path_detection() {
// Regression test for #7582.
// Temporary directory for the history files.
let hist_tmpdir = fish_tempfile::new_dir().unwrap();
let hist_dir = Some(osstr2wcstring(hist_tmpdir.path()));
// Temporary directory for the files we will detect.
let tmpdir = fish_tempfile::new_dir().unwrap();
@@ -2309,7 +2337,8 @@ fn test_history_path_detection() {
test_vars.set_one(L!("PWD"), global_mode, wdir_path.clone());
test_vars.set_one(L!("HOME"), global_mode, wdir_path.clone());
let history = History::new_with_directory(L!("path_detection"), hist_dir);
let history =
create_test_history(L!("path_detection"), &osstr2wcstring(hist_tmpdir.path()));
history.clear();
assert_eq!(history.size(), 0);
history.add_pending_with_file_detection(

View File

@@ -47,8 +47,8 @@
parse_text_face_for_highlight,
},
history::{
History, HistorySearch, PersistenceMode, SearchDirection, SearchFlags, SearchType,
history_session_id, in_private_mode,
History, HistoryId, HistorySearch, MemoryHistoryId, PersistenceMode, SearchDirection,
SearchFlags, SearchType, history_id, in_private_mode,
},
input_common::{
BackgroundColorQuery, CharEvent, CharInputStyle, CursorPositionQuery,
@@ -369,8 +369,11 @@ pub fn current_data() -> Option<&'static mut ReaderData> {
use fish_widestring::word_char::{WordCharClass, is_blank};
/// Add a new reader to the reader stack.
/// If `history_name` is empty, then save history in-memory only; do not write it to disk.
pub fn reader_push<'a>(parser: &'a Parser, history_name: &wstr, conf: ReaderConfig) -> Reader<'a> {
pub fn reader_push<'a>(
parser: &'a Parser,
history_id: HistoryId,
conf: ReaderConfig,
) -> Reader<'a> {
assert_is_main_thread();
let inputfd = conf.inputfd;
let input_data = if !parser.interactive_initialized.swap(true) {
@@ -398,7 +401,7 @@ pub fn reader_push<'a>(parser: &'a Parser, history_name: &wstr, conf: ReaderConf
} else {
InputData::new(inputfd, *parser.blocking_query_timeout.borrow())
};
let hist = History::new(history_name);
let hist = History::new(history_id);
hist.resolve_pending();
let data = ReaderData::new(input_data, hist, conf, reader_data_stack().is_empty());
reader_data_stack().push(data);
@@ -427,7 +430,7 @@ pub fn fake_scoped_reader<'a>(parser: &'a Parser) -> impl ScopeGuarding<Target =
inputfd,
..Default::default()
};
let hist = History::new(L!(""));
let hist = History::new(HistoryId::Memory(MemoryHistoryId::PrivateMode));
let input_data = InputData::new(inputfd, None);
let data = ReaderData::new(input_data, hist, conf, reader_data_stack().is_empty());
reader_data_stack().push(data);
@@ -830,7 +833,7 @@ fn read_i(parser: &Parser) {
conf.right_prompt_cmd = RIGHT_PROMPT_FUNCTION_NAME.to_owned();
}
let mut data = reader_push(parser, &history_session_id(parser.vars()), conf);
let mut data = reader_push(parser, history_id(parser.vars()), conf);
data.import_history_if_necessary();
// Set up tty protocols. These should be enabled while we're reading interactively,
@@ -1052,14 +1055,14 @@ pub fn restore_term_mode() {
}
/// Change the history file for the current command reading context.
pub fn reader_change_history(name: &wstr) {
pub fn reader_change_history(history_id: HistoryId) {
// We don't need to _change_ if we're not initialized yet.
let Some(data) = current_data() else {
return;
};
data.history.save();
data.history = History::new(name);
data.history = History::new(history_id);
commandline_state_snapshot().history = Some(data.history.clone());
}