Add trailing slash (not space) to variable name completions that produce valid paths

Closes #5798

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,

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()
-                    env_name.slice_from(varlen).to_owned(),
-                    CompleteFlags::empty(),
-                )
             } else {
-                let comp = whole_var.slice_to(start_offset).to_owned() + env_name.as_utfstr();
+                flags |= CompleteFlags::REPLACES_TOKEN | CompleteFlags::DONT_ESCAPE;
-                let flags = CompleteFlags::REPLACES_TOKEN | CompleteFlags::DONT_ESCAPE;
+                whole_var.slice_to(start_offset).to_owned() + env_name.as_utfstr()
-                (comp, flags)
             };
 
             let mut desc = WString::new();

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(_, _))
     }

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<char> {
 ///
 /// 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 let Some(trailer) = trailer {
-            if !have_space_after_token {
+            if !have_trailer {
-                sb.push(' ');
+                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 let Some(mut trailer) = trailer {
-        if quote.is_some() && unescaped_quote(command_line, insertion_point) != quote {
+        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 {
+        if !have_trailer {
-            result.insert(new_cursor_pos, ' ');
+            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(),

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::<Vec<_>>(),
         [
             "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"),

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,