From 0f1408e0ea2c22c32b254f57fa7ba4322d0749d8 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Mon, 6 Jan 2025 16:53:09 +0100 Subject: [PATCH] Also autosuggest lines from multi-line command lines in history My history often has erroneous single-line commands followed by corrected versions. Sometimes the corrected versions only exist within a multi-line commandline. This means that autosuggestion skips over the corrected versions and return a false positive. Fix that by splitting the commandline into lines and suggesting those, in reverse chronological order. One other wart: shift-delete won't delete such autosuggestions from history; instead it will flash the screen. Line boundaries are not the best heuristic but they are an improvement for the most part and fits with the current approach where autosuggestion always operates on the entire line. In future we should operate on processes and jobs. But it may be tricky - a backgrounding `&` should probably be included (in both?) but `&&` or `;` probably not. See also the discussion in https://github.com/fish-shell/fish-shell/commit/1c4e5cadf23d38350fd281bac85a6908c2414ce2#diff-267c9f4da66412a9f439ac08d224356fe24265b5e1cebb6c44c2d55b96414513R59 --- CHANGELOG.rst | 3 ++- src/history.rs | 6 ++++++ src/reader.rs | 34 +++++++++++++++++++++------------- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e5d8bd35f..473824675 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,7 +12,8 @@ Scripting improvements Interactive improvements ------------------------ -- Autosuggestions are now also provided in multi-line command lines. Like `ctrl-r`, autosuggestions operate only on the current line. +- Autosuggestions are now also provided in multi-line command lines. Like `ctrl-r`, autosuggestions operate only on the current line. +- Autosuggestions used to not suggest multi-line commandlines from history; now autosuggestions include individual lines from multi-line command lines. - New feature flag ``buffered-enter-noexec`` with the following effect: when typing a command and :kbd:`enter` while the previous one is still running, the new one will no longer execute immediately. Similarly, keys that are bound to shell commands will be ignored. This mitigates a security issue where a command like ``cat malicious-file.txt`` could write terminal escape codes prompting the terminal to write arbitrary text to fish's standard input. diff --git a/src/history.rs b/src/history.rs index 458836a41..e1aa9d85f 100644 --- a/src/history.rs +++ b/src/history.rs @@ -91,6 +91,8 @@ pub enum SearchType { Contains, /// Search for commands starting with the given string. Prefix, + /// Search for commands where any line matches the given string. + LinePrefix, /// Search for commands containing the given glob pattern. ContainsGlob, /// Search for commands starting with the given glob pattern. @@ -291,6 +293,10 @@ pub fn matches_search(&self, term: &wstr, typ: SearchType, case_sensitive: bool) find_subslice(term.as_slice(), content_to_match.as_slice()).is_some() } SearchType::Prefix => content_to_match.as_slice().starts_with(term.as_slice()), + SearchType::LinePrefix => content_to_match + .as_char_slice() + .split(|&c| c == '\n') + .any(|line| line.starts_with(term.as_char_slice())), SearchType::ContainsGlob => { let mut pat = parse_util_unescape_wildcards(term); if !pat.starts_with(ANY_STRING) { diff --git a/src/reader.rs b/src/reader.rs index 1d201a09b..e42ad1383 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -2977,7 +2977,7 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { let is_autosuggestion = self.is_at_autosuggestion(); if is_history_search || is_autosuggestion { self.input_data.function_set_status(true); - if is_autosuggestion && !self.autosuggestion.is_from_history { + if is_autosuggestion && !self.autosuggestion.is_whole_item_from_history { self.flash(); return; } @@ -4532,7 +4532,7 @@ struct Autosuggestion { icase: bool, // Whether the autosuggestion is a whole match from history. - is_from_history: bool, + is_whole_item_from_history: bool, } impl Autosuggestion { @@ -4573,14 +4573,14 @@ fn new( search_string_range: Range, text: WString, icase: bool, - is_from_history: bool, + is_whole_item_from_history: bool, ) -> Self { Self { autosuggestion: Autosuggestion { text, search_string_range, icase, - is_from_history, + is_whole_item_from_history, }, command_line, needs_load: vec![], @@ -4625,25 +4625,33 @@ fn get_autosuggestion_performer( parse_util_process_extent(&command_line, cursor_pos, None).start, ) == search_string_range { - let mut searcher = - HistorySearch::new_with_type(history, search_string.to_owned(), SearchType::Prefix); + let mut searcher = HistorySearch::new_with_type( + history, + search_string.to_owned(), + SearchType::LinePrefix, + ); while !ctx.check_cancel() && searcher.go_to_next_match(SearchDirection::Backward) { let item = searcher.current_item(); - // Skip items with newlines because they make terrible autosuggestions. - if item.str().contains('\n') { - continue; - } + // Suggest only a single line each time. + let matched_line = item + .str() + .as_char_slice() + .split(|&c| c == '\n') + .rev() + .find(|line| line.starts_with(search_string.as_char_slice())) + .unwrap(); if autosuggest_validate_from_history(item, &working_directory, &ctx) { // The command autosuggestion was handled specially, so we're done. // History items are case-sensitive, see #3978. + let is_whole = matched_line.len() == item.str().len(); return AutosuggestionResult::new( command_line, search_string_range, - searcher.current_string().to_owned(), + matched_line.into(), /*icase=*/ false, - /*is_history=*/ true, + is_whole, ); } } @@ -4693,7 +4701,7 @@ fn get_autosuggestion_performer( search_string_range.clone(), suggestion, true, // normal completions are case-insensitive - /*is_from_history=*/ false, + /*is_whole_item_from_history=*/ false, ); result.needs_load = needs_load; result