From 11c7310f17ea2e8507ca4fc0f7444ef6a22a071f Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Wed, 26 Mar 2025 20:38:31 +0100 Subject: [PATCH] Prevent commandline modification inside abbreviation callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consider command line modifications triggered from fish script via abbreviation expansion: function my-abbr-func commandline -r "" echo expanded end abbr -a foo --function my-abbr-func Prior to commit 8386088b3d (Update commandline state changes eagerly as well, 2024-04-11), we'd silently ignore the command line modification. This is because the abbreviation machinery runs something similar to if my-abbr-func commandline -rt expanded end except without running "apply_commandline_state_changes()" after "my-abbr-func", so the «commandline -r ""» update is lost. Commit 8386088b3d applies the commandline change immediately in the abbrevation function callback, invalidating abbrevation-expansion state. The abbreviation design does not tell us what should happen here. Let's ignore commandline modifications for now. This mostly matches historical behavior. Unlike historical behavior we also ignore modifications if the callback fails: function my-abbr-func commandline -r "" false end Remove the resulting dead code in editable_line. See #11324 --- src/builtins/commandline.rs | 11 +++++++---- src/builtins/read.rs | 7 ++++++- src/editable_line.rs | 28 +++++++--------------------- src/parser.rs | 4 ++++ src/reader.rs | 18 +++++++++++++++--- 5 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/builtins/commandline.rs b/src/builtins/commandline.rs index d1d04cf05..39fed1cc2 100644 --- a/src/builtins/commandline.rs +++ b/src/builtins/commandline.rs @@ -60,6 +60,7 @@ enum TokenMode { /// \param buff the original command line buffer /// \param cursor_pos the position of the cursor in the command line fn replace_part( + parser: &Parser, range: Range, insert: &wstr, insert_mode: AppendMode, @@ -94,9 +95,9 @@ fn replace_part( out.push_utfstr(&buff[range.end..]); if search_field_mode { - commandline_set_search_field(out, Some(out_pos)); + commandline_set_search_field(parser, out, Some(out_pos)); } else { - commandline_set_buffer(Some(out), Some(out_pos)); + commandline_set_buffer(parser, Some(out), Some(out_pos)); } } @@ -547,7 +548,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) } line_offset + new_coord }; - commandline_set_buffer(None, Some(new_pos)); + commandline_set_buffer(parser, None, Some(new_pos)); } else { streams.out.append(sprintf!( "%d\n", @@ -709,7 +710,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) .saturating_add_signed(isize::try_from(new_pos).unwrap()), current_buffer.len(), ); - commandline_set_buffer(None, Some(new_pos)); + commandline_set_buffer(parser, None, Some(new_pos)); } else { streams .out @@ -731,6 +732,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) ); } else if positional_args == 1 { replace_part( + parser, range, args[w.wopt_index], append_mode, @@ -741,6 +743,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) } else { let sb = join_strings(&w.argv[w.wopt_index..], '\n'); replace_part( + parser, range, &sb, append_mode, diff --git a/src/builtins/read.rs b/src/builtins/read.rs index 6d48d7f1d..24b7cb9f5 100644 --- a/src/builtins/read.rs +++ b/src/builtins/read.rs @@ -234,7 +234,12 @@ fn read_interactive( // Keep in-memory history only. reader_push(parser, L!(""), conf); - commandline_set_buffer(Some(commandline.to_owned()), None); + let _modifiable_commandline = parser.scope().readonly_commandline.then(|| { + parser.push_scope(|s| { + s.readonly_commandline = false; + }) + }); + commandline_set_buffer(parser, Some(commandline.to_owned()), None); let mline = { let _interactive = parser.push_scope(|s| s.is_interactive = true); diff --git a/src/editable_line.rs b/src/editable_line.rs index 1c69374a7..433ccf0b1 100644 --- a/src/editable_line.rs +++ b/src/editable_line.rs @@ -41,11 +41,7 @@ pub fn new(range: std::ops::Range, replacement: WString) -> Self { /// Currently exposed for testing only. pub fn apply_edit(target: &mut WString, colors: &mut Vec, edit: &Edit) { let range = &edit.range; - if target.is_empty() { - *target = edit.replacement.clone(); - } else { - target.replace_range(range.clone(), &edit.replacement); - } + target.replace_range(range.clone(), &edit.replacement); // Now do the same to highlighting. let last_color = edit @@ -54,16 +50,10 @@ pub fn apply_edit(target: &mut WString, colors: &mut Vec, edit: & .checked_sub(1) .map(|i| colors[i]) .unwrap_or_default(); - if colors.is_empty() { - *colors = std::iter::repeat(last_color) - .take(edit.replacement.len()) - .collect(); - } else { - colors.splice( - range.clone(), - std::iter::repeat(last_color).take(edit.replacement.len()), - ); - } + colors.splice( + range.clone(), + std::iter::repeat(last_color).take(edit.replacement.len()), + ); } /// The history of all edits to some command line. @@ -195,11 +185,7 @@ pub fn push_edit(&mut self, mut edit: Edit, allow_coalesce: bool) { .truncate(self.undo_history.edits_applied); } edit.cursor_position_before_edit = self.pending_position.take().unwrap_or(self.position()); - edit.old = if self.text.is_empty() { - L!("").to_owned() - } else { - self.text[range.clone()].to_owned() - }; + edit.old = self.text[range.clone()].to_owned(); apply_edit(&mut self.text, &mut self.colors, &edit); self.set_position(cursor_position_after_edit(&edit)); assert_eq!( @@ -218,7 +204,7 @@ pub fn undo(&mut self) -> bool { let mut last_group_id = None; let position_before_undo = self.position(); let end = self.undo_history.edits_applied; - while !self.undo_history.edits.is_empty() && self.undo_history.edits_applied != 0 { + while self.undo_history.edits_applied != 0 { let edit = &self.undo_history.edits[self.undo_history.edits_applied - 1]; if did_undo && edit diff --git a/src/parser.rs b/src/parser.rs index 4ab2a3b04..981856009 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -232,6 +232,9 @@ pub struct ScopedData { /// Whether we are currently interactive. pub is_interactive: bool, + /// Whether the command line is closed for modification from fish script. + pub readonly_commandline: bool, + /// Whether to suppress fish_trace output. This occurs in the prompt, event handlers, and key /// bindings. pub suppress_fish_trace: bool, @@ -253,6 +256,7 @@ fn default() -> Self { eval_level: -1, is_subshell: false, is_event: false, + readonly_commandline: false, is_interactive: false, suppress_fish_trace: false, read_limit: 0, diff --git a/src/reader.rs b/src/reader.rs index f07aab2ed..21d4122ae 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -983,6 +983,9 @@ pub fn reader_schedule_prompt_repaint() { } pub fn reader_execute_readline_cmd(parser: &Parser, ch: CharEvent) { + if parser.scope().readonly_commandline { + return; + } if let Some(data) = current_data() { let mut data = Reader { parser, data }; let CharEvent::Readline(readline_cmd_evt) = &ch else { @@ -1062,7 +1065,10 @@ pub fn commandline_get_state(sync: bool) -> CommandlineState { /// Set the command line text and position. This may be called on a background thread; the reader /// will pick it up when it is done executing. -pub fn commandline_set_buffer(text: Option, cursor_pos: Option) { +pub fn commandline_set_buffer(parser: &Parser, text: Option, cursor_pos: Option) { + if parser.scope().readonly_commandline { + return; + } { let mut state = commandline_state_snapshot(); if let Some(text) = text { @@ -1073,7 +1079,10 @@ pub fn commandline_set_buffer(text: Option, cursor_pos: Option) current_data().map(|data| data.apply_commandline_state_changes()); } -pub fn commandline_set_search_field(text: WString, cursor_pos: Option) { +pub fn commandline_set_search_field(parser: &Parser, text: WString, cursor_pos: Option) { + if parser.scope().readonly_commandline { + return; + } { let mut state = commandline_state_snapshot(); assert!(state.search_field.is_some()); @@ -5321,7 +5330,10 @@ fn expand_replacer( let mut cmd = escape(&repl.replacement); cmd.push(' '); cmd.push_utfstr(&escape(token)); - let _not_interactive = parser.push_scope(|s| s.is_interactive = false); + let _not_interactive = parser.push_scope(|s| { + s.is_interactive = false; + s.readonly_commandline = true; + }); let mut outputs = vec![]; if exec_subshell(