From 6e2c5d4365bd961cde4fd63613905c987e888d40 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 25 Jan 2025 09:04:38 +0100 Subject: [PATCH] Add trailing slash (not space) to variable name completions that produce valid paths Closes #5798 --- src/builtins/complete.rs | 2 ++ src/complete.rs | 15 ++++++------ src/operation_context.rs | 9 ++++++++ src/reader.rs | 49 ++++++++++++++++++++++++++++++++-------- src/tests/complete.rs | 10 +++++++- src/tests/reader.rs | 10 ++++++++ 6 files changed, 76 insertions(+), 19 deletions(-) diff --git a/src/builtins/complete.rs b/src/builtins/complete.rs index b8896bf22..618653834 100644 --- a/src/builtins/complete.rs +++ b/src/builtins/complete.rs @@ -4,6 +4,7 @@ use crate::complete::{complete_add_wrapper, complete_remove_wrapper, CompletionR use crate::highlight::colorize; use crate::highlight::highlight_shell; use crate::nix::isatty; +use crate::operation_context::OperationContext; use crate::parse_constants::ParseErrorList; use crate::parse_util::parse_util_detect_errors_in_argument_list; use crate::parse_util::{parse_util_detect_errors, parse_util_token_extent}; @@ -507,6 +508,7 @@ pub fn complete(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> let faux_cmdline = &do_complete_param[token.clone()]; let mut tmp_cursor = faux_cmdline.len(); let mut faux_cmdline_with_completion = completion_apply_to_command_line( + &OperationContext::background_interruptible(parser.vars()), &next.completion, next.flags, faux_cmdline, diff --git a/src/complete.rs b/src/complete.rs index d16745195..feea6a220 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -119,6 +119,8 @@ bitflags! { const REPLACES_LINE = 1 << 7; /// If replacing the entire token, keep the "foo=" prefix. const KEEP_VARIABLE_OVERRIDE_PREFIX = 1 << 8; + /// This is a variable name. + const VARIABLE_NAME = 1 << 9; } } @@ -1683,16 +1685,13 @@ impl<'ctx> Completer<'ctx> { continue; }; - let (comp, flags) = if !r#match.requires_full_replacement() { + let mut flags = CompleteFlags::VARIABLE_NAME; + let comp = if !r#match.requires_full_replacement() { // Take only the suffix. - ( - env_name.slice_from(varlen).to_owned(), - CompleteFlags::empty(), - ) + env_name.slice_from(varlen).to_owned() } else { - let comp = whole_var.slice_to(start_offset).to_owned() + env_name.as_utfstr(); - let flags = CompleteFlags::REPLACES_TOKEN | CompleteFlags::DONT_ESCAPE; - (comp, flags) + flags |= CompleteFlags::REPLACES_TOKEN | CompleteFlags::DONT_ESCAPE; + whole_var.slice_to(start_offset).to_owned() + env_name.as_utfstr() }; let mut desc = WString::new(); diff --git a/src/operation_context.rs b/src/operation_context.rs index 7569c2161..f1b7ffb1d 100644 --- a/src/operation_context.rs +++ b/src/operation_context.rs @@ -5,6 +5,7 @@ use crate::parser::Parser; use crate::proc::JobGroupRef; use crate::reader::read_generation_count; +use crate::signal::signal_check_cancel; /// A common helper which always returns false. pub fn no_cancel() -> bool { @@ -118,6 +119,14 @@ impl<'a> OperationContext<'a> { } } + pub fn background_interruptible(env: &dyn Environment) -> OperationContext { + OperationContext::background_with_cancel_checker( + env, + Box::new(|| signal_check_cancel() != 0), + EXPANSION_LIMIT_BACKGROUND, + ) + } + pub fn has_parser(&self) -> bool { matches!(self.vars, Vars::Parser(_) | Vars::TestOnly(_, _)) } diff --git a/src/reader.rs b/src/reader.rs index 9a5eccae0..8be78265c 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -59,8 +59,10 @@ use crate::complete::{ CompletionRequestOptions, }; use crate::editable_line::{line_at_cursor, range_of_line_at_cursor, Edit, EditableLine}; +use crate::env::EnvStack; use crate::env::{EnvMode, Environment, Statuses}; use crate::exec::exec_subshell; +use crate::expand::expand_one; use crate::expand::{expand_string, expand_tilde, ExpandFlags, ExpandResultCode}; use crate::fallback::fish_wcwidth; use crate::fd_readable_set::poll_fd_readable; @@ -144,6 +146,7 @@ use crate::wcstringutil::{ string_prefixes_string_case_insensitive, StringFuzzyMatch, }; use crate::wildcard::wildcard_has; +use crate::wutil::wstat; use crate::wutil::{fstat, perror}; use crate::{abbrs, event, function}; @@ -4061,6 +4064,7 @@ impl ReaderData { let new_cmd_line = match completion { None => Cow::Borrowed(&self.cycle_command_line), Some(completion) => Cow::Owned(completion_apply_to_command_line( + &OperationContext::background_interruptible(EnvStack::globals()), // To-do: include locals. &completion.completion, completion.flags, &self.cycle_command_line, @@ -4766,6 +4770,7 @@ fn get_autosuggestion_performer( sort_and_prioritize(&mut completions, complete_flags); let comp = &completions[0]; let full_line = completion_apply_to_command_line( + &OperationContext::background_interruptible(&vars), &comp.completion, comp.flags, &command_line, @@ -5986,22 +5991,25 @@ pub(crate) fn get_quote(cmd_str: &wstr, len: usize) -> Option { /// /// Return The completed string pub fn completion_apply_to_command_line( + ctx: &OperationContext, val_str: &wstr, flags: CompleteFlags, command_line: &wstr, inout_cursor_pos: &mut usize, append_only: bool, ) -> WString { - let add_space = !flags.contains(CompleteFlags::NO_SPACE); + let mut trailer = (!flags.contains(CompleteFlags::NO_SPACE)).then_some(' '); let do_replace_token = flags.contains(CompleteFlags::REPLACES_TOKEN); let do_replace_line = flags.contains(CompleteFlags::REPLACES_LINE); let do_escape = !flags.contains(CompleteFlags::DONT_ESCAPE); let no_tilde = flags.contains(CompleteFlags::DONT_ESCAPE_TILDES); let keep_variable_override = flags.contains(CompleteFlags::KEEP_VARIABLE_OVERRIDE_PREFIX); + let is_variable_name = flags.contains(CompleteFlags::VARIABLE_NAME); let cursor_pos = *inout_cursor_pos; let mut back_into_trailing_quote = false; - let have_space_after_token = command_line.char_at(cursor_pos) == ' '; + assert!(!is_variable_name || command_line.char_at(cursor_pos) != '/'); + let have_trailer = command_line.char_at(cursor_pos) == ' '; if do_replace_line { assert!(!do_escape, "unsupported completion flag"); @@ -6015,14 +6023,27 @@ pub fn completion_apply_to_command_line( } let mut escape_flags = EscapeFlags::empty(); - if append_only || !add_space { + if append_only || trailer.is_none() { escape_flags.insert(EscapeFlags::NO_QUOTED); } if no_tilde { escape_flags.insert(EscapeFlags::NO_TILDE); } + let maybe_add_slash = |trailer: &mut char, token: &wstr| { + let mut expanded = token.to_owned(); + if expand_one(&mut expanded, ExpandFlags::FAIL_ON_CMDSUBST, ctx, None) + && wstat(&expanded).is_ok_and(|md| md.is_dir()) + { + *trailer = '/'; + } + }; + if do_replace_token { + if is_variable_name { + assert!(!do_escape); + maybe_add_slash(trailer.as_mut().unwrap(), val_str); + } let mut move_cursor = 0; let (range, _) = parse_util_token_extent(command_line, cursor_pos); @@ -6045,9 +6066,9 @@ pub fn completion_apply_to_command_line( move_cursor += val_str.len(); } - if add_space { - if !have_space_after_token { - sb.push(' '); + if let Some(trailer) = trailer { + if !have_trailer { + sb.push(trailer); } move_cursor += 1; } @@ -6101,14 +6122,21 @@ pub fn completion_apply_to_command_line( result.insert_utfstr(insertion_point, &replaced); let mut new_cursor_pos = insertion_point + replaced.len() + if back_into_trailing_quote { 1 } else { 0 }; - if add_space { - if quote.is_some() && unescaped_quote(command_line, insertion_point) != quote { + if let Some(mut trailer) = trailer { + if is_variable_name { + let (tok, _) = parse_util_token_extent(command_line, cursor_pos); + maybe_add_slash(&mut trailer, &result[tok.start..new_cursor_pos]); + } + if trailer != '/' + && quote.is_some() + && unescaped_quote(command_line, insertion_point) != quote + { // This is a quoted parameter, first print a quote. result.insert(new_cursor_pos, quote.unwrap()); new_cursor_pos += 1; } - if !have_space_after_token { - result.insert(new_cursor_pos, ' '); + if !have_trailer { + result.insert(new_cursor_pos, trailer); } new_cursor_pos += 1; } @@ -6447,6 +6475,7 @@ impl<'a> Reader<'a> { let (_elt, el) = self.active_edit_line(); let mut cursor = el.position(); let new_command_line = completion_apply_to_command_line( + &OperationContext::background_interruptible(self.parser.vars()), val, flags, el.text(), diff --git a/src/tests/complete.rs b/src/tests/complete.rs index 21f267e30..316ce0a94 100644 --- a/src/tests/complete.rs +++ b/src/tests/complete.rs @@ -38,6 +38,8 @@ fn test_complete() { (WString::from_str("ALPHA!"), WString::new()), (WString::from_str("gamma1"), WString::new()), (WString::from_str("GAMMA2"), WString::new()), + (WString::from_str("SOMEDIR"), L!("/").to_owned()), + (WString::from_str("SOMEVAR"), WString::new()), ]), }, }; @@ -56,7 +58,7 @@ fn test_complete() { .collect::>(), [ "alpha", "ALPHA!", "Bar1", "Bar2", "Bar3", "Foo1", "Foo2", "Foo3", "gamma1", "GAMMA2", - "PWD" + "PWD", "SOMEDIR", "SOMEVAR", ] .into_iter() .map(|s| s.to_owned()) @@ -156,6 +158,7 @@ fn test_complete() { ); let mut cursor = cmdline.len(); let newcmdline = completion_apply_to_command_line( + &ctx, &completions[0].completion, completions[0].flags, cmdline, @@ -216,9 +219,14 @@ fn test_complete() { ); } + unique_completion_applies_as!("echo $SOMEV", r"AR", "echo $SOMEVAR "); + unique_completion_applies_as!("echo $SOMED", r"IR", "echo $SOMEDIR/"); + unique_completion_applies_as!(r#"echo "$SOMED"#, r"IR", r#"echo "$SOMEDIR/"#); + // #8820 let mut cursor_pos = 11; let newcmdline = completion_apply_to_command_line( + &ctx, L!("Debug/"), CompleteFlags::REPLACES_TOKEN | CompleteFlags::NO_SPACE, L!("mv debug debug"), diff --git a/src/tests/reader.rs b/src/tests/reader.rs index ffd3a7a4c..627daf442 100644 --- a/src/tests/reader.rs +++ b/src/tests/reader.rs @@ -1,5 +1,7 @@ use crate::complete::CompleteFlags; +use crate::operation_context::{no_cancel, OperationContext}; use crate::reader::{combine_command_and_autosuggestion, completion_apply_to_command_line}; +use crate::tests::prelude::*; use crate::wchar::prelude::*; #[test] @@ -47,6 +49,8 @@ fn test_autosuggestion_combining() { #[test] fn test_completion_insertions() { + let parser = TestParser::new(); + macro_rules! validate { ( $line:expr, $completion:expr, @@ -64,7 +68,13 @@ fn test_completion_insertions() { expected.remove(out_cursor_pos); let mut cursor_pos = in_cursor_pos; + let result = completion_apply_to_command_line( + &OperationContext::test_only_foreground( + &parser, + parser.vars(), + Box::new(no_cancel), + ), completion, $flags, &line,