Fix line-wise autosuggestion false positive when command doesn't exist

If my history has

	git clean -dxf &&
	./autogen.sh &&
	./configure --prefix=...

then autosuggestions for "./conf" will show the third line, even if
./configure does not exist.

This is because even for line-wise autosuggestions, we only check
validity of the first command ("git"). Fix that by checking
the command from the line that's actually suggested.

The next commit will fix the issue that line-wise autosuggestions
may not actually be commands.

See also #12290

src/highlight/highlight.rs

@@ -17,7 +17,7 @@
 use crate::function;
 use crate::future_feature_flags::{FeatureFlag, feature_test};
 use crate::highlight::file_tester::FileTester;
-use crate::history::{HistoryItem, all_paths_are_valid};
+use crate::history::all_paths_are_valid;
 use crate::operation_context::OperationContext;
 use crate::parse_constants::{
     ParseKeyword, ParseTokenType, ParseTreeFlags, SourceRange, StatementDecoration,
@@ -334,14 +334,16 @@ pub fn is_veritable_cd(expanded_command: &wstr) -> bool {
 /// autosuggestion is valid. It may not be valid if e.g. it is attempting to cd into a directory
 /// which does not exist.
 pub fn autosuggest_validate_from_history(
-    item: &HistoryItem,
+    suggested_command: &wstr,
+    required_paths: &[WString],
     working_directory: &wstr,
     ctx: &OperationContext<'_>,
 ) -> bool {
     assert_is_background_thread();
 
     // Parse the string.
-    let Some((parsed_command, mut cd_dir)) = autosuggest_parse_command(item.str(), ctx) else {
+    let Some((parsed_command, mut cd_dir)) = autosuggest_parse_command(suggested_command, ctx)
+    else {
         // This is for autosuggestions which are not decorated commands, e.g. function declarations.
         return true;
     };
@@ -373,8 +375,7 @@ pub fn autosuggest_validate_from_history(
     }
 
     // Did the historical command have arguments that look like paths, which aren't paths now?
-    let paths = item.get_required_paths();
-    if !all_paths_are_valid(paths.iter().cloned(), ctx) {
+    if !all_paths_are_valid(required_paths, ctx) {
         return false;
     }
 

src/history/history.rs

@@ -1733,13 +1733,12 @@ pub fn expand_and_detect_paths<P: IntoIterator<Item = WString>>(
 /// Given a list of proposed paths and a context, expand each one and see if it refers to a file.
 /// Wildcard expansions are suppressed.
 /// Returns `true` if `paths` is empty or every path is valid.
-pub fn all_paths_are_valid<P: IntoIterator<Item = WString>>(
-    paths: P,
-    ctx: &OperationContext<'_>,
-) -> bool {
+pub fn all_paths_are_valid(paths: &[WString], ctx: &OperationContext<'_>) -> bool {
     assert_is_background_thread();
     let working_directory = ctx.vars().get_pwd_slash();
-    for mut path in paths {
+    let mut path = WString::new();
+    for unexpanded_path in paths {
+        path.clone_from(unexpanded_path);
         if ctx.check_cancel() {
             return false;
         }

src/reader/reader.rs

@@ -5010,19 +5010,19 @@ fn get_autosuggestion_performer(
             while !ctx.check_cancel() && searcher.go_to_next_match(SearchDirection::Backward) {
                 let item = searcher.current_item();
 
-                let (matched_part, icase) = if search_type == SearchType::Prefix {
-                    let mut matched_part =
+                let (suggested_command, icase) = if search_type == SearchType::Prefix {
+                    let mut suggested_command =
                         item.str().starts_with(search_string).then_some(item.str());
                     let mut icase = false;
                     // Only check for a case-insensitive match if we haven't already found one
-                    if matched_part.is_none() && icase_history_result.is_none() {
+                    if suggested_command.is_none() && icase_history_result.is_none() {
                         icase = true;
-                        matched_part =
+                        suggested_command =
                             string_prefixes_string_case_insensitive(search_string, item.str())
                                 .then_some(item.str());
                     }
 
-                    (matched_part, icase)
+                    (suggested_command, icase)
                 } else {
                     // The history items may have multiple lines of text.
                     // Only suggest the line that actually contains the search string.
@@ -5034,20 +5034,20 @@ fn get_autosuggestion_performer(
                         .map(wstr::from_char_slice);
 
                     let mut icase = false;
-                    let mut matched_part =
+                    let mut suggested_command =
                         lines.clone().find(|line| line.starts_with(search_string));
 
                     // Only check for a case-insensitive match if we haven't already found one
-                    if matched_part.is_none() && icase_history_result.is_none() {
+                    if suggested_command.is_none() && icase_history_result.is_none() {
                         icase = true;
-                        matched_part = lines.into_iter().find(|line| {
+                        suggested_command = lines.into_iter().find(|line| {
                             string_prefixes_string_case_insensitive(search_string, line)
                         });
                     }
 
-                    (matched_part, icase)
+                    (suggested_command, icase)
                 };
-                let Some(matched_part) = matched_part else {
+                let Some(suggested_command) = suggested_command else {
                     assert!(
                         icase_history_result.is_some(),
                         "couldn't find line matching search {search_string:?} in history item {item:?} (did history search yield a bogus result?)"
@@ -5055,13 +5055,18 @@ fn get_autosuggestion_performer(
                     continue;
                 };
 
-                if autosuggest_validate_from_history(item, &working_directory, &ctx) {
+                if autosuggest_validate_from_history(
+                    suggested_command,
+                    item.get_required_paths(),
+                    &working_directory,
+                    &ctx,
+                ) {
                     // The command autosuggestion was handled specially, so we're done.
-                    let is_whole = matched_part.len() == item.str().len();
+                    let is_whole = suggested_command.len() == item.str().len();
                     let result = AutosuggestionResult::new(
                         command_line.clone(),
                         range.clone(),
-                        matched_part.into(),
+                        suggested_command.into(),
                         icase,
                         is_whole,
                     );

tests/pexpects/autosuggest.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-from pexpect_helper import SpawnedProc
+from pexpect_helper import SpawnedProc, control
 
 sp = SpawnedProc()
 send, sendline, sleep, expect_prompt = (
@@ -72,3 +72,11 @@ run("mkdir problems")
 send("cd pro")
 use_suggestion(delay=0.5)
 expect_prompt(">cd problems/\r\n")
+
+send(control("c"))
+run("touch configure && chmod +x configure")
+run("echo clean &&\n./configure")
+run("rm configure")
+send("./con")
+use_suggestion()
+expect_prompt(">./con\r\n")