From 58e7a50de8a1056834d52831ec3a897aea7396bf Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 10 Jan 2026 10:50:54 +0100 Subject: [PATCH] Fix line-wise autosuggestion false positive when line doesn't start command To reduce the likelihood of false positive line-wise history autosuggestions, we only them when the cursor's line starts a new process ("parse_util_process_extent"). There are still false positives. Given $ true && somecommand $ echo " someothercommand " typing "some" suggests "someothercommand" from history even though that was not actually used as command. Fix this by using similar rules for suggestion candidates. Might help #12290 --- src/highlight/highlight.rs | 14 ++++++-- src/parse_util.rs | 6 ++++ src/reader/reader.rs | 60 ++++++++++++++++++++--------------- tests/pexpects/autosuggest.py | 8 +++++ 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/highlight/highlight.rs b/src/highlight/highlight.rs index e94a61afe..bde670518 100644 --- a/src/highlight/highlight.rs +++ b/src/highlight/highlight.rs @@ -23,7 +23,8 @@ ParseKeyword, ParseTokenType, ParseTreeFlags, SourceRange, StatementDecoration, }; use crate::parse_util::{ - MaybeParentheses, parse_util_locate_cmdsubst_range, parse_util_slice_length, + MaybeParentheses, parse_util_locate_cmdsubst_range, parse_util_process_first_token_offset, + parse_util_slice_length, }; use crate::path::{path_as_implicit_cd, path_get_cdpath, path_get_path, paths_are_same_file}; use crate::terminal::Outputter; @@ -334,14 +335,23 @@ 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( - suggested_command: &wstr, + item_commandline: &wstr, + suggested_range: std::ops::Range, required_paths: &[WString], working_directory: &wstr, ctx: &OperationContext<'_>, ) -> bool { assert_is_background_thread(); + if suggested_range != (0..item_commandline.char_count()) + && parse_util_process_first_token_offset(item_commandline, suggested_range.start) + .is_some_and(|offset| offset != suggested_range.start) + { + return false; + } + // Parse the string. + let suggested_command = &item_commandline[suggested_range]; 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. diff --git a/src/parse_util.rs b/src/parse_util.rs index e0199b8da..756075d00 100644 --- a/src/parse_util.rs +++ b/src/parse_util.rs @@ -387,6 +387,12 @@ pub fn parse_util_process_extent( job_or_process_extent(true, buff, cursor_pos, out_tokens) } +pub fn parse_util_process_first_token_offset(buff: &wstr, cursor_pos: usize) -> Option { + let mut tokens = vec![]; + parse_util_process_extent(buff, cursor_pos, Some(&mut tokens)); + tokens.first().map(|tok| tok.offset()) +} + /// Find the beginning and end of the process definition under the cursor /// /// \param buff the string to search for subshells diff --git a/src/reader/reader.rs b/src/reader/reader.rs index 5472127b3..61aa5e0d7 100644 --- a/src/reader/reader.rs +++ b/src/reader/reader.rs @@ -106,6 +106,7 @@ use crate::parse_util::MaybeParentheses; use crate::parse_util::SPACES_PER_INDENT; use crate::parse_util::parse_util_process_extent; +use crate::parse_util::parse_util_process_first_token_offset; use crate::parse_util::{ parse_util_cmdsubst_extent, parse_util_compute_indents, parse_util_contains_wildcards, parse_util_detect_errors, parse_util_escape_string_with_quote, parse_util_escape_wildcards, @@ -4992,7 +4993,8 @@ fn get_autosuggestion_performer( parse_util_process_extent(&command_line, cursor_pos, Some(&mut tokens)); range_of_line_at_cursor( &command_line, - tokens.first().map_or(cursor_pos, |tok| tok.offset()), + parse_util_process_first_token_offset(&command_line, cursor_pos) + .unwrap_or(cursor_pos), ) == range }; if !cursor_line_has_process_start { @@ -5010,44 +5012,51 @@ fn get_autosuggestion_performer( while !ctx.check_cancel() && searcher.go_to_next_match(SearchDirection::Backward) { let item = searcher.current_item(); - let (suggested_command, icase) = if search_type == SearchType::Prefix { - let mut suggested_command = - item.str().starts_with(search_string).then_some(item.str()); + let full = item.str(); + let (suggested_range, icase) = if search_type == SearchType::Prefix { + let mut suggested_range = + full.starts_with(search_string).then_some(0..full.len()); let mut icase = false; // Only check for a case-insensitive match if we haven't already found one - if suggested_command.is_none() && icase_history_result.is_none() { + if suggested_range.is_none() && icase_history_result.is_none() { icase = true; - suggested_command = - string_prefixes_string_case_insensitive(search_string, item.str()) - .then_some(item.str()); + suggested_range = + string_prefixes_string_case_insensitive(search_string, full) + .then_some(0..full.len()); } - (suggested_command, icase) + (suggested_range, icase) } else { // The history items may have multiple lines of text. // Only suggest the line that actually contains the search string. - let lines = item - .str() - .as_char_slice() - .split(|&c| c == '\n') - .rev() - .map(wstr::from_char_slice); + let newlines = full + .char_indices() + .filter_map(|(i, c)| (c == '\n').then_some(i)); + let line_ranges = Some(0) + .into_iter() + .chain(newlines.clone().map(|i| i + 1)) + .zip(newlines.chain(Some(full.char_count()).into_iter())) + .map(|(start, end)| start..end); let mut icase = false; - let mut suggested_command = - lines.clone().find(|line| line.starts_with(search_string)); + let mut suggested_range = line_ranges + .clone() + .find(|range| full[range.clone()].starts_with(search_string)); // Only check for a case-insensitive match if we haven't already found one - if suggested_command.is_none() && icase_history_result.is_none() { + if suggested_range.is_none() && icase_history_result.is_none() { icase = true; - suggested_command = lines.into_iter().find(|line| { - string_prefixes_string_case_insensitive(search_string, line) + suggested_range = line_ranges.into_iter().find(|range| { + string_prefixes_string_case_insensitive( + search_string, + &full[range.clone()], + ) }); } - (suggested_command, icase) + (suggested_range, icase) }; - let Some(suggested_command) = suggested_command else { + let Some(suggested_range) = suggested_range 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?)" @@ -5056,17 +5065,18 @@ fn get_autosuggestion_performer( }; if autosuggest_validate_from_history( - suggested_command, + full, + suggested_range.clone(), item.get_required_paths(), &working_directory, &ctx, ) { // The command autosuggestion was handled specially, so we're done. - let is_whole = suggested_command.len() == item.str().len(); + let is_whole = suggested_range.len() == item.str().len(); let result = AutosuggestionResult::new( command_line.clone(), range.clone(), - suggested_command.into(), + full[suggested_range].into(), icase, is_whole, ); diff --git a/tests/pexpects/autosuggest.py b/tests/pexpects/autosuggest.py index 0858809f0..d27017bc9 100644 --- a/tests/pexpects/autosuggest.py +++ b/tests/pexpects/autosuggest.py @@ -80,3 +80,11 @@ run("rm configure") send("./con") use_suggestion() expect_prompt(">./con\r\n") + +send(control("c")) +run("touch somecommand") +run("chmod +x somecommand") +run('echo "multiline-token\n./somecommand arg1 arg2"') +send("./some") +use_suggestion() +expect_prompt(">./somecommand \r\n")