Update commandline state snapshot lazily

I think commit 8386088b3 (Update commandline state changes eagerly as well,
2024-04-11) broke the alt-s binding.

This is because we update the commandline state snapshot (which is consumed
by builtin commandline and others) only at key points.  This seems like a
dubious optimization.  With the new streamlined bind execution semantics,
this doesn't really work anymore; any shell command can run any number of
commands like "commandline -i foo" which should synchronize.

Do the simple thing of calculating the snapshot whenever needed.
This commit is contained in:
Johannes Altmanninger 2024-04-13 09:03:00 +02:00
parent edb5cb7226
commit 4f536d6a9b
6 changed files with 42 additions and 27 deletions

View File

@ -181,7 +181,7 @@ fn write_part(
/// The commandline builtin. It is used for specifying a new value for the commandline. /// The commandline builtin. It is used for specifying a new value for the commandline.
pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option<c_int> { pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Option<c_int> {
let rstate = commandline_get_state(); let rstate = commandline_get_state(true);
let mut buffer_part = None; let mut buffer_part = None;
let mut cut_at_cursor = false; let mut cut_at_cursor = false;

View File

@ -485,7 +485,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) ->
let do_complete_param = match do_complete_param { let do_complete_param = match do_complete_param {
None => { None => {
// No argument given, try to use the current commandline. // No argument given, try to use the current commandline.
let commandline_state = commandline_get_state(); let commandline_state = commandline_get_state(true);
if !commandline_state.initialized { if !commandline_state.initialized {
// This corresponds to using 'complete -C' in non-interactive mode. // This corresponds to using 'complete -C' in non-interactive mode.
// See #2361 . // See #2361 .

View File

@ -243,7 +243,7 @@ pub fn history(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) ->
// Use the default history if we have none (which happens if invoked non-interactively, e.g. // Use the default history if we have none (which happens if invoked non-interactively, e.g.
// from webconfig.py. // from webconfig.py.
let history = commandline_get_state() let history = commandline_get_state(true)
.history .history
.unwrap_or_else(|| History::with_name(&history_session_id(parser.vars()))); .unwrap_or_else(|| History::with_name(&history_session_id(parser.vars())));

View File

@ -353,7 +353,7 @@ impl EnvScopedImpl {
if !is_main_thread() { if !is_main_thread() {
return None; return None;
} }
let history = commandline_get_state().history.unwrap_or_else(|| { let history = commandline_get_state(true).history.unwrap_or_else(|| {
let fish_history_var = self.getf(L!("fish_history"), EnvMode::default()); let fish_history_var = self.getf(L!("fish_history"), EnvMode::default());
let session_id = history_session_id_from_var(fish_history_var); let session_id = history_session_id_from_var(fish_history_var);
History::with_name(&session_id) History::with_name(&session_id)

View File

@ -156,8 +156,8 @@ static INTERRUPTED: AtomicI32 = AtomicI32::new(0);
/// This is set from a signal handler. /// This is set from a signal handler.
static SIGHUP_RECEIVED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); static SIGHUP_RECEIVED: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
/// A singleton snapshot of the reader state. This is updated when the reader changes. This is /// A singleton snapshot of the reader state. This is factored out for thread-safety reasons:
/// factored out for thread-safety reasons: it may be fetched on a background thread. /// it may be fetched on a background thread.
fn commandline_state_snapshot() -> MutexGuard<'static, CommandlineState> { fn commandline_state_snapshot() -> MutexGuard<'static, CommandlineState> {
static STATE: Mutex<CommandlineState> = Mutex::new(CommandlineState::new()); static STATE: Mutex<CommandlineState> = Mutex::new(CommandlineState::new());
STATE.lock().unwrap() STATE.lock().unwrap()
@ -233,7 +233,6 @@ fn reader_push_ret(
if reader_data_stack().len() == 1 { if reader_data_stack().len() == 1 {
reader_interactive_init(parser); reader_interactive_init(parser);
} }
data.update_commandline_state();
data data
} }
@ -252,7 +251,6 @@ pub fn reader_pop() {
new_reader new_reader
.screen .screen
.reset_abandoning_line(usize::try_from(termsize_last().width).unwrap()); .reset_abandoning_line(usize::try_from(termsize_last().width).unwrap());
new_reader.update_commandline_state();
} else { } else {
reader_interactive_destroy(); reader_interactive_destroy();
*commandline_state_snapshot() = CommandlineState::new(); *commandline_state_snapshot() = CommandlineState::new();
@ -907,7 +905,6 @@ pub fn reader_execute_readline_cmd(ch: CharEvent) {
} }
data.save_screen_state(); data.save_screen_state();
data.handle_char_event(Some(ch)); data.handle_char_event(Some(ch));
data.update_commandline_state();
} }
} }
@ -940,7 +937,10 @@ pub fn reader_readline(nchars: usize) -> Option<WString> {
} }
/// Get the command line state. This may be fetched on a background thread. /// Get the command line state. This may be fetched on a background thread.
pub fn commandline_get_state() -> CommandlineState { pub fn commandline_get_state(sync: bool) -> CommandlineState {
if sync {
current_data().map(|data| data.update_commandline_state());
}
commandline_state_snapshot().clone() commandline_state_snapshot().clone()
} }
@ -1182,27 +1182,34 @@ impl ReaderData {
self.pager_selection_changed(); self.pager_selection_changed();
} }
} }
// Ensure that the commandline builtin sees our new state.
self.update_commandline_state();
} }
/// Reflect our current data in the command line state snapshot. /// Reflect our current data in the command line state snapshot.
/// This is called before we run any fish script, so that the commandline builtin can see our
/// state.
fn update_commandline_state(&self) { fn update_commandline_state(&self) {
let mut snapshot = commandline_state_snapshot(); let mut snapshot = commandline_state_snapshot();
snapshot.text = self.command_line.text().to_owned(); if snapshot.text != self.command_line.text() {
snapshot.text = self.command_line.text().to_owned();
}
snapshot.cursor_pos = self.command_line.position(); snapshot.cursor_pos = self.command_line.position();
snapshot.history = Some(self.history.clone()); snapshot.history = Some(self.history.clone());
snapshot.selection = self.get_selection(); snapshot.selection = self.get_selection();
snapshot.pager_mode = !self.pager.is_empty(); snapshot.pager_mode = !self.pager.is_empty();
snapshot.pager_fully_disclosed = self.current_page_rendering.remaining_to_disclose == 0; snapshot.pager_fully_disclosed = self.current_page_rendering.remaining_to_disclose == 0;
snapshot.search_field = self.pager.search_field_shown.then(|| { if snapshot
( .search_field
self.pager.search_field_line.text().to_owned(), .as_ref()
self.pager.search_field_line.position(), .is_none_or(|(text, position)| {
) text != self.pager.search_field_line.text()
}); || *position != self.pager.search_field_line.position()
})
{
snapshot.search_field = self.pager.search_field_shown.then(|| {
(
self.pager.search_field_line.text().to_owned(),
self.pager.search_field_line.position(),
)
});
}
snapshot.search_mode = self.history_search.active(); snapshot.search_mode = self.history_search.active();
snapshot.initialized = true; snapshot.initialized = true;
} }
@ -1211,7 +1218,7 @@ impl ReaderData {
/// incorporating changes from the commandline builtin. /// incorporating changes from the commandline builtin.
fn apply_commandline_state_changes(&mut self) { fn apply_commandline_state_changes(&mut self) {
// Only the text and cursor position may be changed. // Only the text and cursor position may be changed.
let state = commandline_get_state(); let state = commandline_get_state(false);
if state.text != self.command_line.text() if state.text != self.command_line.text()
|| state.cursor_pos != self.command_line.position() || state.cursor_pos != self.command_line.position()
{ {
@ -1904,7 +1911,6 @@ impl ReaderData {
/// Run a sequence of commands from an input binding. /// Run a sequence of commands from an input binding.
fn run_input_command_scripts(&mut self, cmd: &wstr) { fn run_input_command_scripts(&mut self, cmd: &wstr) {
self.update_commandline_state();
self.eval_bind_cmd(cmd); self.eval_bind_cmd(cmd);
// Restore tty to shell modes. // Restore tty to shell modes.
@ -4554,7 +4560,6 @@ impl ReaderData {
let (elt, el) = self.active_edit_line(); let (elt, el) = self.active_edit_line();
if self.conf.expand_abbrev_ok && elt == EditableLineTag::Commandline { if self.conf.expand_abbrev_ok && elt == EditableLineTag::Commandline {
// Try expanding abbreviations. // Try expanding abbreviations.
self.update_commandline_state();
let cursor_pos = el.position().saturating_sub(cursor_backtrack); let cursor_pos = el.position().saturating_sub(cursor_backtrack);
if let Some(replacement) = if let Some(replacement) =
reader_expand_abbreviation_at_cursor(el.text(), cursor_pos, self.parser()) reader_expand_abbreviation_at_cursor(el.text(), cursor_pos, self.parser())
@ -5284,9 +5289,6 @@ impl ReaderData {
// up to the end of the token we're completing. // up to the end of the token we're completing.
let cmdsub = &el.text()[cmdsub_range.start..token_range.end]; let cmdsub = &el.text()[cmdsub_range.start..token_range.end];
// Ensure that `commandline` inside the completions gets the current state.
self.update_commandline_state();
let (comp, _needs_load) = complete( let (comp, _needs_load) = complete(
cmdsub, cmdsub,
CompletionRequestOptions::normal(), CompletionRequestOptions::normal(),

View File

@ -0,0 +1,13 @@
#RUN: %fish %s
#REQUIRES: command -v tmux
isolated-tmux-start
isolated-tmux send-keys "function prepend; commandline --cursor 0; commandline -i echo; end" Enter
isolated-tmux send-keys "bind c-g prepend" Enter
isolated-tmux send-keys C-l
isolated-tmux send-keys 'printf'
isolated-tmux send-keys C-g Space
tmux-sleep
isolated-tmux capture-pane -p
# CHECK: prompt 2> echo printf