From 80d92dcc6d6ec06f6dd08825c114649a0e879fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20H=C3=B8rl=C3=BCck=20Berg?= <36937807+henrikhorluck@users.noreply.github.com> Date: Sat, 15 Jul 2023 02:24:48 +0200 Subject: [PATCH 1/7] Port the easy part of wildcard.{h,cpp} - wildcard_match is now closer to the original that is linked in a comment, as pointer-arithmetic translates very poorly. The act of calling wildcard patterns wc or wildcard is kinda confusing when wc elsewhere is widechar. --- fish-rust/build.rs | 1 + fish-rust/src/builtins/string/match.rs | 7 +- fish-rust/src/ffi.rs | 2 - fish-rust/src/flog.rs | 5 +- fish-rust/src/wildcard.rs | 217 ++++++++++++++++++++++++- src/fish_tests.cpp | 23 --- src/wildcard.cpp | 95 ----------- src/wildcard.h | 27 ++- 8 files changed, 240 insertions(+), 137 deletions(-) diff --git a/fish-rust/build.rs b/fish-rust/build.rs index e69e9c674..3e6ef0ccb 100644 --- a/fish-rust/build.rs +++ b/fish-rust/build.rs @@ -87,6 +87,7 @@ fn main() { "fish-rust/src/trace.rs", "fish-rust/src/util.rs", "fish-rust/src/wait_handle.rs", + "fish-rust/src/wildcard.rs", ]; cxx_build::bridges(&source_files) .flag_if_supported("-std=c++11") diff --git a/fish-rust/src/builtins/string/match.rs b/fish-rust/src/builtins/string/match.rs index 64495224f..62c268cbb 100644 --- a/fish-rust/src/builtins/string/match.rs +++ b/fish-rust/src/builtins/string/match.rs @@ -6,8 +6,7 @@ use super::*; use crate::env::{EnvMode, EnvVar, EnvVarFlags}; use crate::flog::FLOG; use crate::parse_util::parse_util_unescape_wildcards; -use crate::wchar_ffi::WCharToFFI; -use crate::wildcard::ANY_STRING; +use crate::wildcard::{wildcard_match, ANY_STRING}; #[derive(Default)] pub struct Match<'args> { @@ -380,13 +379,11 @@ impl<'opts, 'args> WildCardMatcher<'opts, 'args> { fn report_matches(&mut self, arg: &wstr, streams: &mut io_streams_t) { // Note: --all is a no-op for glob matching since the pattern is always matched // against the entire argument. - use crate::ffi::wildcard_match; - let subject = match self.opts.ignore_case { true => arg.to_lowercase(), false => arg.to_owned(), }; - let m = wildcard_match(&subject.to_ffi(), &self.pattern.to_ffi(), false); + let m = wildcard_match(subject, &self.pattern, false); if m ^ self.opts.invert_match { self.total_matched += 1; diff --git a/fish-rust/src/ffi.rs b/fish-rust/src/ffi.rs index 7e3cdf9d2..fdfa22ce7 100644 --- a/fish-rust/src/ffi.rs +++ b/fish-rust/src/ffi.rs @@ -45,7 +45,6 @@ include_cpp! { #include "reader.h" #include "screen.h" #include "tokenizer.h" - #include "wildcard.h" #include "wutil.h" // We need to block these types so when exposing C++ to Rust. @@ -92,7 +91,6 @@ include_cpp! { generate!("log_extra_to_flog_file") - generate!("wildcard_match") generate!("wgettext_ptr") generate!("block_t") diff --git a/fish-rust/src/flog.rs b/fish-rust/src/flog.rs index 4e1f3ddeb..14e209979 100644 --- a/fish-rust/src/flog.rs +++ b/fish-rust/src/flog.rs @@ -1,7 +1,6 @@ -use crate::ffi::wildcard_match; use crate::parse_util::parse_util_unescape_wildcards; use crate::wchar::prelude::*; -use crate::wchar_ffi::WCharToFFI; +use crate::wildcard::wildcard_match; use libc::c_int; use std::io::Write; use std::os::unix::prelude::*; @@ -212,7 +211,7 @@ fn apply_one_wildcard(wc_esc: &wstr, sense: bool) { let wc = parse_util_unescape_wildcards(wc_esc); let mut match_found = false; for cat in categories::all_categories() { - if wildcard_match(&cat.name.to_ffi(), &wc.to_ffi(), false) { + if wildcard_match(cat.name, &wc, false) { cat.enabled.store(sense, Ordering::Relaxed); match_found = true; } diff --git a/fish-rust/src/wildcard.rs b/fish-rust/src/wildcard.rs index 00b773743..1f6608f72 100644 --- a/fish-rust/src/wildcard.rs +++ b/fish-rust/src/wildcard.rs @@ -1,6 +1,14 @@ // Enumeration of all wildcard types. -use crate::common::{char_offset, WILDCARD_RESERVED_BASE}; +use cxx::CxxWString; + +use crate::common::{ + char_offset, unescape_string, UnescapeFlags, UnescapeStringStyle, WILDCARD_RESERVED_BASE, +}; +use crate::future_feature_flags::feature_test; +use crate::future_feature_flags::FeatureFlag; +use crate::wchar::prelude::*; +use crate::wchar_ffi::WCharFromFFI; /// Character representing any character except '/' (slash). pub const ANY_CHAR: char = char_offset(WILDCARD_RESERVED_BASE, 0); @@ -11,3 +19,210 @@ pub const ANY_STRING_RECURSIVE: char = char_offset(WILDCARD_RESERVED_BASE, 2); /// This is a special pseudo-char that is not used other than to mark the /// end of the the special characters so we can sanity check the enum range. pub const ANY_SENTINEL: char = char_offset(WILDCARD_RESERVED_BASE, 3); + +/// Expand the wildcard by matching against the filesystem. +/// +/// wildcard_expand works by dividing the wildcard into segments at each directory boundary. Each +/// segment is processed separately. All except the last segment are handled by matching the +/// wildcard segment against all subdirectories of matching directories, and recursively calling +/// wildcard_expand for matches. On the last segment, matching is made to any file, and all matches +/// are inserted to the list. +/// +/// If wildcard_expand encounters any errors (such as insufficient privileges) during matching, no +/// error messages will be printed and wildcard_expand will continue the matching process. +/// +/// \param wc The wildcard string +/// \param working_directory The working directory +/// \param flags flags for the search. Can be any combination of for_completions and +/// executables_only +/// \param output The list in which to put the output +/// +enum WildcardResult { + /// The wildcard did not match. + NoMatch, + /// The wildcard did match. + Match, + /// Expansion was cancelled (e.g. control-C). + Cancel, + /// Expansion produced too many results. + Overflow, +} + +// pub fn wildcard_expand_string(wc: &wstr, working_directory: &wstr, flags: ExpandFlags, cancel_checker: impl CancelChecker, output: *mut completion_receiver_t) -> WildcardResult { +// todo!() +// } + +/// Test whether the given wildcard matches the string. Does not perform any I/O. +/// +/// \param str The string to test +/// \param wc The wildcard to test against +/// \param leading_dots_fail_to_match if set, strings with leading dots are assumed to be hidden +/// files and are not matched (default was false) +/// +/// \return true if the wildcard matched +#[must_use] +pub fn wildcard_match( + name: impl AsRef, + pattern: impl AsRef, + leading_dots_fail_to_match: bool, +) -> bool { + let name = name.as_ref(); + let pattern = pattern.as_ref(); + // Hackish fix for issue #270. Prevent wildcards from matching . or .., but we must still allow + // literal matches. + if leading_dots_fail_to_match && (name == L!(".") || name == L!("..")) { + // The string is '.' or '..' so the only possible match is an exact match. + return name == pattern; + } + + // Near Linear implementation as proposed here https://research.swtch.com/glob. + let mut px = 0; + let mut nx = 0; + let mut next_px = 0; + let mut next_nx = 0; + + while px < pattern.len() || nx < name.len() { + if px < pattern.len() { + match pattern.char_at(px) { + ANY_STRING | ANY_STRING_RECURSIVE => { + // Ignore hidden file + if leading_dots_fail_to_match && nx == 0 && name.char_at(0) == '.' { + return false; + } + + // Common case of * at the end. In that case we can early out since we know it will + // match. + if px == pattern.len() - 1 { + return true; + } + + // Try to match at nx. + // If that doesn't work out, restart at nx+1 next. + next_px = px; + next_nx = nx + 1; + px += 1; + continue; + } + ANY_CHAR => { + if nx < name.len() { + if nx == 0 && name.char_at(nx) == '.' { + return false; + } + + px += 1; + nx += 1; + continue; + } + } + c => { + // ordinary char + if nx < name.len() && name.char_at(nx) == c { + px += 1; + nx += 1; + continue; + } + } + } + } + + // Mismatch. Maybe restart. + if 0 < next_nx && next_nx <= name.len() { + px = next_px; + nx = next_nx; + continue; + } + return false; + } + // Matched all of pattern to all of name. Success. + true +} + +// Check if the string has any unescaped wildcards (e.g. ANY_STRING). +#[inline] +#[must_use] +fn wildcard_has_internal(s: impl AsRef) -> bool { + s.as_ref() + .chars() + .any(|c| matches!(c, ANY_STRING | ANY_STRING_RECURSIVE | ANY_CHAR)) +} + +/// Check if the specified string contains wildcards (e.g. *). +#[must_use] +fn wildcard_has(s: impl AsRef) -> bool { + let s = s.as_ref(); + let qmark_is_wild = !feature_test(FeatureFlag::qmark_noglob); + // Fast check for * or ?; if none there is no wildcard. + // Note some strings contain * but no wildcards, e.g. if they are quoted. + if !s.contains('*') && (!qmark_is_wild || !s.contains('?')) { + return false; + } + let unescaped = + unescape_string(s, UnescapeStringStyle::Script(UnescapeFlags::SPECIAL)).unwrap_or_default(); + return wildcard_has_internal(unescaped); +} + +/// Test wildcard completion. +// pub fn wildcard_complete(str: &wstr, wc: &wstr, desc_func: impl Fn(&wstr) -> WString, out: *mut completion_receiver_t, expand_flags: ExpandFlags, flags: CompleteFlags) -> bool { +// todo!() +// } + +#[cfg(test)] +mod tests { + use super::*; + use crate::future_feature_flags::scoped_test; + + #[test] + fn test_wildcards() { + assert!(!wildcard_has(L!(""))); + assert!(wildcard_has(L!("*"))); + assert!(!wildcard_has(L!("\\*"))); + + let wc = L!("foo*bar"); + assert!(wildcard_has(wc) && !wildcard_has_internal(wc)); + let wc = unescape_string(wc, UnescapeStringStyle::Script(UnescapeFlags::SPECIAL)).unwrap(); + assert!(!wildcard_has(&wc) && wildcard_has_internal(&wc)); + + scoped_test(FeatureFlag::qmark_noglob, false, || { + assert!(wildcard_has(L!("?"))); + assert!(!wildcard_has(L!("\\?"))); + }); + + scoped_test(FeatureFlag::qmark_noglob, true, || { + assert!(!wildcard_has(L!("?"))); + assert!(!wildcard_has(L!("\\?"))); + }); + } +} + +#[cxx::bridge] +mod ffi { + extern "C++" { + include!("wutil.h"); + } + extern "Rust" { + #[cxx_name = "wildcard_match_ffi"] + fn wildcard_match_ffi( + str: &CxxWString, + wc: &CxxWString, + leading_dots_fail_to_match: bool, + ) -> bool; + + #[cxx_name = "wildcard_has"] + fn wildcard_has_ffi(s: &CxxWString) -> bool; + + #[cxx_name = "wildcard_has_internal"] + fn wildcard_has_internal_ffi(s: &CxxWString) -> bool; + } +} + +fn wildcard_match_ffi(str: &CxxWString, wc: &CxxWString, leading_dots_fail_to_match: bool) -> bool { + wildcard_match(str.from_ffi(), wc.from_ffi(), leading_dots_fail_to_match) +} + +fn wildcard_has_ffi(s: &CxxWString) -> bool { + wildcard_has(s.from_ffi()) +} + +fn wildcard_has_internal_ffi(s: &CxxWString) -> bool { + wildcard_has_internal(s.from_ffi()) +} diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 7c4d86c7b..722aeb8af 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2444,28 +2444,6 @@ static void test_autoload() { autoload_tester_t::run_test(); } -static void test_wildcards() { - say(L"Testing wildcards"); - do_test(!wildcard_has(L"")); - do_test(wildcard_has(L"*")); - do_test(!wildcard_has(L"\\*")); - do_test(!wildcard_has(L"\"*\"")); - - wcstring wc = L"foo*bar"; - do_test(wildcard_has(wc) && !wildcard_has_internal(wc)); - unescape_string_in_place(&wc, UNESCAPE_SPECIAL); - do_test(!wildcard_has(wc) && wildcard_has_internal(wc)); - - auto saved = feature_test(feature_flag_t::qmark_noglob); - feature_set(feature_flag_t::qmark_noglob, false); - do_test(wildcard_has(L"?")); - do_test(!wildcard_has(L"\\?")); - feature_set(feature_flag_t::qmark_noglob, true); - do_test(!wildcard_has(L"?")); - do_test(!wildcard_has(L"\\?")); - feature_set(feature_flag_t::qmark_noglob, saved); -} - static void test_complete() { say(L"Testing complete"); @@ -5595,7 +5573,6 @@ static const test_t s_tests[]{ {TEST_GROUP("word_motion"), test_word_motion}, {TEST_GROUP("is_potential_path"), test_is_potential_path}, {TEST_GROUP("colors"), test_colors}, - {TEST_GROUP("wildcard"), test_wildcards}, {TEST_GROUP("complete"), test_complete}, {TEST_GROUP("autoload"), test_autoload}, {TEST_GROUP("input"), test_input}, diff --git a/src/wildcard.cpp b/src/wildcard.cpp index e88cdb220..95fb2d82b 100644 --- a/src/wildcard.cpp +++ b/src/wildcard.cpp @@ -39,101 +39,6 @@ static size_t wildcard_find(const wchar_t *wc) { return wcstring::npos; } -bool wildcard_has_internal(const wchar_t *s, size_t len) { - for (size_t i = 0; i < len; i++) { - wchar_t c = s[i]; - if (c == ANY_CHAR || c == ANY_STRING || c == ANY_STRING_RECURSIVE) { - return true; - } - } - return false; -} - -// Note we want to handle embedded nulls (issue #1631). -bool wildcard_has(const wchar_t *str, size_t len) { - assert(str != nullptr); - const wchar_t *end = str + len; - bool qmark_is_wild = !feature_test(feature_flag_t::qmark_noglob); - // Fast check for * or ?; if none there is no wildcard. - // Note some strings contain * but no wildcards, e.g. if they are quoted. - if (std::find(str, end, L'*') == end && (!qmark_is_wild || std::find(str, end, L'?') == end)) { - return false; - } - wcstring unescaped; - if (auto tmp = unescape_string(wcstring{str, len}, UNESCAPE_SPECIAL)) { - unescaped = *tmp; - } - return wildcard_has_internal(unescaped); -} - -/// Check whether the string str matches the wildcard string wc. -/// -/// \param str String to be matched. -/// \param wc The wildcard. -/// \param leading_dots_fail_to_match Whether files beginning with dots should not be matched -/// against wildcards. -bool wildcard_match(const wcstring &str, const wcstring &wc, bool leading_dots_fail_to_match) { - // Hackish fix for issue #270. Prevent wildcards from matching . or .., but we must still allow - // literal matches. - if (leading_dots_fail_to_match && (str == L"." || str == L"..")) { - // The string is '.' or '..' so the only possible match is an exact match. - return str == wc; - } - - // Near Linear implementation as proposed here https://research.swtch.com/glob. - const wchar_t *const str_start = str.c_str(); - const wchar_t *wc_x = wc.c_str(); - const wchar_t *str_x = str_start; - const wchar_t *restart_wc_x = wc.c_str(); - const wchar_t *restart_str_x = str_start; - - bool restart_is_out_of_str = false; - for (; *wc_x != 0 || *str_x != 0;) { - bool is_first = (str_x == str_start); - if (*wc_x != 0) { - if (*wc_x == ANY_STRING || *wc_x == ANY_STRING_RECURSIVE) { - // Ignore hidden file - if (leading_dots_fail_to_match && is_first && str[0] == L'.') { - return false; - } - - // Common case of * at the end. In that case we can early out since we know it will - // match. - if (wc_x[1] == L'\0') { - return true; - } - // Try to match at str_x. - // If that doesn't work out, restart at str_x+1 next. - restart_wc_x = wc_x; - restart_str_x = str_x + 1; - restart_is_out_of_str = (*str_x == 0); - wc_x++; - continue; - } else if (*wc_x == ANY_CHAR && *str_x != 0) { - if (is_first && *str_x == L'.') { - return false; - } - wc_x++; - str_x++; - continue; - } else if (*str_x != 0 && *str_x == *wc_x) { // ordinary character - wc_x++; - str_x++; - continue; - } - } - // Mismatch. Maybe restart. - if (restart_str_x != str.c_str() && !restart_is_out_of_str) { - wc_x = restart_wc_x; - str_x = restart_str_x; - continue; - } - return false; - } - // Matched all of pattern to all of name. Success. - return true; -} - // This does something horrible refactored from an even more horrible function. static wcstring resolve_description(const wcstring &full_completion, wcstring *completion, expand_flags_t expand_flags, diff --git a/src/wildcard.h b/src/wildcard.h index 55a76fb96..d60c37fb6 100644 --- a/src/wildcard.h +++ b/src/wildcard.h @@ -75,6 +75,11 @@ wildcard_result_t wildcard_expand_string(const wcstring &wc, const wcstring &wor const cancel_checker_t &cancel_checker, completion_receiver_t *output); +#if INCLUDE_RUST_HEADERS + +#include "wildcard.rs.h" + +#else /// Test whether the given wildcard matches the string. Does not perform any I/O. /// /// \param str The string to test @@ -83,18 +88,24 @@ wildcard_result_t wildcard_expand_string(const wcstring &wc, const wcstring &wor /// files and are not matched /// /// \return true if the wildcard matched -bool wildcard_match(const wcstring &str, const wcstring &wc, - bool leading_dots_fail_to_match = false); +bool wildcard_match_ffi(const wcstring &str, const wcstring &wc, bool leading_dots_fail_to_match); // Check if the string has any unescaped wildcards (e.g. ANY_STRING). -bool wildcard_has_internal(const wchar_t *s, size_t len); -inline bool wildcard_has_internal(const wcstring &s) { - return wildcard_has_internal(s.c_str(), s.size()); -} +bool wildcard_has_internal(const wcstring &s); /// Check if the specified string contains wildcards (e.g. *). -bool wildcard_has(const wchar_t *s, size_t len); -inline bool wildcard_has(const wcstring &s) { return wildcard_has(s.c_str(), s.size()); } +bool wildcard_has(const wcstring &s); + +#endif + +inline bool wildcard_match(const wcstring &str, const wcstring &wc, + bool leading_dots_fail_to_match = false) { + return wildcard_match_ffi(str, wc, leading_dots_fail_to_match); + } + +inline bool wildcard_has(const wchar_t *s, size_t len) { + return wildcard_has(wcstring(s, len)); +}; /// Test wildcard completion. wildcard_result_t wildcard_complete(const wcstring &str, const wchar_t *wc, From e0bbf3eee9611bad4bc4828a3a03f64e589f8eb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20H=C3=B8rl=C3=BCck=20Berg?= <36937807+henrikhorluck@users.noreply.github.com> Date: Wed, 26 Jul 2023 15:03:03 +0200 Subject: [PATCH 2/7] Port wilcard.{cpp,h} to Rust - This is untested and unused, string ownership is very much subject to change - Ports the minimally necessary parts of complete.rs as well - This should fix an infinite loop in `create_directory` in `path.rs`, the first `wstat` loop only breaks if it fails with an error that's different from EAGAIN --- fish-rust/src/common.rs | 2 +- fish-rust/src/complete.rs | 181 ++++++ fish-rust/src/lib.rs | 1 + fish-rust/src/wcstringutil.rs | 6 +- fish-rust/src/wildcard.rs | 1100 ++++++++++++++++++++++++++++++++- 5 files changed, 1267 insertions(+), 23 deletions(-) create mode 100644 fish-rust/src/complete.rs diff --git a/fish-rust/src/common.rs b/fish-rust/src/common.rs index e8855792c..3ad25510e 100644 --- a/fish-rust/src/common.rs +++ b/fish-rust/src/common.rs @@ -1258,7 +1258,7 @@ pub fn assert_is_background_thread() { /// Format the specified size (in bytes, kilobytes, etc.) into the specified stringbuffer. #[widestrs] -fn format_size(mut sz: i64) -> WString { +pub fn format_size(mut sz: i64) -> WString { let mut result = WString::new(); const sz_names: [&wstr; 8] = ["kB"L, "MB"L, "GB"L, "TB"L, "PB"L, "EB"L, "ZB"L, "YB"L]; if sz < 0 { diff --git a/fish-rust/src/complete.rs b/fish-rust/src/complete.rs new file mode 100644 index 000000000..3d5a1f486 --- /dev/null +++ b/fish-rust/src/complete.rs @@ -0,0 +1,181 @@ +/// Prototypes for functions related to tab-completion. +/// +/// These functions are used for storing and retrieving tab-completion data, as well as for +/// performing tab-completion. +use crate::wchar::prelude::*; +use crate::wcstringutil::StringFuzzyMatch; +use bitflags::bitflags; + +#[derive(Default, Debug)] +pub struct CompletionMode { + /// If set, skip file completions. + pub no_files: bool, + pub force_files: bool, + /// If set, require a parameter after completion. + pub requires_param: bool, +} + +/// Character that separates the completion and description on programmable completions. +pub const PROG_COMPLETE_SEP: char = '\t'; + +bitflags! { + #[derive(Default)] + pub struct CompleteFlags: u8 { + /// Do not insert space afterwards if this is the only completion. (The default is to try insert + /// a space). + const NO_SPACE = 1 << 0; + /// This is not the suffix of a token, but replaces it entirely. + const REPLACES_TOKEN = 1 << 1; + /// This completion may or may not want a space at the end - guess by checking the last + /// character of the completion. + const AUTO_SPACE = 1 << 2; + /// This completion should be inserted as-is, without escaping. + const DONT_ESCAPE = 1 << 3; + /// If you do escape, don't escape tildes. + const DONT_ESCAPE_TILDES = 1 << 4; + /// Do not sort supplied completions + const DONT_SORT = 1 << 5; + /// This completion looks to have the same string as an existing argument. + const DUPLICATES_ARGUMENT = 1 << 6; + /// This completes not just a token but replaces the entire commandline. + const REPLACES_COMMANDLINE = 1 << 7; + } +} + +#[derive(Debug)] +pub struct Completion { + pub completion: WString, + pub description: WString, + pub r#match: StringFuzzyMatch, + pub flags: CompleteFlags, +} + +impl Default for Completion { + fn default() -> Self { + Self { + completion: Default::default(), + description: Default::default(), + r#match: StringFuzzyMatch::exact_match(), + flags: Default::default(), + } + } +} + +impl From for Completion { + fn from(completion: WString) -> Completion { + Completion { + completion, + ..Default::default() + } + } +} + +impl Completion { + /// \return whether this replaces its token. + pub fn replaces_token(&self) -> bool { + self.flags.contains(CompleteFlags::REPLACES_TOKEN) + } + /// \return whether this replaces the entire commandline. + pub fn replaces_commandline(&self) -> bool { + self.flags.contains(CompleteFlags::REPLACES_COMMANDLINE) + } + + /// \return the completion's match rank. Lower ranks are better completions. + pub fn rank(&self) -> u32 { + self.r#match.rank() + } + + /// If this completion replaces the entire token, prepend a prefix. Otherwise do nothing. + pub fn prepend_token_prefix(&mut self, prefix: impl AsRef) { + if self.flags.contains(CompleteFlags::REPLACES_TOKEN) { + self.completion.insert_utfstr(0, prefix.as_ref()); + } + } +} + +/// A completion receiver accepts completions. It is essentially a wrapper around Vec with +/// some conveniences. +#[derive(Default, Debug)] +pub struct CompletionReceiver { + /// Our list of completions. + completions: Vec, + /// The maximum number of completions to add. If our list length exceeds this, then new + /// completions are not added. Note 0 has no special significance here - use + /// usize::MAX instead. + limit: usize, +} + +// We are only wrapping a `Vec`, any non-mutable methods can be safely deferred to the +// Vec-impl +impl std::ops::Deref for CompletionReceiver { + type Target = Vec; + + fn deref(&self) -> &Self::Target { + &self.completions + } +} + +impl CompletionReceiver { + pub fn new(limit: usize) -> Self { + Self { + limit, + ..Default::default() + } + } + + /// Add a completion. + /// \return true on success, false if this would overflow the limit. + #[must_use] + pub fn add(&mut self, comp: impl Into) -> bool { + if self.completions.len() >= self.limit { + return false; + } + self.completions.push(comp.into()); + return true; + } + + /// Add a list of completions. + /// \return true on success, false if this would overflow the limit. + #[must_use] + pub fn extend( + &mut self, + iter: impl IntoIterator>, + ) -> bool { + let iter = iter.into_iter(); + if iter.len() > self.limit - self.completions.len() { + return false; + } + self.completions.extend(iter); + // this only fails if the ExactSizeIterator impl is bogus + assert!( + self.completions.len() <= self.limit, + "ExactSizeIterator returned more items than it should" + ); + true + } + + /// Swap our completions with a new list. + pub fn swap(&mut self, lst: &mut Vec) { + // XXX: This completly breaks our completions.len() <= self.limit invariant + std::mem::swap(&mut self.completions, lst); + } + + /// Clear the list of completions. This retains the storage inside completions_ which can be + /// useful to prevent allocations. + pub fn clear(&mut self) { + self.completions.clear(); + } + + /// \return the list of completions, clearing it. + pub fn take(&mut self) -> Vec { + std::mem::take(&mut self.completions) + } + + /// \return a new, empty receiver whose limit is our remaining capacity. + /// This is useful for e.g. recursive calls when you want to act on the result before adding it. + pub fn subreceiver(&self) -> Self { + // XXX: this should not need to be saturating, we have a faulty invariant + let remaining_capacity = self.limit.saturating_sub(self.completions.len()); + Self::new(remaining_capacity) + } +} diff --git a/fish-rust/src/lib.rs b/fish-rust/src/lib.rs index a0e9903ea..9f475bba0 100644 --- a/fish-rust/src/lib.rs +++ b/fish-rust/src/lib.rs @@ -23,6 +23,7 @@ mod ast; mod builtins; mod color; mod compat; +mod complete; mod curses; mod env; mod env_dispatch; diff --git a/fish-rust/src/wcstringutil.rs b/fish-rust/src/wcstringutil.rs index 8753a8833..5bd2fa0b3 100644 --- a/fish-rust/src/wcstringutil.rs +++ b/fish-rust/src/wcstringutil.rs @@ -98,10 +98,10 @@ pub enum CaseFold { } /// A lightweight value-type describing how closely a string fuzzy-matches another string. -#[derive(Debug, Eq, PartialEq)] +#[derive(Debug, Eq, PartialEq, Clone)] pub struct StringFuzzyMatch { - typ: ContainType, - case_fold: CaseFold, + pub typ: ContainType, + pub case_fold: CaseFold, } impl StringFuzzyMatch { diff --git a/fish-rust/src/wildcard.rs b/fish-rust/src/wildcard.rs index 1f6608f72..042835ee3 100644 --- a/fish-rust/src/wildcard.rs +++ b/fish-rust/src/wildcard.rs @@ -1,14 +1,27 @@ // Enumeration of all wildcard types. +use std::collections::HashSet; +use std::io::ErrorKind; +use std::os::unix::prelude::*; +use std::{fs, io}; + use cxx::CxxWString; +use libc::{mode_t, ELOOP, S_IXGRP, S_IXOTH, S_IXUSR, X_OK}; use crate::common::{ - char_offset, unescape_string, UnescapeFlags, UnescapeStringStyle, WILDCARD_RESERVED_BASE, + char_offset, format_size, is_windows_subsystem_for_linux, unescape_string, CancelChecker, + UnescapeFlags, UnescapeStringStyle, WILDCARD_RESERVED_BASE, }; +use crate::complete::{CompleteFlags, Completion, CompletionReceiver, PROG_COMPLETE_SEP}; +use crate::expand::ExpandFlags; use crate::future_feature_flags::feature_test; use crate::future_feature_flags::FeatureFlag; use crate::wchar::prelude::*; use crate::wchar_ffi::WCharFromFFI; +use crate::wcstringutil::{ + string_fuzzy_match_string, string_suffixes_string_case_insensitive, CaseFold, +}; +use crate::wutil::{lwstat, waccess, wstat}; /// Character representing any character except '/' (slash). pub const ANY_CHAR: char = char_offset(WILDCARD_RESERVED_BASE, 0); @@ -20,6 +33,1013 @@ pub const ANY_STRING_RECURSIVE: char = char_offset(WILDCARD_RESERVED_BASE, 2); /// end of the the special characters so we can sanity check the enum range. pub const ANY_SENTINEL: char = char_offset(WILDCARD_RESERVED_BASE, 3); +#[derive(PartialEq)] +pub enum WildcardResult { + /// The wildcard did not match. + NoMatch, + /// The wildcard did match. + Match, + /// Expansion was cancelled (e.g. control-C). + Cancel, + /// Expansion produced too many results. + Overflow, +} + +fn resolve_description<'f>( + full_completion: &wstr, + completion: &mut &wstr, + expand_flags: ExpandFlags, + description_func: Option<&'f dyn Fn(&wstr) -> WString>, +) -> WString { + if let Some(complete_sep_loc) = completion.find_char(PROG_COMPLETE_SEP) { + // This completion has an embedded description, do not use the generic description. + assert!( + completion.len() > complete_sep_loc, + "resolve_description lacks a length assumption" + ); + let (comp, description) = completion.split_at(complete_sep_loc + 1); + *completion = comp; + return description.to_owned(); + } + + if let Some(f) = description_func { + if expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { + return f(full_completion); + } + } + + WString::new() +} + +// A transient parameter pack needed by wildcard_complete. +struct WcCompletePack<'orig, 'f> { + pub orig: &'orig wstr, + pub desc_func: Option<&'f dyn Fn(&wstr) -> WString>, + pub expand_flags: ExpandFlags, +} + +// Weirdly specific and non-reusable helper function that makes its one call site much clearer. +fn has_prefix_match(comps: &CompletionReceiver, first: usize) -> bool { + comps[first..] + .iter() + .any(|c| c.r#match.is_exact_or_prefix() && c.r#match.case_fold == CaseFold::samecase) +} + +/// Matches the string against the wildcard, and if the wildcard is a possible completion of the +/// string, the remainder of the string is inserted into the out vector. +/// +/// We ignore ANY_STRING_RECURSIVE here. The consequence is that you cannot tab complete ** +/// wildcards. This is historic behavior. +/// is_first_call is default false. +#[allow(clippy::unnecessary_unwrap)] +fn wildcard_complete_internal( + str: &wstr, + wc: &wstr, + params: &WcCompletePack, + flags: CompleteFlags, + // it is easier to recurse with this over taking it by value + out: &mut Option<&mut CompletionReceiver>, + is_first_call: bool, +) -> WildcardResult { + // Maybe early out for hidden files. We require that the wildcard match these exactly (i.e. a + // dot); ANY_STRING not allowed. + if is_first_call + && !params + .expand_flags + .contains(ExpandFlags::ALLOW_NONLITERAL_LEADING_DOT) + && str.char_at(0) == '.' + && wc.char_at(0) == '.' + { + return WildcardResult::NoMatch; + } + + // Locate the next wildcard character position, e.g. ANY_CHAR or ANY_STRING. + let next_wc_char_pos = wc + .chars() + .position(|c| matches!(c, ANY_CHAR | ANY_STRING | ANY_STRING_RECURSIVE)); + + // Maybe we have no more wildcards at all. This includes the empty string. + if next_wc_char_pos.is_none() { + // Try matching + let Some(m) = string_fuzzy_match_string(wc, str, false) else { + return WildcardResult::NoMatch; + }; + + // If we're not allowing fuzzy match, then we require a prefix match. + let needs_prefix_match = !params.expand_flags.contains(ExpandFlags::FUZZY_MATCH); + if needs_prefix_match && m.is_exact_or_prefix() { + return WildcardResult::NoMatch; + } + + // The match was successful. If the string is not requested we're done. + let Some(out) = out else { + return WildcardResult::Match; + }; + + // Wildcard complete. + let full_replacement = + m.requires_full_replacement() || flags.contains(CompleteFlags::REPLACES_TOKEN); + + // If we are not replacing the token, be careful to only store the part of the string after + // the wildcard. + assert!(!full_replacement || wc.len() <= str.len()); + let mut out_completion = match full_replacement { + true => params.orig, + false => str.slice_from(wc.len()), + }; + let out_desc = resolve_description( + params.orig, + &mut out_completion, + params.expand_flags, + params.desc_func, + ); + + // Note: out_completion may be empty if the completion really is empty, e.g. tab-completing + // 'foo' when a file 'foo' exists. + let local_flags = flags + | full_replacement + .then_some(CompleteFlags::REPLACES_TOKEN) + .unwrap_or_default(); + if !out.add(Completion { + completion: out_completion.to_owned(), + description: out_desc, + flags: local_flags, + r#match: m, + }) { + return WildcardResult::Overflow; + } + return WildcardResult::Match; + } else if next_wc_char_pos.unwrap() > 0 { + let next_wc_char_pos = next_wc_char_pos.unwrap(); + // The literal portion of a wildcard cannot be longer than the string itself, + // e.g. `abc*` can never match a string that is only two characters long. + if next_wc_char_pos >= str.len() { + return WildcardResult::NoMatch; + } + + // Here we have a non-wildcard prefix. Note that we don't do fuzzy matching for stuff before + // a wildcard, so just do case comparison and then recurse. + if str.slice_to(next_wc_char_pos) == wc.slice_to(next_wc_char_pos) { + // Normal match. + return wildcard_complete_internal( + str.slice_from(next_wc_char_pos), + wc.slice_from(next_wc_char_pos), + params, + flags, + out, + false, + ); + } + // TODO: acually be case-insensitive + if str.slice_to(next_wc_char_pos).to_lowercase() + == wc.slice_to(next_wc_char_pos).to_lowercase() + { + // Case insensitive match. + return wildcard_complete_internal( + str.slice_from(next_wc_char_pos), + wc.slice_from(next_wc_char_pos), + params, + flags | CompleteFlags::REPLACES_TOKEN, + out, + false, + ); + } + + return WildcardResult::NoMatch; + } + + // Our first character is a wildcard. + match wc.char_at(0) { + ANY_CHAR => { + if str.is_empty() { + return WildcardResult::NoMatch; + } + return wildcard_complete_internal( + str.slice_from(1), + wc.slice_from(1), + params, + flags, + out, + false, + ); + } + ANY_STRING => { + // Hackish. If this is the last character of the wildcard, then just complete with + // the empty string. This fixes cases like "f*" -> "f*o". + if wc.char_at(1) == '\0' { + return wildcard_complete_internal(L!(""), L!(""), params, flags, out, false); + } + + // Try all submatches. Issue #929: if the recursive call gives us a prefix match, + // just stop. This is sloppy - what we really want to do is say, once we've seen a + // match of a particular type, ignore all matches of that type further down the + // string, such that the wildcard produces the "minimal match.". + let mut has_match = false; + for i in 0..str.len() { + let before_count = out.as_ref().map(|o| o.len()).unwrap_or_default(); + let submatch_res = wildcard_complete_internal( + str.slice_from(i), + wc.slice_from(i), + params, + flags, + out, + false, + ); + + match submatch_res { + WildcardResult::NoMatch => continue, + WildcardResult::Match => { + has_match = true; + // If out is NULL, we don't care about the actual matches. If out is not + // NULL but we have a prefix match, stop there. + if out.is_none() || has_prefix_match(out.as_ref().unwrap(), before_count) { + return WildcardResult::Match; + } + continue; + } + // Note early return + WildcardResult::Cancel | WildcardResult::Overflow => return submatch_res, + } + } + + return match has_match { + true => WildcardResult::Match, + false => WildcardResult::NoMatch, + }; + } + // We don't even try with this one. + ANY_STRING_RECURSIVE => WildcardResult::NoMatch, + _ => unreachable!(), + } +} + +pub fn wildcard_complete<'f>( + str: &wstr, + wc: &wstr, + desc_func: Option<&'f dyn Fn(&wstr) -> WString>, + mut out: Option<&mut CompletionReceiver>, + expand_flags: ExpandFlags, + flags: CompleteFlags, +) -> WildcardResult { + let params = WcCompletePack { + orig: str, + desc_func, + expand_flags, + }; + + return wildcard_complete_internal(str, wc, ¶ms, flags, &mut out, true); +} + +/// Obtain a description string for the file specified by the filename. +/// +/// The returned value is a string constant and should not be free'd. +/// +/// \param filename The file for which to find a description string +/// \param lstat_res The result of calling lstat on the file +/// \param lbuf The struct buf output of calling lstat on the file +/// \param stat_res The result of calling stat on the file +/// \param buf The struct buf output of calling stat on the file +/// \param err The errno value after a failed stat call on the file. +fn file_get_desc( + filename: &wstr, + lstat: Option, + stat: Option>, +) -> &'static wstr { + if lstat.is_none() { + return wgettext!("file"); + } + + let stat = stat.unwrap(); + let lstat = lstat.unwrap(); + if lstat.is_symlink() { + return match stat { + Ok(stat) if stat.is_dir() => wgettext!("dir symlink"), + Ok(stat) + if (stat.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0 + && waccess(filename, X_OK) == 0 => + { + // Weird group permissions and other such issues make it non-trivial to find out if + // we can actually execute a file using the result from stat. It is much safer to + // use the access function, since it tells us exactly what we want to know. + wgettext!("command link") + } + Ok(_) => wgettext!("symlink"), + Err(e) if e.kind() == ErrorKind::NotFound => wgettext!("broken symlink"), + Err(e) if e.raw_os_error().unwrap() == ELOOP => wgettext!("symlink loop"), + _ => { + // On unknown errors we do nothing. The file will be given the default 'File' + // description or one based on the suffix. + wgettext!("file") + } + }; + } + + let Ok(stat) = stat else { + // Assuming that the metadata was zero if stat-call failed + return wgettext!("file"); + }; + + if stat.file_type().is_char_device() { + wgettext!("char device") + } else if stat.file_type().is_block_device() { + wgettext!("block device") + } else if stat.file_type().is_fifo() { + wgettext!("fifo") + } else if stat.file_type().is_socket() { + wgettext!("socket") + } else if stat.is_dir() { + wgettext!("directory") + } else if (stat.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0 + && waccess(filename, X_OK) == 0 + { + // Weird group permissions and other such issues make it non-trivial to find out if we can + // actually execute a file using the result from stat. It is much safer to use the access + // function, since it tells us exactly what we want to know. + wgettext!("command") + } else { + wgettext!("file") + } +} + +/// Test if the given file is an executable (if executables_only) or directory (if +/// directories_only). If it matches, call wildcard_complete() with some description that we make +/// up. Note that the filename came from a readdir() call, so we know it exists. +fn wildcard_test_flags_then_complete( + filepath: &wstr, + filename: &wstr, + wc: &wstr, + expand_flags: ExpandFlags, + out: Option<&mut CompletionReceiver>, + known_dir: bool, +) -> bool { + let executables_only = expand_flags.contains(ExpandFlags::EXECUTABLES_ONLY); + let need_directory = expand_flags.contains(ExpandFlags::DIRECTORIES_ONLY); + // Fast path: If we need directories, and we already know it is one, + // and we don't need to do anything else, just return it. + // This is a common case for cd completions, and removes the `stat` entirely in case the system + // supports it. + if known_dir && !executables_only && !expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { + return wildcard_complete( + &(WString::from(filename) + L!("/")), + wc, + Some(&|_| L!("").to_owned()), + out, + expand_flags, + CompleteFlags::NO_SPACE, + ) == WildcardResult::Match; + } + // Check if it will match before stat(). + if wildcard_complete( + filename, + wc, + None, + None, + expand_flags, + CompleteFlags::default(), + ) != WildcardResult::Match + { + return false; + } + + let lstat: Option = lwstat(filepath).ok(); + let stat: Option>; + if let Some(md) = &lstat { + if md.is_symlink() { + // In order to differentiate between e.g. broken symlinks and symlink loops, we also + // need to know the error status of wstat. + stat = Some(wstat(filepath)); + } else { + stat = Some(Ok(md.clone())); + } + } else { + stat = None; + } + + let (file_size, is_directory, is_executable) = stat + .as_ref() + .and_then(|s| s.as_ref().map(|s| (s.len(), s.is_dir(), s.is_file())).ok()) + .unwrap_or_default(); + + if need_directory && !is_directory { + return false; + } + + if executables_only && (!is_executable || waccess(filepath, X_OK) != 0) { + return false; + } + + if executables_only + && is_windows_subsystem_for_linux() + && string_suffixes_string_case_insensitive(L!(".dll"), filename) + { + return false; + } + + // Compute the description. + let mut desc = WString::new(); + if expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { + desc = file_get_desc(filename, lstat, stat).to_owned(); + + if !is_directory && !is_executable { + if !desc.is_empty() { + desc.push_utfstr(L!(", ")); + } + desc.push_utfstr(&format_size(file_size as i64)); + } + } + + // Append a / if this is a directory. Note this requirement may be the only reason we have to + // call stat() in some cases. + let x = |_: &wstr| desc.clone(); + let desc_func: Option<&dyn Fn(&wstr) -> WString> = Some(&x); + if is_directory { + return wildcard_complete( + &(filename.to_owned() + L!("/")), + wc, + desc_func, + out, + expand_flags, + CompleteFlags::NO_SPACE, + ) == WildcardResult::Match; + } + + wildcard_complete( + filename, + wc, + desc_func, + out, + expand_flags, + CompleteFlags::empty(), + ) == WildcardResult::Match +} + +use expander::WildCardExpander; +mod expander { + use libc::F_OK; + + use crate::{ + common::scoped_push, + complete::CompleteFlags, + path::append_path_component, + wcstringutil::string_fuzzy_match_string, + wutil::{dir_iter::DirIter, normalize_path, waccess, FileId}, + }; + + use super::*; + + pub struct WildCardExpander<'receiver, 'c> { + /// A function to call to check cancellation. + cancel_checker: &'c CancelChecker, + /// The working directory to resolve paths against + working_directory: WString, + /// The set of items we have resolved, used to efficiently avoid duplication. + completion_set: HashSet, + /// The set of file IDs we have visited, used to avoid symlink loops. + visited_files: HashSet, + /// Flags controlling expansion. + flags: ExpandFlags, + /// Resolved items get inserted into here. This is transient of course. + resolved_completions: &'receiver mut CompletionReceiver, + /// Whether we have been interrupted. + did_interrupt: bool, + /// Whether we have overflowed. + did_overflow: bool, + /// Whether we have successfully added any completions. + did_add: bool, + /// Whether some parent expansion is fuzzy, and therefore completions always prepend their prefix + /// This variable is a little suspicious - it should be passed along, not stored here + /// If we ever try to do parallel wildcard expansion we'll have to remove this + has_fuzzy_ancestor: bool, + } + + impl<'receiver, 'c> WildCardExpander<'receiver, 'c> { + pub fn new( + working_directory: WString, + flags: ExpandFlags, + cancel_checker: &'c CancelChecker, + resolved_completions: &'receiver mut CompletionReceiver, + ) -> Self { + Self { + cancel_checker, + working_directory, + completion_set: resolved_completions + .iter() + .map(|c| c.completion.to_owned()) + .collect(), + visited_files: HashSet::new(), + flags, + resolved_completions, + did_add: false, + did_interrupt: false, + did_overflow: false, + has_fuzzy_ancestor: false, + } + } + + /// The real implementation of wildcard expansion is in this function. Other functions are just + /// wrappers around this one. + /// + /// This function traverses the relevant directory tree looking for matches, and recurses when + /// needed to handle wildcards spanning multiple components and recursive wildcards. + /// + /// Args: + /// base_dir: the "working directory" against which the wildcard is to be resolved + /// wc: the wildcard string itself, e.g. foo*bar/baz (where * is actually ANY_CHAR) + /// effective_prefix: the string that should be prepended for completions that replace their token. + /// This is usually the same thing as the original wildcard, but for fuzzy matching, we + /// expand intermediate segments. effective_prefix is always either empty, or ends with a slash + pub fn expand(&mut self, base_dir: &wstr, wc: &wstr, effective_prefix: &wstr) { + if self.interrupted_or_overflowed() { + return; + } + + // Get the current segment and compute interesting properties about it. + let next_slash = wc.find_char('/'); + let is_last_segment = next_slash.is_none(); + let wc_segment_len = next_slash.unwrap_or(wc.char_count()); + let wc_segment = wc.slice_to(wc_segment_len); + let segment_has_wildcards = wildcard_has_internal(wc_segment); + let wc_remainder = next_slash.map(|n| wc.slice_from(n)).unwrap_or(L!("")); + + if wc_segment.is_empty() { + assert!(!segment_has_wildcards); + if is_last_segment { + self.expand_trailing_slash(base_dir, effective_prefix); + } else { + let mut prefix = effective_prefix.to_owned(); + prefix.push('/'); + self.expand(base_dir, wc_remainder, &prefix); + } + } else if !segment_has_wildcards && !is_last_segment { + // Literal intermediate match. Note that we may not be able to actually read the directory + // (issue #2099). + assert!(next_slash.is_some()); + + // Absolute path of the intermediate directory + let mut intermediate_dirpath = base_dir.to_owned() + wc_segment; + intermediate_dirpath.push('/'); + + // This just trumps everything + let before = self.resolved_completions.len(); + let mut prefix = effective_prefix.to_owned() + wc_segment; + prefix.push('/'); + self.expand(&intermediate_dirpath, wc_remainder, &prefix); + + // Maybe try a fuzzy match (#94) if nothing was found with the literal match. Respect + // EXPAND_NO_DIRECTORY_ABBREVIATIONS (issue #2413). + // Don't do fuzzy matches if the literal segment was valid (#3211) + let allow_fuzzy = self.flags.contains(ExpandFlags::FUZZY_MATCH) + && !self.flags.contains(ExpandFlags::NO_FUZZY_DIRECTORIES); + if allow_fuzzy + && self.resolved_completions.len() == before + && waccess(&intermediate_dirpath, F_OK) != 0 + { + assert!(self.flags.contains(ExpandFlags::FOR_COMPLETIONS)); + if let Ok(mut base_dir_iter) = self.open_dir(base_dir, false) { + self.expand_literal_intermediate_segment_with_fuzz( + base_dir, + &mut base_dir_iter, + wc_segment, + wc_remainder, + effective_prefix, + ); + } + } + } else { + assert!(!wc_segment.is_empty() && (segment_has_wildcards || is_last_segment)); + + if !is_last_segment && matches!(wc_segment.as_char_slice(), [ANY_STRING_RECURSIVE]) + { + // Hack for #7222. This is an intermediate wc segment that is exactly **. The + // tail matches in subdirectories as normal, but also the current directory. + // That is, '**/bar' may match 'bar' and 'foo/bar'. + // Implement this by matching the wildcard tail only, in this directory. + // Note if the segment is not exactly ANY_STRING_RECURSIVE then the segment may only + // match subdirectories. + self.expand(base_dir, wc_remainder, effective_prefix); + if self.interrupted_or_overflowed() { + return; + } + } + + // return "." and ".." entries if we're doing completions + let Ok(mut dir) = self.open_dir(base_dir, /* return . and .. */ self.flags.contains(ExpandFlags::FOR_COMPLETIONS)) else { + return; + }; + + if is_last_segment { + // Last wildcard segment, nonempty wildcard. + self.expand_last_segment(base_dir, &mut dir, wc_segment, effective_prefix); + } else { + // Not the last segment, nonempty wildcard. + assert!(next_slash.is_some()); + let mut prefix = effective_prefix.to_owned() + wc_segment; + prefix.push('/'); + self.expand_intermediate_segment( + base_dir, + &mut dir, + wc_segment, + wc_remainder, + &prefix, + ); + } + + let Some(asr_idx) = wc_segment.find_char(ANY_STRING_RECURSIVE) else { + return; + }; + + // Apply the recursive **. + // Construct a "head + any" wildcard for matching stuff in this directory, and an + // "any + tail" wildcard for matching stuff in subdirectories. Note that the + // ANY_STRING_RECURSIVE character is present in both the head and the tail. + let head_any = wc_segment.slice_to(asr_idx + 1); + let any_tail = wc.slice_from(asr_idx); + assert!(head_any.chars().last().unwrap() == ANY_STRING_RECURSIVE); + assert!(any_tail.char_at(0) == ANY_STRING_RECURSIVE); + + dir.rewind(); + self.expand_intermediate_segment( + base_dir, + &mut dir, + head_any, + any_tail, + effective_prefix, + ); + } + } + + pub fn status_code(&self) -> WildcardResult { + if self.did_interrupt { + return WildcardResult::Cancel; + } else if self.did_overflow { + return WildcardResult::Overflow; + } + match self.did_add { + true => WildcardResult::Match, + false => WildcardResult::NoMatch, + } + } + } + + impl<'receiver, 'c> WildCardExpander<'receiver, 'c> { + /// We are a trailing slash - expand at the end. + fn expand_trailing_slash(&mut self, base_dir: &wstr, prefix: &wstr) { + if self.interrupted_or_overflowed() { + return; + } + + if !self.flags.contains(ExpandFlags::FOR_COMPLETIONS) { + // Trailing slash and not accepting incomplete, e.g. `echo /xyz/`. Insert this file, we already know it exists! + self.add_expansion_result(base_dir.to_owned()); + return; + } + // Trailing slashes and accepting incomplete, e.g. `echo /xyz/`. Everything is added. + let Ok(mut dir) = self.open_dir(base_dir, false) else { + return; + }; + + // wreaddir_resolving without the out argument is just wreaddir. + // So we can use the information in case we need it. + let need_dir = self.flags.contains(ExpandFlags::DIRECTORIES_ONLY); + + while let Some(Ok(entry)) = dir.next() { + if self.interrupted_or_overflowed() { + break; + } + + // Note that is_dir() may cause a stat() call. + let known_dir = need_dir && entry.is_dir(); + if need_dir && !known_dir { + continue; + }; + if !entry.name.is_empty() && !entry.name.starts_with('.') { + self.try_add_completion_result( + &(WString::from(base_dir) + entry.name.as_utfstr()), + &entry.name, + L!(""), + prefix, + known_dir, + ); + } + } + } + + /// Given a directory base_dir, which is opened as base_dir_iter, expand an intermediate segment + /// of the wildcard. Treat ANY_STRING_RECURSIVE as ANY_STRING. wc_segment is the wildcard + /// segment for this directory, wc_remainder is the wildcard for subdirectories, + /// prefix is the prefix for completions. + fn expand_intermediate_segment( + &mut self, + base_dir: &wstr, + base_dir_iter: &mut DirIter, + wc_segment: &wstr, + wc_remainder: &wstr, + prefix: &wstr, + ) { + while !self.interrupted_or_overflowed() { + let Some(Ok(entry)) = base_dir_iter.next() else { + break; + }; + // Note that it's critical we ignore leading dots here, else we may descend into . and .. + if !wildcard_match(&entry.name, wc_segment, true) { + // Doesn't match the wildcard for this segment, skip it. + continue; + } + if !entry.is_dir() { + continue; + } + + let Some(statbuf) = entry.stat() else { + continue; + }; + + let file_id = FileId::from_stat(&statbuf); + if !self.visited_files.insert(file_id.clone()) { + // Symlink loop! This directory was already visited, so skip it. + continue; + } + + let mut full_path: WString = base_dir.to_owned() + entry.name.as_utfstr(); + full_path.push('/'); + + let mut prefix: WString = prefix.to_owned() + wc_segment; + prefix.push('/'); + self.expand(&full_path, wc_remainder, &prefix); + + // Now remove the visited file. This is for #2414: only directories "beneath" us should be + // considered visited. + self.visited_files.remove(&file_id); + } + } + + /// Given a directory base_dir, which is opened as base_dir_fp, expand an intermediate literal + /// segment. Use a fuzzy matching algorithm. + fn expand_literal_intermediate_segment_with_fuzz( + &mut self, + base_dir: &wstr, + base_dir_iter: &mut DirIter, + wc_segment: &wstr, + wc_remainder: &wstr, + prefix: &wstr, + ) { + // Mark that we are fuzzy for the duration of this function + let mut this = scoped_push(self, |e| &mut e.has_fuzzy_ancestor, true); + while !this.interrupted_or_overflowed() { + let Some(Ok(entry)) = base_dir_iter.next() else { + break; + }; + + // Don't bother with . and .. + if entry.name == "." || entry.name == ".." { + continue; + } + + let Some(m) = string_fuzzy_match_string(wc_segment, &entry.name, false) else { + continue; + }; + if m.is_samecase_exact() { + continue; + } + + // Note is_dir() may trigger a stat call. + if !entry.is_dir() { + continue; + } + + // Determine the effective prefix for our children. + // Normally this would be the wildcard segment, but here we know our segment doesn't have + // wildcards ("literal") and we are doing fuzzy expansion, which means we replace the + // segment with files found through fuzzy matching. + let mut child_prefix = prefix.to_owned() + entry.name.as_utfstr(); + child_prefix.push('/'); + let child_prefix = child_prefix; + + let mut new_full_path = base_dir.to_owned() + entry.name.as_utfstr(); + new_full_path.push('/'); + + // Ok, this directory matches. Recurse to it. Then mark each resulting completion as fuzzy. + let before = this.resolved_completions.len(); + this.expand(&new_full_path, wc_remainder, &child_prefix); + let after = this.resolved_completions.len(); + + assert!(before <= after); + for i in before..after { + // Mark the completion as replacing. + let c = this.resolved_completions.get_mut(i).unwrap(); + + if !c.replaces_token() { + c.flags |= CompleteFlags::REPLACES_TOKEN; + c.prepend_token_prefix(&child_prefix); + } + + // And every match must be made at least as fuzzy as ours. + // TODO: justify this, tests do not exercise it yet. + if m.rank() > c.r#match.rank() { + // Our match is fuzzier. + c.r#match = m.clone(); + } + } + } + } + + /// Given a directory base_dir, which is opened as base_dir_iter, expand the last segment of the + /// wildcard. Treat ANY_STRING_RECURSIVE as ANY_STRING. wc is the wildcard segment to use for + /// matching, wc_remainder is the wildcard for subdirectories, prefix is the prefix for + /// completions. + fn expand_last_segment( + &mut self, + base_dir: &wstr, + base_dir_iter: &mut DirIter, + wc: &wstr, + prefix: &wstr, + ) { + let is_dir = false; + let need_dir = self.flags.contains(ExpandFlags::DIRECTORIES_ONLY); + + while !self.interrupted_or_overflowed() { + let Some(Ok(entry)) = base_dir_iter.next() else { + break; + }; + + if need_dir && entry.is_dir() { + continue; + } + + if self.flags.contains(ExpandFlags::FOR_COMPLETIONS) { + self.try_add_completion_result( + &(base_dir.to_owned() + entry.name.as_utfstr()), + &entry.name, + wc, + prefix, + is_dir, + ); + } else { + // Normal wildcard expansion, not for completions. + if wildcard_match( + &entry.name, + wc, + true, /* skip files with leading dots */ + ) { + self.add_expansion_result(base_dir.to_owned() + entry.name.as_utfstr()); + } + } + } + } + + /// Indicate whether we should cancel wildcard expansion. This latches 'interrupt'. + fn interrupted_or_overflowed(&mut self) -> bool { + self.did_interrupt |= (self.cancel_checker)(); + self.did_interrupt || self.did_overflow + } + + fn add_expansion_result(&mut self, result: WString) { + // This function is only for the non-completions case. + assert!(!self.flags.contains(ExpandFlags::FOR_COMPLETIONS)); + if self.completion_set.insert(result.clone()) && !self.resolved_completions.add(result) + { + self.did_overflow = true; + } + } + + // Given a start point as an absolute path, for any directory that has exactly one non-hidden + // entity in it which is itself a directory, return that. The result is a relative path. For + // example, if start_point is '/usr' we may return 'local/bin/'. + // + // The result does not have a leading slash, but does have a trailing slash if non-empty. + fn descend_unique_hierarchy(&mut self, start_point: &mut WString) -> WString { + assert!(!start_point.is_empty() && !start_point.starts_with('/')); + + let mut unique_hierarchy = WString::from(""); + let abs_unique_hierarchy = start_point; + + // Ensure we don't fall into a symlink loop. + // Ideally we would compare both devices and inodes, but devices require a stat call, so we + // use inodes exclusively. + let mut visited_inodes: HashSet = HashSet::new(); + + loop { + let mut unique_entry = WString::new(); + let Ok(mut dir) = DirIter::new(abs_unique_hierarchy) else { + break; + }; + + while let Some(Ok(entry)) = dir.next() { + if entry.name.is_empty() || entry.name.starts_with('.') { + // either hidden, or . and .. entries -- skip them + continue; + } + if !visited_inodes.insert(entry.inode) { + // Either we've visited this inode already or there's multiple files; + // either way stop. + break; + } else if entry.is_dir() && unique_entry.is_empty() { + // first candidate + unique_entry = entry.name.to_owned(); + } else { + // We either have two or more candidates, or the child is not a directory. We're + // done. + unique_entry.clear(); + break; + } + } + + // We stop if we got two or more entries; also stop if we got zero or were interrupted + if unique_entry.is_empty() || self.interrupted_or_overflowed() { + break; + } + + append_path_component(&mut unique_hierarchy, &unique_entry); + unique_hierarchy.push('/'); + + append_path_component(abs_unique_hierarchy, &unique_entry); + abs_unique_hierarchy.push('/'); + } + + return unique_hierarchy; + } + + fn try_add_completion_result( + &mut self, + filepath: &wstr, + filename: &wstr, + wildcard: &wstr, + prefix: &wstr, + known_dir: bool, + ) { + // This function is only for the completions case. + assert!(self.flags.contains(ExpandFlags::FOR_COMPLETIONS)); + let mut abs_path = self.working_directory.clone(); + append_path_component(&mut abs_path, filepath); + + // We must normalize the path to allow 'cd ..' to operate on logical paths. + if self.flags.contains(ExpandFlags::SPECIAL_FOR_CD) { + abs_path = normalize_path(&abs_path, true); + } + + let before = self.resolved_completions.len(); + + if wildcard_test_flags_then_complete( + &abs_path, + filename, + wildcard, + self.flags, + Some(self.resolved_completions), + known_dir, + ) { + // Hack. We added this completion result based on the last component of the wildcard. + // Prepend our prefix to each wildcard that replaces its token. + // Note that prepend_token_prefix is a no-op unless COMPLETE_REPLACES_TOKEN is set + let after = self.resolved_completions.len(); + for i in before..after { + let c = self.resolved_completions.get_mut(i).unwrap(); + if self.has_fuzzy_ancestor && !(c.flags.contains(CompleteFlags::REPLACES_TOKEN)) + { + c.flags |= CompleteFlags::REPLACES_TOKEN; + c.prepend_token_prefix(wildcard); + } + c.prepend_token_prefix(prefix); + } + + // Implement special_for_cd_autosuggestion by descending the deepest unique + // hierarchy we can, and then appending any components to each new result. + // Only descend deepest unique for cd autosuggest and not for cd tab completion + // (issue #4402). + if self + .flags + .contains(ExpandFlags::SPECIAL_FOR_CD_AUTOSUGGESTION) + { + let unique_hierarchy = self.descend_unique_hierarchy(&mut abs_path); + if !unique_hierarchy.is_empty() { + for i in before..after { + let c = self.resolved_completions.get_mut(i).unwrap(); + c.completion.push_utfstr(&unique_hierarchy); + } + } + } + + self.did_add = true; + } + } + + // Helper to resolve using our prefix. + /// dotdot default is false + fn open_dir(&self, base_dir: &wstr, dotdot: bool) -> std::io::Result { + let mut path = self.working_directory.clone(); + append_path_component(&mut path, base_dir); + if self.flags.contains(ExpandFlags::SPECIAL_FOR_CD) { + // cd operates on logical paths. + // for example, cd ../ should complete "without resolving symlinks". + path = normalize_path(&path, true); + } + + return match dotdot { + true => DirIter::new_with_dots(&path), + false => DirIter::new(&path), + }; + } + } +} + /// Expand the wildcard by matching against the filesystem. /// /// wildcard_expand works by dividing the wildcard into segments at each directory boundary. Each @@ -37,20 +1057,66 @@ pub const ANY_SENTINEL: char = char_offset(WILDCARD_RESERVED_BASE, 3); /// executables_only /// \param output The list in which to put the output /// -enum WildcardResult { - /// The wildcard did not match. - NoMatch, - /// The wildcard did match. - Match, - /// Expansion was cancelled (e.g. control-C). - Cancel, - /// Expansion produced too many results. - Overflow, -} +pub fn wildcard_expand_string( + wc: &wstr, + working_directory: &wstr, + flags: ExpandFlags, + cancel_checker: &CancelChecker, + output: &mut CompletionReceiver, +) -> WildcardResult { + // Fuzzy matching only if we're doing completions. + assert!( + flags.contains(ExpandFlags::FOR_COMPLETIONS) || !flags.contains(ExpandFlags::FUZZY_MATCH) + ); -// pub fn wildcard_expand_string(wc: &wstr, working_directory: &wstr, flags: ExpandFlags, cancel_checker: impl CancelChecker, output: *mut completion_receiver_t) -> WildcardResult { -// todo!() -// } + // ExpandFlags::SPECIAL_FOR_CD requires expand_flag::DIRECTORIES_ONLY and + // ExpandFlags::FOR_COMPLETIONS and !expand_flag::GEN_DESCRIPTIONS. + assert!( + !(flags.contains(ExpandFlags::SPECIAL_FOR_CD)) + || ((flags.contains(ExpandFlags::DIRECTORIES_ONLY)) + && (flags.contains(ExpandFlags::FOR_COMPLETIONS)) + && (!flags.contains(ExpandFlags::GEN_DESCRIPTIONS))) + ); + + // Hackish fix for issue #1631. We are about to call c_str(), which will produce a string + // truncated at any embedded nulls. We could fix this by passing around the size, etc. However + // embedded nulls are never allowed in a filename, so we just check for them and return 0 (no + // matches) if there is an embedded null. + if wc.contains('\0') { + return WildcardResult::NoMatch; + } + + // We do not support tab-completing recursive (**) wildcards. This is historic behavior. + // Do not descend any directories if there is a ** wildcard. + if flags.contains(ExpandFlags::FOR_COMPLETIONS) && wc.contains(ANY_STRING_RECURSIVE) { + return WildcardResult::NoMatch; + } + + // Compute the prefix and base dir. The prefix is what we prepend for filesystem operations + // (i.e. the working directory), the base_dir is the part of the wildcard consumed thus far, + // which we also have to append. The difference is that the base_dir is returned as part of the + // expansion, and the prefix is not. + // + // Check for a leading slash. If we find one, we have an absolute path: the prefix is empty, the + // base dir is /, and the wildcard is the remainder. If we don't find one, the prefix is the + // working directory, the base dir is empty. + let prefix; + let base_dir; + let effective_wc; + if wc.starts_with(L!("/")) { + prefix = L!(""); + base_dir = L!("/"); + effective_wc = wc.slice_from(1); + } else { + prefix = working_directory; + base_dir = L!(""); + effective_wc = wc; + } + + let mut expander = WildCardExpander::new(prefix.to_owned(), flags, cancel_checker, output); + expander.expand(base_dir, effective_wc, base_dir); + return expander.status_code(); +} /// Test whether the given wildcard matches the string. Does not perform any I/O. /// @@ -161,11 +1227,6 @@ fn wildcard_has(s: impl AsRef) -> bool { return wildcard_has_internal(unescaped); } -/// Test wildcard completion. -// pub fn wildcard_complete(str: &wstr, wc: &wstr, desc_func: impl Fn(&wstr) -> WString, out: *mut completion_receiver_t, expand_flags: ExpandFlags, flags: CompleteFlags) -> bool { -// todo!() -// } - #[cfg(test)] mod tests { use super::*; @@ -199,6 +1260,7 @@ mod ffi { extern "C++" { include!("wutil.h"); } + extern "Rust" { #[cxx_name = "wildcard_match_ffi"] fn wildcard_match_ffi( From d277a50564431049825f4c86b6bdfa705407ea3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20H=C3=B8rl=C3=BCck=20Berg?= <36937807+henrikhorluck@users.noreply.github.com> Date: Sun, 6 Aug 2023 23:41:33 +0200 Subject: [PATCH 3/7] Combine previous attempt into this *singing it's the best of both worlds* --- fish-rust/src/complete.rs | 10 +- fish-rust/src/wildcard.rs | 332 +++++++++++++++++++------------------- 2 files changed, 177 insertions(+), 165 deletions(-) diff --git a/fish-rust/src/complete.rs b/fish-rust/src/complete.rs index 3d5a1f486..789450602 100644 --- a/fish-rust/src/complete.rs +++ b/fish-rust/src/complete.rs @@ -108,10 +108,16 @@ pub struct CompletionReceiver { // We are only wrapping a `Vec`, any non-mutable methods can be safely deferred to the // Vec-impl impl std::ops::Deref for CompletionReceiver { - type Target = Vec; + type Target = [Completion]; fn deref(&self) -> &Self::Target { - &self.completions + self.completions.as_slice() + } +} + +impl std::ops::DerefMut for CompletionReceiver { + fn deref_mut(&mut self) -> &mut Self::Target { + self.completions.as_mut_slice() } } diff --git a/fish-rust/src/wildcard.rs b/fish-rust/src/wildcard.rs index 042835ee3..fb1c33a53 100644 --- a/fish-rust/src/wildcard.rs +++ b/fish-rust/src/wildcard.rs @@ -1,5 +1,6 @@ // Enumeration of all wildcard types. +use std::cmp::Ordering; use std::collections::HashSet; use std::io::ErrorKind; use std::os::unix::prelude::*; @@ -14,6 +15,7 @@ use crate::common::{ }; use crate::complete::{CompleteFlags, Completion, CompletionReceiver, PROG_COMPLETE_SEP}; use crate::expand::ExpandFlags; +use crate::fallback::wcscasecmp; use crate::future_feature_flags::feature_test; use crate::future_feature_flags::FeatureFlag; use crate::wchar::prelude::*; @@ -22,6 +24,20 @@ use crate::wcstringutil::{ string_fuzzy_match_string, string_suffixes_string_case_insensitive, CaseFold, }; use crate::wutil::{lwstat, waccess, wstat}; +use once_cell::sync::Lazy; + +static COMPLETE_EXEC_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("command")); +static COMPLETE_EXEC_LINK_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("command link")); +static COMPLETE_CHAR_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("char device")); +static COMPLETE_BLOCK_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("block device")); +static COMPLETE_FIFO_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("fifo")); +static COMPLETE_FILE_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("file")); +static COMPLETE_SYMLINK_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("symlink")); +static COMPLETE_DIRECTORY_SYMLINK_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("dir symlink")); +static COMPLETE_BROKEN_SYMLINK_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("broken symlink")); +static COMPLETE_LOOP_SYMLINK_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("symlink loop")); +static COMPLETE_SOCKET_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("socket")); +static COMPLETE_DIRECTORY_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("directory")); /// Character representing any character except '/' (slash). pub const ANY_CHAR: char = char_offset(WILDCARD_RESERVED_BASE, 0); @@ -93,12 +109,12 @@ fn has_prefix_match(comps: &CompletionReceiver, first: usize) -> bool { /// is_first_call is default false. #[allow(clippy::unnecessary_unwrap)] fn wildcard_complete_internal( - str: &wstr, + s: &wstr, wc: &wstr, params: &WcCompletePack, flags: CompleteFlags, // it is easier to recurse with this over taking it by value - out: &mut Option<&mut CompletionReceiver>, + mut out: Option<&mut CompletionReceiver>, is_first_call: bool, ) -> WildcardResult { // Maybe early out for hidden files. We require that the wildcard match these exactly (i.e. a @@ -107,8 +123,8 @@ fn wildcard_complete_internal( && !params .expand_flags .contains(ExpandFlags::ALLOW_NONLITERAL_LEADING_DOT) - && str.char_at(0) == '.' - && wc.char_at(0) == '.' + && s.char_at(0) == '.' + && wc.char_at(0) != '.' { return WildcardResult::NoMatch; } @@ -121,7 +137,7 @@ fn wildcard_complete_internal( // Maybe we have no more wildcards at all. This includes the empty string. if next_wc_char_pos.is_none() { // Try matching - let Some(m) = string_fuzzy_match_string(wc, str, false) else { + let Some(m) = string_fuzzy_match_string(wc, s, false) else { return WildcardResult::NoMatch; }; @@ -142,10 +158,10 @@ fn wildcard_complete_internal( // If we are not replacing the token, be careful to only store the part of the string after // the wildcard. - assert!(!full_replacement || wc.len() <= str.len()); + assert!(!full_replacement || wc.len() <= s.len()); let mut out_completion = match full_replacement { true => params.orig, - false => str.slice_from(wc.len()), + false => s.slice_from(wc.len()), }; let out_desc = resolve_description( params.orig, @@ -156,10 +172,11 @@ fn wildcard_complete_internal( // Note: out_completion may be empty if the completion really is empty, e.g. tab-completing // 'foo' when a file 'foo' exists. - let local_flags = flags - | full_replacement - .then_some(CompleteFlags::REPLACES_TOKEN) - .unwrap_or_default(); + let local_flags = if full_replacement { + flags | CompleteFlags::REPLACES_TOKEN + } else { + flags + }; if !out.add(Completion { completion: out_completion.to_owned(), description: out_desc, @@ -169,34 +186,27 @@ fn wildcard_complete_internal( return WildcardResult::Overflow; } return WildcardResult::Match; - } else if next_wc_char_pos.unwrap() > 0 { - let next_wc_char_pos = next_wc_char_pos.unwrap(); + } else if let Some(next_wc_char_pos @ 1..) = next_wc_char_pos { // The literal portion of a wildcard cannot be longer than the string itself, // e.g. `abc*` can never match a string that is only two characters long. - if next_wc_char_pos >= str.len() { + if next_wc_char_pos >= s.len() { return WildcardResult::NoMatch; } + let (s_pre, s_suf) = s.split_at(next_wc_char_pos); + let (wc_pre, wc_suf) = wc.split_at(next_wc_char_pos); + // Here we have a non-wildcard prefix. Note that we don't do fuzzy matching for stuff before // a wildcard, so just do case comparison and then recurse. - if str.slice_to(next_wc_char_pos) == wc.slice_to(next_wc_char_pos) { + if s_pre == wc_pre { // Normal match. - return wildcard_complete_internal( - str.slice_from(next_wc_char_pos), - wc.slice_from(next_wc_char_pos), - params, - flags, - out, - false, - ); + return wildcard_complete_internal(s_suf, wc_suf, params, flags, out, false); } - // TODO: acually be case-insensitive - if str.slice_to(next_wc_char_pos).to_lowercase() - == wc.slice_to(next_wc_char_pos).to_lowercase() - { + + if wcscasecmp(s_pre, wc_pre) == Ordering::Equal { // Case insensitive match. return wildcard_complete_internal( - str.slice_from(next_wc_char_pos), + s.slice_from(next_wc_char_pos), wc.slice_from(next_wc_char_pos), params, flags | CompleteFlags::REPLACES_TOKEN, @@ -209,13 +219,14 @@ fn wildcard_complete_internal( } // Our first character is a wildcard. + assert_eq!(next_wc_char_pos, Some(0)); match wc.char_at(0) { ANY_CHAR => { - if str.is_empty() { + if s.is_empty() { return WildcardResult::NoMatch; } return wildcard_complete_internal( - str.slice_from(1), + s.slice_from(1), wc.slice_from(1), params, flags, @@ -226,7 +237,7 @@ fn wildcard_complete_internal( ANY_STRING => { // Hackish. If this is the last character of the wildcard, then just complete with // the empty string. This fixes cases like "f*" -> "f*o". - if wc.char_at(1) == '\0' { + if wc.len() == 1 { return wildcard_complete_internal(L!(""), L!(""), params, flags, out, false); } @@ -235,14 +246,14 @@ fn wildcard_complete_internal( // match of a particular type, ignore all matches of that type further down the // string, such that the wildcard produces the "minimal match.". let mut has_match = false; - for i in 0..str.len() { + for i in 0..s.len() { let before_count = out.as_ref().map(|o| o.len()).unwrap_or_default(); let submatch_res = wildcard_complete_internal( - str.slice_from(i), - wc.slice_from(i), + s.slice_from(i), + wc.slice_from(1), params, flags, - out, + out.as_deref_mut(), false, ); @@ -252,7 +263,11 @@ fn wildcard_complete_internal( has_match = true; // If out is NULL, we don't care about the actual matches. If out is not // NULL but we have a prefix match, stop there. - if out.is_none() || has_prefix_match(out.as_ref().unwrap(), before_count) { + let Some(out) = out.as_mut() else { + return WildcardResult::Match; + }; + + if has_prefix_match(out, before_count) { return WildcardResult::Match; } continue; @@ -274,20 +289,20 @@ fn wildcard_complete_internal( } pub fn wildcard_complete<'f>( - str: &wstr, + s: &wstr, wc: &wstr, desc_func: Option<&'f dyn Fn(&wstr) -> WString>, - mut out: Option<&mut CompletionReceiver>, + out: Option<&mut CompletionReceiver>, expand_flags: ExpandFlags, flags: CompleteFlags, ) -> WildcardResult { let params = WcCompletePack { - orig: str, + orig: s, desc_func, expand_flags, }; - return wildcard_complete_internal(str, wc, ¶ms, flags, &mut out, true); + return wildcard_complete_internal(s, wc, ¶ms, flags, out, true); } /// Obtain a description string for the file specified by the filename. @@ -305,59 +320,54 @@ fn file_get_desc( lstat: Option, stat: Option>, ) -> &'static wstr { - if lstat.is_none() { - return wgettext!("file"); + let Some(lstat) = lstat else { + return *COMPLETE_FILE_DESC; + }; + + fn is_executable(buf: &fs::Metadata, filename: &wstr) -> bool { + // Weird group permissions and other such issues make it non-trivial to find out if + // we can actually execute a file using the result from stat. It is much safer to + // use the access function, since it tells us exactly what we want to know. + (buf.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH) != 0) && waccess(filename, X_OK) == 0 } + // stat was only queried if lstat succeeded let stat = stat.unwrap(); - let lstat = lstat.unwrap(); if lstat.is_symlink() { return match stat { - Ok(stat) if stat.is_dir() => wgettext!("dir symlink"), - Ok(stat) - if (stat.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0 - && waccess(filename, X_OK) == 0 => - { - // Weird group permissions and other such issues make it non-trivial to find out if - // we can actually execute a file using the result from stat. It is much safer to - // use the access function, since it tells us exactly what we want to know. - wgettext!("command link") - } - Ok(_) => wgettext!("symlink"), - Err(e) if e.kind() == ErrorKind::NotFound => wgettext!("broken symlink"), - Err(e) if e.raw_os_error().unwrap() == ELOOP => wgettext!("symlink loop"), + Ok(stat) if stat.is_dir() => *COMPLETE_DIRECTORY_SYMLINK_DESC, + Ok(stat) if is_executable(&stat, filename) => *COMPLETE_EXEC_LINK_DESC, + Ok(_) => *COMPLETE_SYMLINK_DESC, + Err(e) if e.kind() == ErrorKind::NotFound => *COMPLETE_BROKEN_SYMLINK_DESC, + Err(e) if e.raw_os_error().unwrap() == ELOOP => *COMPLETE_LOOP_SYMLINK_DESC, _ => { // On unknown errors we do nothing. The file will be given the default 'File' // description or one based on the suffix. - wgettext!("file") + *COMPLETE_FILE_DESC } }; } let Ok(stat) = stat else { // Assuming that the metadata was zero if stat-call failed - return wgettext!("file"); + return *COMPLETE_FILE_DESC; }; - if stat.file_type().is_char_device() { - wgettext!("char device") - } else if stat.file_type().is_block_device() { - wgettext!("block device") - } else if stat.file_type().is_fifo() { - wgettext!("fifo") - } else if stat.file_type().is_socket() { - wgettext!("socket") - } else if stat.is_dir() { - wgettext!("directory") - } else if (stat.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0 - && waccess(filename, X_OK) == 0 - { - // Weird group permissions and other such issues make it non-trivial to find out if we can - // actually execute a file using the result from stat. It is much safer to use the access - // function, since it tells us exactly what we want to know. - wgettext!("command") + let ft = stat.file_type(); + if ft.is_char_device() { + *COMPLETE_CHAR_DESC + } else if ft.is_block_device() { + *COMPLETE_BLOCK_DESC + } else if ft.is_fifo() { + *COMPLETE_FIFO_DESC + } else if ft.is_socket() { + *COMPLETE_SOCKET_DESC + } else if ft.is_dir() { + *COMPLETE_DIRECTORY_DESC + } else if is_executable(&stat, filename) { + *COMPLETE_EXEC_DESC } else { - wgettext!("file") + *COMPLETE_FILE_DESC } } @@ -369,7 +379,7 @@ fn wildcard_test_flags_then_complete( filename: &wstr, wc: &wstr, expand_flags: ExpandFlags, - out: Option<&mut CompletionReceiver>, + out: &mut CompletionReceiver, known_dir: bool, ) -> bool { let executables_only = expand_flags.contains(ExpandFlags::EXECUTABLES_ONLY); @@ -380,10 +390,10 @@ fn wildcard_test_flags_then_complete( // supports it. if known_dir && !executables_only && !expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { return wildcard_complete( - &(WString::from(filename) + L!("/")), + &(filename.to_owned() + L!("/")), wc, Some(&|_| L!("").to_owned()), - out, + Some(out), expand_flags, CompleteFlags::NO_SPACE, ) == WildcardResult::Match; @@ -415,10 +425,11 @@ fn wildcard_test_flags_then_complete( stat = None; } - let (file_size, is_directory, is_executable) = stat - .as_ref() - .and_then(|s| s.as_ref().map(|s| (s.len(), s.is_dir(), s.is_file())).ok()) - .unwrap_or_default(); + let (file_size, is_directory, is_executable) = if let Some(Ok(md)) = &stat { + (md.len(), md.is_dir(), md.is_file()) + } else { + (0, false, false) + }; if need_directory && !is_directory { return false; @@ -436,28 +447,34 @@ fn wildcard_test_flags_then_complete( } // Compute the description. - let mut desc = WString::new(); - if expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { - desc = file_get_desc(filename, lstat, stat).to_owned(); + let desc = if expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { + let mut desc = file_get_desc(filename, lstat, stat).to_owned(); if !is_directory && !is_executable { if !desc.is_empty() { desc.push_utfstr(L!(", ")); } - desc.push_utfstr(&format_size(file_size as i64)); + desc.push_utfstr(&format_size(file_size.try_into().unwrap())); } - } + + Some(desc) + } else { + None + }; // Append a / if this is a directory. Note this requirement may be the only reason we have to // call stat() in some cases. - let x = |_: &wstr| desc.clone(); + let x = |_: &wstr| match desc.as_ref() { + Some(d) => d.to_owned(), + None => WString::new(), + }; let desc_func: Option<&dyn Fn(&wstr) -> WString> = Some(&x); if is_directory { return wildcard_complete( &(filename.to_owned() + L!("/")), wc, desc_func, - out, + Some(out), expand_flags, CompleteFlags::NO_SPACE, ) == WildcardResult::Match; @@ -467,7 +484,7 @@ fn wildcard_test_flags_then_complete( filename, wc, desc_func, - out, + Some(out), expand_flags, CompleteFlags::empty(), ) == WildcardResult::Match @@ -487,11 +504,11 @@ mod expander { use super::*; - pub struct WildCardExpander<'receiver, 'c> { + pub struct WildCardExpander<'e> { /// A function to call to check cancellation. - cancel_checker: &'c CancelChecker, + cancel_checker: &'e CancelChecker, /// The working directory to resolve paths against - working_directory: WString, + working_directory: &'e wstr, /// The set of items we have resolved, used to efficiently avoid duplication. completion_set: HashSet, /// The set of file IDs we have visited, used to avoid symlink loops. @@ -499,7 +516,7 @@ mod expander { /// Flags controlling expansion. flags: ExpandFlags, /// Resolved items get inserted into here. This is transient of course. - resolved_completions: &'receiver mut CompletionReceiver, + resolved_completions: &'e mut CompletionReceiver, /// Whether we have been interrupted. did_interrupt: bool, /// Whether we have overflowed. @@ -512,12 +529,12 @@ mod expander { has_fuzzy_ancestor: bool, } - impl<'receiver, 'c> WildCardExpander<'receiver, 'c> { + impl<'e> WildCardExpander<'e> { pub fn new( - working_directory: WString, + working_directory: &'e wstr, flags: ExpandFlags, - cancel_checker: &'c CancelChecker, - resolved_completions: &'receiver mut CompletionReceiver, + cancel_checker: &'e CancelChecker, + resolved_completions: &'e mut CompletionReceiver, ) -> Self { Self { cancel_checker, @@ -554,12 +571,16 @@ mod expander { } // Get the current segment and compute interesting properties about it. - let next_slash = wc.find_char('/'); - let is_last_segment = next_slash.is_none(); - let wc_segment_len = next_slash.unwrap_or(wc.char_count()); - let wc_segment = wc.slice_to(wc_segment_len); + let (wc_segment, wc_remainder) = if let Some(next_slash) = wc.find_char('/') { + let (seg, rem) = wc.split_at(next_slash); + let rem_without_slash = rem.slice_from(1); + (seg, Some(rem_without_slash)) + } else { + (wc, None) + }; + let is_last_segment = wc_remainder.is_none(); + let segment_has_wildcards = wildcard_has_internal(wc_segment); - let wc_remainder = next_slash.map(|n| wc.slice_from(n)).unwrap_or(L!("")); if wc_segment.is_empty() { assert!(!segment_has_wildcards); @@ -568,21 +589,19 @@ mod expander { } else { let mut prefix = effective_prefix.to_owned(); prefix.push('/'); - self.expand(base_dir, wc_remainder, &prefix); + self.expand(base_dir, wc_remainder.unwrap(), &prefix); } } else if !segment_has_wildcards && !is_last_segment { // Literal intermediate match. Note that we may not be able to actually read the directory // (issue #2099). - assert!(next_slash.is_some()); + let wc_remainder = wc_remainder.unwrap(); // TODO: if-let-chains // Absolute path of the intermediate directory - let mut intermediate_dirpath = base_dir.to_owned() + wc_segment; - intermediate_dirpath.push('/'); + let intermediate_dirpath: WString = base_dir.to_owned() + wc_segment + L!("/"); // This just trumps everything let before = self.resolved_completions.len(); - let mut prefix = effective_prefix.to_owned() + wc_segment; - prefix.push('/'); + let prefix: WString = effective_prefix.to_owned() + wc_segment + L!("/"); self.expand(&intermediate_dirpath, wc_remainder, &prefix); // Maybe try a fuzzy match (#94) if nothing was found with the literal match. Respect @@ -590,6 +609,7 @@ mod expander { // Don't do fuzzy matches if the literal segment was valid (#3211) let allow_fuzzy = self.flags.contains(ExpandFlags::FUZZY_MATCH) && !self.flags.contains(ExpandFlags::NO_FUZZY_DIRECTORIES); + if allow_fuzzy && self.resolved_completions.len() == before && waccess(&intermediate_dirpath, F_OK) != 0 @@ -616,32 +636,32 @@ mod expander { // Implement this by matching the wildcard tail only, in this directory. // Note if the segment is not exactly ANY_STRING_RECURSIVE then the segment may only // match subdirectories. - self.expand(base_dir, wc_remainder, effective_prefix); + self.expand(base_dir, wc_remainder.unwrap(), effective_prefix); if self.interrupted_or_overflowed() { return; } } // return "." and ".." entries if we're doing completions - let Ok(mut dir) = self.open_dir(base_dir, /* return . and .. */ self.flags.contains(ExpandFlags::FOR_COMPLETIONS)) else { + let Ok(mut dir) = self.open_dir( + base_dir, /* return . and .. */ + self.flags.contains(ExpandFlags::FOR_COMPLETIONS), + ) else { return; }; - if is_last_segment { - // Last wildcard segment, nonempty wildcard. - self.expand_last_segment(base_dir, &mut dir, wc_segment, effective_prefix); - } else { + if let Some(wc_remainder) = wc_remainder { // Not the last segment, nonempty wildcard. - assert!(next_slash.is_some()); - let mut prefix = effective_prefix.to_owned() + wc_segment; - prefix.push('/'); self.expand_intermediate_segment( base_dir, &mut dir, wc_segment, wc_remainder, - &prefix, + &(effective_prefix.to_owned() + wc_segment + L!("/")), ); + } else { + // Last wildcard segment, nonempty wildcard. + self.expand_last_segment(base_dir, &mut dir, wc_segment, effective_prefix); } let Some(asr_idx) = wc_segment.find_char(ANY_STRING_RECURSIVE) else { @@ -655,7 +675,7 @@ mod expander { let head_any = wc_segment.slice_to(asr_idx + 1); let any_tail = wc.slice_from(asr_idx); assert!(head_any.chars().last().unwrap() == ANY_STRING_RECURSIVE); - assert!(any_tail.char_at(0) == ANY_STRING_RECURSIVE); + assert!(any_tail.chars().next().unwrap() == ANY_STRING_RECURSIVE); dir.rewind(); self.expand_intermediate_segment( @@ -673,15 +693,15 @@ mod expander { return WildcardResult::Cancel; } else if self.did_overflow { return WildcardResult::Overflow; - } - match self.did_add { - true => WildcardResult::Match, - false => WildcardResult::NoMatch, + } else if self.did_add { + WildcardResult::Match + } else { + WildcardResult::NoMatch } } } - impl<'receiver, 'c> WildCardExpander<'receiver, 'c> { + impl<'e> WildCardExpander<'e> { /// We are a trailing slash - expand at the end. fn expand_trailing_slash(&mut self, base_dir: &wstr, prefix: &wstr) { if self.interrupted_or_overflowed() { @@ -714,7 +734,7 @@ mod expander { }; if !entry.name.is_empty() && !entry.name.starts_with('.') { self.try_add_completion_result( - &(WString::from(base_dir) + entry.name.as_utfstr()), + &(base_dir.to_owned() + entry.name.as_utfstr()), &entry.name, L!(""), prefix, @@ -759,11 +779,9 @@ mod expander { continue; } - let mut full_path: WString = base_dir.to_owned() + entry.name.as_utfstr(); - full_path.push('/'); + let full_path: WString = base_dir.to_owned() + entry.name.as_utfstr() + L!("/"); + let prefix: WString = prefix.to_owned() + wc_segment + L!("/"); - let mut prefix: WString = prefix.to_owned() + wc_segment; - prefix.push('/'); self.expand(&full_path, wc_remainder, &prefix); // Now remove the visited file. This is for #2414: only directories "beneath" us should be @@ -797,6 +815,7 @@ mod expander { let Some(m) = string_fuzzy_match_string(wc_segment, &entry.name, false) else { continue; }; + // The first port had !n.is_samecase_exact if m.is_samecase_exact() { continue; } @@ -810,12 +829,8 @@ mod expander { // Normally this would be the wildcard segment, but here we know our segment doesn't have // wildcards ("literal") and we are doing fuzzy expansion, which means we replace the // segment with files found through fuzzy matching. - let mut child_prefix = prefix.to_owned() + entry.name.as_utfstr(); - child_prefix.push('/'); - let child_prefix = child_prefix; - - let mut new_full_path = base_dir.to_owned() + entry.name.as_utfstr(); - new_full_path.push('/'); + let child_prefix: WString = prefix.to_owned() + entry.name.as_utfstr() + L!("/"); + let new_full_path: WString = base_dir.to_owned() + entry.name.as_utfstr() + L!("/"); // Ok, this directory matches. Recurse to it. Then mark each resulting completion as fuzzy. let before = this.resolved_completions.len(); @@ -823,10 +838,8 @@ mod expander { let after = this.resolved_completions.len(); assert!(before <= after); - for i in before..after { + for c in this.resolved_completions[before..after].iter_mut() { // Mark the completion as replacing. - let c = this.resolved_completions.get_mut(i).unwrap(); - if !c.replaces_token() { c.flags |= CompleteFlags::REPLACES_TOKEN; c.prepend_token_prefix(&child_prefix); @@ -895,9 +908,11 @@ mod expander { fn add_expansion_result(&mut self, result: WString) { // This function is only for the non-completions case. assert!(!self.flags.contains(ExpandFlags::FOR_COMPLETIONS)); - if self.completion_set.insert(result.clone()) && !self.resolved_completions.add(result) - { - self.did_overflow = true; + #[allow(clippy::collapsible_if)] + if self.completion_set.insert(result.clone()) { + if !self.resolved_completions.add(result) { + self.did_overflow = true; + } } } @@ -909,7 +924,7 @@ mod expander { fn descend_unique_hierarchy(&mut self, start_point: &mut WString) -> WString { assert!(!start_point.is_empty() && !start_point.starts_with('/')); - let mut unique_hierarchy = WString::from(""); + let mut unique_hierarchy = WString::new(); let abs_unique_hierarchy = start_point; // Ensure we don't fall into a symlink loop. @@ -968,7 +983,7 @@ mod expander { ) { // This function is only for the completions case. assert!(self.flags.contains(ExpandFlags::FOR_COMPLETIONS)); - let mut abs_path = self.working_directory.clone(); + let mut abs_path = self.working_directory.to_owned(); append_path_component(&mut abs_path, filepath); // We must normalize the path to allow 'cd ..' to operate on logical paths. @@ -983,15 +998,14 @@ mod expander { filename, wildcard, self.flags, - Some(self.resolved_completions), + self.resolved_completions, known_dir, ) { // Hack. We added this completion result based on the last component of the wildcard. // Prepend our prefix to each wildcard that replaces its token. // Note that prepend_token_prefix is a no-op unless COMPLETE_REPLACES_TOKEN is set let after = self.resolved_completions.len(); - for i in before..after { - let c = self.resolved_completions.get_mut(i).unwrap(); + for c in self.resolved_completions[before..after].iter_mut() { if self.has_fuzzy_ancestor && !(c.flags.contains(CompleteFlags::REPLACES_TOKEN)) { c.flags |= CompleteFlags::REPLACES_TOKEN; @@ -1010,8 +1024,7 @@ mod expander { { let unique_hierarchy = self.descend_unique_hierarchy(&mut abs_path); if !unique_hierarchy.is_empty() { - for i in before..after { - let c = self.resolved_completions.get_mut(i).unwrap(); + for c in self.resolved_completions[before..after].iter_mut() { c.completion.push_utfstr(&unique_hierarchy); } } @@ -1024,7 +1037,7 @@ mod expander { // Helper to resolve using our prefix. /// dotdot default is false fn open_dir(&self, base_dir: &wstr, dotdot: bool) -> std::io::Result { - let mut path = self.working_directory.clone(); + let mut path = self.working_directory.to_owned(); append_path_component(&mut path, base_dir); if self.flags.contains(ExpandFlags::SPECIAL_FOR_CD) { // cd operates on logical paths. @@ -1100,20 +1113,13 @@ pub fn wildcard_expand_string( // Check for a leading slash. If we find one, we have an absolute path: the prefix is empty, the // base dir is /, and the wildcard is the remainder. If we don't find one, the prefix is the // working directory, the base dir is empty. - let prefix; - let base_dir; - let effective_wc; - if wc.starts_with(L!("/")) { - prefix = L!(""); - base_dir = L!("/"); - effective_wc = wc.slice_from(1); + let (prefix, base_dir, effective_wc) = if wc.starts_with(L!("/")) { + (L!(""), L!("/"), wc.slice_from(1)) } else { - prefix = working_directory; - base_dir = L!(""); - effective_wc = wc; - } + (working_directory, L!(""), wc) + }; - let mut expander = WildCardExpander::new(prefix.to_owned(), flags, cancel_checker, output); + let mut expander = WildCardExpander::new(prefix, flags, cancel_checker, output); expander.expand(base_dir, effective_wc, base_dir); return expander.status_code(); } From d6a9ad66a7e0796a62f8d01964b885294ed4853e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20H=C3=B8rl=C3=BCck=20Berg?= <36937807+henrikhorluck@users.noreply.github.com> Date: Sun, 27 Aug 2023 19:35:31 +0200 Subject: [PATCH 4/7] Allow CancelChecker to be FnMut --- fish-rust/src/wildcard.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/fish-rust/src/wildcard.rs b/fish-rust/src/wildcard.rs index fb1c33a53..b233bb26a 100644 --- a/fish-rust/src/wildcard.rs +++ b/fish-rust/src/wildcard.rs @@ -10,8 +10,8 @@ use cxx::CxxWString; use libc::{mode_t, ELOOP, S_IXGRP, S_IXOTH, S_IXUSR, X_OK}; use crate::common::{ - char_offset, format_size, is_windows_subsystem_for_linux, unescape_string, CancelChecker, - UnescapeFlags, UnescapeStringStyle, WILDCARD_RESERVED_BASE, + char_offset, format_size, is_windows_subsystem_for_linux, unescape_string, UnescapeFlags, + UnescapeStringStyle, WILDCARD_RESERVED_BASE, }; use crate::complete::{CompleteFlags, Completion, CompletionReceiver, PROG_COMPLETE_SEP}; use crate::expand::ExpandFlags; @@ -506,7 +506,7 @@ mod expander { pub struct WildCardExpander<'e> { /// A function to call to check cancellation. - cancel_checker: &'e CancelChecker, + cancel_checker: &'e mut dyn FnMut() -> bool, /// The working directory to resolve paths against working_directory: &'e wstr, /// The set of items we have resolved, used to efficiently avoid duplication. @@ -533,7 +533,7 @@ mod expander { pub fn new( working_directory: &'e wstr, flags: ExpandFlags, - cancel_checker: &'e CancelChecker, + cancel_checker: &'e mut dyn FnMut() -> bool, resolved_completions: &'e mut CompletionReceiver, ) -> Self { Self { @@ -1070,11 +1070,11 @@ mod expander { /// executables_only /// \param output The list in which to put the output /// -pub fn wildcard_expand_string( +pub fn wildcard_expand_string<'closure>( wc: &wstr, working_directory: &wstr, flags: ExpandFlags, - cancel_checker: &CancelChecker, + mut cancel_checker: impl FnMut() -> bool + 'closure, output: &mut CompletionReceiver, ) -> WildcardResult { // Fuzzy matching only if we're doing completions. @@ -1119,7 +1119,7 @@ pub fn wildcard_expand_string( (working_directory, L!(""), wc) }; - let mut expander = WildCardExpander::new(prefix, flags, cancel_checker, output); + let mut expander = WildCardExpander::new(prefix, flags, &mut cancel_checker, output); expander.expand(base_dir, effective_wc, base_dir); return expander.status_code(); } From 5407d0b7859c27bbc72c80ef26bc579be09df953 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20H=C3=B8rl=C3=BCck=20Berg?= <36937807+henrikhorluck@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:38:49 +0200 Subject: [PATCH 5/7] Apply code review fixes --- fish-rust/src/complete.rs | 6 ------ fish-rust/src/wildcard.rs | 18 +++++++----------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/fish-rust/src/complete.rs b/fish-rust/src/complete.rs index 789450602..8d9b846d3 100644 --- a/fish-rust/src/complete.rs +++ b/fish-rust/src/complete.rs @@ -160,12 +160,6 @@ impl CompletionReceiver { true } - /// Swap our completions with a new list. - pub fn swap(&mut self, lst: &mut Vec) { - // XXX: This completly breaks our completions.len() <= self.limit invariant - std::mem::swap(&mut self.completions, lst); - } - /// Clear the list of completions. This retains the storage inside completions_ which can be /// useful to prevent allocations. pub fn clear(&mut self) { diff --git a/fish-rust/src/wildcard.rs b/fish-rust/src/wildcard.rs index b233bb26a..49bf94e02 100644 --- a/fish-rust/src/wildcard.rs +++ b/fish-rust/src/wildcard.rs @@ -69,13 +69,9 @@ fn resolve_description<'f>( ) -> WString { if let Some(complete_sep_loc) = completion.find_char(PROG_COMPLETE_SEP) { // This completion has an embedded description, do not use the generic description. - assert!( - completion.len() > complete_sep_loc, - "resolve_description lacks a length assumption" - ); - let (comp, description) = completion.split_at(complete_sep_loc + 1); - *completion = comp; - return description.to_owned(); + let description = completion[complete_sep_loc + 1..].to_owned(); + *completion = &completion[..complete_sep_loc]; + return description; } if let Some(f) = description_func { @@ -143,7 +139,7 @@ fn wildcard_complete_internal( // If we're not allowing fuzzy match, then we require a prefix match. let needs_prefix_match = !params.expand_flags.contains(ExpandFlags::FUZZY_MATCH); - if needs_prefix_match && m.is_exact_or_prefix() { + if needs_prefix_match && !m.is_exact_or_prefix() { return WildcardResult::NoMatch; } @@ -464,11 +460,11 @@ fn wildcard_test_flags_then_complete( // Append a / if this is a directory. Note this requirement may be the only reason we have to // call stat() in some cases. - let x = |_: &wstr| match desc.as_ref() { + let desc_func = |_: &wstr| match desc.as_ref() { Some(d) => d.to_owned(), None => WString::new(), }; - let desc_func: Option<&dyn Fn(&wstr) -> WString> = Some(&x); + let desc_func: Option<&dyn Fn(&wstr) -> WString> = Some(&desc_func); if is_directory { return wildcard_complete( &(filename.to_owned() + L!("/")), @@ -874,7 +870,7 @@ mod expander { break; }; - if need_dir && entry.is_dir() { + if need_dir && !entry.is_dir() { continue; } From 6ee81c0f1507c91d7030513f1a5411b282cd71a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20H=C3=B8rl=C3=BCck=20Berg?= <36937807+henrikhorluck@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:46:53 +0200 Subject: [PATCH 6/7] Crash if invariant is broken --- fish-rust/src/complete.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fish-rust/src/complete.rs b/fish-rust/src/complete.rs index 8d9b846d3..ea149c9a7 100644 --- a/fish-rust/src/complete.rs +++ b/fish-rust/src/complete.rs @@ -174,8 +174,10 @@ impl CompletionReceiver { /// \return a new, empty receiver whose limit is our remaining capacity. /// This is useful for e.g. recursive calls when you want to act on the result before adding it. pub fn subreceiver(&self) -> Self { - // XXX: this should not need to be saturating, we have a faulty invariant - let remaining_capacity = self.limit.saturating_sub(self.completions.len()); + let remaining_capacity = self + .limit + .checked_sub(self.completions.len()) + .expect("length should never be larger than limit"); Self::new(remaining_capacity) } } From 0cc1aef725da68be48265202d40a177cb7e686d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20H=C3=B8rl=C3=BCck=20Berg?= <36937807+henrikhorluck@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:58:54 +0200 Subject: [PATCH 7/7] Forward-port #9931 --- fish-rust/src/wildcard.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/fish-rust/src/wildcard.rs b/fish-rust/src/wildcard.rs index 49bf94e02..8704770f6 100644 --- a/fish-rust/src/wildcard.rs +++ b/fish-rust/src/wildcard.rs @@ -315,17 +315,20 @@ fn file_get_desc( filename: &wstr, lstat: Option, stat: Option>, + definitely_executable: bool, ) -> &'static wstr { let Some(lstat) = lstat else { return *COMPLETE_FILE_DESC; }; - fn is_executable(buf: &fs::Metadata, filename: &wstr) -> bool { + let is_executable = |buf: &fs::Metadata, filename: &wstr| -> bool { // Weird group permissions and other such issues make it non-trivial to find out if // we can actually execute a file using the result from stat. It is much safer to // use the access function, since it tells us exactly what we want to know. - (buf.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH) != 0) && waccess(filename, X_OK) == 0 - } + definitely_executable + || (buf.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH) != 0) + && waccess(filename, X_OK) == 0 + }; // stat was only queried if lstat succeeded let stat = stat.unwrap(); @@ -444,7 +447,7 @@ fn wildcard_test_flags_then_complete( // Compute the description. let desc = if expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { - let mut desc = file_get_desc(filename, lstat, stat).to_owned(); + let mut desc = file_get_desc(filename, lstat, stat, executables_only).to_owned(); if !is_directory && !is_executable { if !desc.is_empty() {