h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-05-29 18:51:15 -03:00
Code Issues Packages Projects Releases Wiki Activity

Prevent commandline modification inside abbreviation callbacks

Browse Source 
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
This commit is contained in:
Johannes Altmanninger
2025-03-26 20:38:31 +01:00
parent d88f5ddbaf
commit 11c7310f17
5 changed files with 39 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/builtins/commandline.rs
View File

@@ -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<usize>,
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,

7
src/builtins/read.rs
View File

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

28
src/editable_line.rs
View File

@@ -41,11 +41,7 @@ pub fn new(range: std::ops::Range<usize>, replacement: WString) -> Self {
/// Currently exposed for testing only.
pub fn apply_edit(target: &mut WString, colors: &mut Vec<HighlightSpec>, 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<HighlightSpec>, 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

4
src/parser.rs
View File

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

18
src/reader.rs
View File

@@ -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<WString>, cursor_pos: Option<usize>) {
pub fn commandline_set_buffer(parser: &Parser, text: Option<WString>, cursor_pos: Option<usize>) {
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<WString>, cursor_pos: Option<usize>)
current_data().map(|data| data.apply_commandline_state_changes());
}
pub fn commandline_set_search_field(text: WString, cursor_pos: Option<usize>) {
pub fn commandline_set_search_field(parser: &Parser, text: WString, cursor_pos: Option<usize>) {
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(