From 9e738193f3355c04b4c0b2e8340fe273e73572e4 Mon Sep 17 00:00:00 2001 From: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Date: Wed, 3 Jul 2024 07:48:06 -0400 Subject: [PATCH] Force completers to sort in fetch() (#13242) # Description This PR fixes the problem pointed out in https://github.com/nushell/nushell/issues/13204, where the Fish-like completions aren't sorted properly (this PR doesn't close that issue because the author there wants more than just fixed sort order). The cause is all of the file/directory completions being fetched first and then sorted all together while being treated as strings. Instead, this PR sorts completions within each individual directory, avoiding treating `/` as part of the path. To do this, I removed the `sort` method from the completer trait (as well as `get_sort_by`) and made all completers sort within the `fetch` method itself. A generic `sort_completions` helper has been added to sort lists of completions, and a more specific `sort_suggestions` helper has been added to sort `Vec`s. As for the actual change that fixes the sort order for file/directory completions, the `complete_rec` helper now sorts the children of each directory before visiting their children. The file and directory completers don't bother sorting at the end (except to move hidden files down). To reviewers: don't let the 29 changed files scare you, most of those are just the test fixtures :) # User-Facing Changes This is the current behavior with prefix matching: ![image](https://github.com/nushell/nushell/assets/45539777/6a36e003-8405-45b5-8cbe-d771e0592709) And with fuzzy matching: ![image](https://github.com/nushell/nushell/assets/45539777/f2cbfdb2-b8fd-491b-a378-779147291d2a) Notice how `partial/hello.txt` is the last suggestion, even though it should come before `partial-a`. This is because the ASCII code for `/` is greater than that of `-`, so `partial-` is put before `partial/`. This is this PR's behavior with prefix matching (`partial/hello.txt` is at the start): ![image](https://github.com/nushell/nushell/assets/45539777/3fcea7c9-e017-428f-aa9c-1707e3ab32e0) And with fuzzy matching: ![image](https://github.com/nushell/nushell/assets/45539777/d55635d4-cdb8-440a-84d6-41111499f9f8) # Tests + Formatting - Modified the partial completions test fixture to test whether this PR even fixed anything - Modified fixture to test sort order of .nu completions (a previous version of my changes didn't sort all the completions at the end but there were no tests catching that) - Added a test for making sure subcommand completions are sorted by Levenshtein distance (a previous version of my changes sorted in alphabetical order but there were no tests catching that) # After Submitting --- crates/nu-cli/src/completions/base.rs | 33 +--- .../src/completions/command_completions.rs | 20 ++- crates/nu-cli/src/completions/completer.rs | 10 +- .../src/completions/completion_common.rs | 75 +++++++-- .../src/completions/custom_completions.rs | 11 +- .../src/completions/directory_completions.rs | 40 +---- .../src/completions/dotnu_completions.rs | 10 +- .../src/completions/file_completions.rs | 40 +---- .../src/completions/flag_completions.rs | 6 +- .../src/completions/variable_completions.rs | 11 +- crates/nu-cli/tests/completions/mod.rs | 146 ++++++++++++------ .../support/completions_helpers.rs | 56 ++++++- .../dir_module/mod.nu} | 0 .../have_ext.exe => dotnu_completions/foo.nu} | 0 .../lib-dir1/bar.nu} | 0 .../lib-dir1/baz.nu} | 0 .../lib-dir1/xyzzy.nu} | 0 .../lib-dir2/asdf.nu} | 0 .../lib-dir2/bat.nu} | 0 .../lib-dir3/spam.nu} | 0 .../partial_completions/partial-a/anotherfile | 0 .../partial-a/have_ext.exe | 0 .../partial-a/have_ext.txt | 0 .../partial_completions/partial-a/hello | 0 .../partial_completions/partial-a/hola | 0 .../partial_completions/partial-b/hello_b | 0 .../partial_completions/partial-b/hi_b | 0 .../partial_completions/partial-c/hello_c | 0 .../partial_completions/partial/hello.txt | 0 29 files changed, 259 insertions(+), 199 deletions(-) rename tests/fixtures/{partial_completions/partial_a/anotherfile => dotnu_completions/dir_module/mod.nu} (100%) rename tests/fixtures/{partial_completions/partial_a/have_ext.exe => dotnu_completions/foo.nu} (100%) rename tests/fixtures/{partial_completions/partial_a/have_ext.txt => dotnu_completions/lib-dir1/bar.nu} (100%) rename tests/fixtures/{partial_completions/partial_a/hello => dotnu_completions/lib-dir1/baz.nu} (100%) rename tests/fixtures/{partial_completions/partial_a/hola => dotnu_completions/lib-dir1/xyzzy.nu} (100%) rename tests/fixtures/{partial_completions/partial_b/hello_b => dotnu_completions/lib-dir2/asdf.nu} (100%) rename tests/fixtures/{partial_completions/partial_b/hi_b => dotnu_completions/lib-dir2/bat.nu} (100%) rename tests/fixtures/{partial_completions/partial_c/hello_c => dotnu_completions/lib-dir3/spam.nu} (100%) create mode 100644 tests/fixtures/partial_completions/partial-a/anotherfile create mode 100644 tests/fixtures/partial_completions/partial-a/have_ext.exe create mode 100644 tests/fixtures/partial_completions/partial-a/have_ext.txt create mode 100644 tests/fixtures/partial_completions/partial-a/hello create mode 100644 tests/fixtures/partial_completions/partial-a/hola create mode 100644 tests/fixtures/partial_completions/partial-b/hello_b create mode 100644 tests/fixtures/partial_completions/partial-b/hi_b create mode 100644 tests/fixtures/partial_completions/partial-c/hello_c create mode 100644 tests/fixtures/partial_completions/partial/hello.txt diff --git a/crates/nu-cli/src/completions/base.rs b/crates/nu-cli/src/completions/base.rs index 0debabe688..cf13dae68c 100644 --- a/crates/nu-cli/src/completions/base.rs +++ b/crates/nu-cli/src/completions/base.rs @@ -1,13 +1,12 @@ -use crate::completions::{CompletionOptions, SortBy}; +use crate::completions::CompletionOptions; use nu_protocol::{ engine::{Stack, StateWorkingSet}, - levenshtein_distance, Span, + Span, }; use reedline::Suggestion; -// Completer trait represents the three stages of the completion -// fetch, filter and sort pub trait Completer { + /// Fetch, filter, and sort completions #[allow(clippy::too_many_arguments)] fn fetch( &mut self, @@ -19,32 +18,6 @@ pub trait Completer { pos: usize, options: &CompletionOptions, ) -> Vec; - - fn get_sort_by(&self) -> SortBy { - SortBy::Ascending - } - - fn sort(&self, items: Vec, prefix: Vec) -> Vec { - let prefix_str = String::from_utf8_lossy(&prefix).to_string(); - let mut filtered_items = items; - - // Sort items - match self.get_sort_by() { - SortBy::LevenshteinDistance => { - filtered_items.sort_by(|a, b| { - let a_distance = levenshtein_distance(&prefix_str, &a.suggestion.value); - let b_distance = levenshtein_distance(&prefix_str, &b.suggestion.value); - a_distance.cmp(&b_distance) - }); - } - SortBy::Ascending => { - filtered_items.sort_by(|a, b| a.suggestion.value.cmp(&b.suggestion.value)); - } - SortBy::None => {} - }; - - filtered_items - } } #[derive(Debug, Default, PartialEq)] diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs index 2549854540..e37f8b2576 100644 --- a/crates/nu-cli/src/completions/command_completions.rs +++ b/crates/nu-cli/src/completions/command_completions.rs @@ -9,7 +9,7 @@ use nu_protocol::{ }; use reedline::Suggestion; -use super::SemanticSuggestion; +use super::{completion_common::sort_suggestions, SemanticSuggestion}; pub struct CommandCompletion { flattened: Vec<(Span, FlatShape)>, @@ -161,7 +161,7 @@ impl Completer for CommandCompletion { &mut self, working_set: &StateWorkingSet, _stack: &Stack, - _prefix: Vec, + prefix: Vec, span: Span, offset: usize, pos: usize, @@ -198,7 +198,11 @@ impl Completer for CommandCompletion { }; if !subcommands.is_empty() { - return subcommands; + return sort_suggestions( + &String::from_utf8_lossy(&prefix), + subcommands, + SortBy::LevenshteinDistance, + ); } let config = working_set.get_config(); @@ -223,11 +227,11 @@ impl Completer for CommandCompletion { vec![] }; - subcommands.into_iter().chain(commands).collect::>() - } - - fn get_sort_by(&self) -> SortBy { - SortBy::LevenshteinDistance + sort_suggestions( + &String::from_utf8_lossy(&prefix), + commands, + SortBy::LevenshteinDistance, + ) } } diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index 007a0e288a..78398b4d19 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -51,8 +51,7 @@ impl NuCompleter { ..Default::default() }; - // Fetch - let mut suggestions = completer.fetch( + completer.fetch( working_set, &self.stack, prefix.clone(), @@ -60,12 +59,7 @@ impl NuCompleter { offset, pos, &options, - ); - - // Sort - suggestions = completer.sort(suggestions, prefix); - - suggestions + ) } fn external_completion( diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs index 26d48a8675..b3bca778a9 100644 --- a/crates/nu-cli/src/completions/completion_common.rs +++ b/crates/nu-cli/src/completions/completion_common.rs @@ -1,16 +1,21 @@ -use crate::completions::{matches, CompletionOptions}; +use crate::{ + completions::{matches, CompletionOptions}, + SemanticSuggestion, +}; use nu_ansi_term::Style; use nu_engine::env_to_string; use nu_path::{expand_to_real_path, home_dir}; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, - Span, + levenshtein_distance, Span, }; use nu_utils::get_ls_colors; use std::path::{ is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP, MAIN_SEPARATOR_STR, }; +use super::SortBy; + #[derive(Clone, Default)] pub struct PathBuiltFromString { parts: Vec, @@ -45,6 +50,7 @@ fn complete_rec( return completions; }; + let mut entries = Vec::new(); for entry in result.filter_map(|e| e.ok()) { let entry_name = entry.file_name().to_string_lossy().into_owned(); let entry_isdir = entry.path().is_dir(); @@ -53,20 +59,26 @@ fn complete_rec( built.isdir = entry_isdir; if !dir || entry_isdir { - match partial.split_first() { - Some((base, rest)) => { - if matches(base, &entry_name, options) { - if !rest.is_empty() || isdir { - completions - .extend(complete_rec(rest, &built, cwd, options, dir, isdir)); - } else { - completions.push(built); - } + entries.push((entry_name, built)); + } + } + + let prefix = partial.first().unwrap_or(&""); + let sorted_entries = sort_completions(prefix, entries, SortBy::Ascending, |(entry, _)| entry); + + for (entry_name, built) in sorted_entries { + match partial.split_first() { + Some((base, rest)) => { + if matches(base, &entry_name, options) { + if !rest.is_empty() || isdir { + completions.extend(complete_rec(rest, &built, cwd, options, dir, isdir)); + } else { + completions.push(built); } } - None => { - completions.push(built); - } + } + None => { + completions.push(built); } } } @@ -256,3 +268,38 @@ pub fn adjust_if_intermediate( readjusted, } } + +/// Convenience function to sort suggestions using [`sort_completions`] +pub fn sort_suggestions( + prefix: &str, + items: Vec, + sort_by: SortBy, +) -> Vec { + sort_completions(prefix, items, sort_by, |it| &it.suggestion.value) +} + +/// # Arguments +/// * `prefix` - What the user's typed, for sorting by Levenshtein distance +pub fn sort_completions( + prefix: &str, + mut items: Vec, + sort_by: SortBy, + get_value: fn(&T) -> &str, +) -> Vec { + // Sort items + match sort_by { + SortBy::LevenshteinDistance => { + items.sort_by(|a, b| { + let a_distance = levenshtein_distance(prefix, get_value(a)); + let b_distance = levenshtein_distance(prefix, get_value(b)); + a_distance.cmp(&b_distance) + }); + } + SortBy::Ascending => { + items.sort_by(|a, b| get_value(a).cmp(get_value(b))); + } + SortBy::None => {} + }; + + items +} diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs index 5d45bb708e..5cab0a8fa2 100644 --- a/crates/nu-cli/src/completions/custom_completions.rs +++ b/crates/nu-cli/src/completions/custom_completions.rs @@ -12,6 +12,8 @@ use nu_protocol::{ use nu_utils::IgnoreCaseExt; use std::collections::HashMap; +use super::completion_common::sort_suggestions; + pub struct CustomCompletion { stack: Stack, decl_id: usize, @@ -125,15 +127,12 @@ impl Completer for CustomCompletion { }) .unwrap_or_default(); - if let Some(custom_completion_options) = custom_completion_options { + let suggestions = if let Some(custom_completion_options) = custom_completion_options { filter(&prefix, suggestions, &custom_completion_options) } else { filter(&prefix, suggestions, completion_options) - } - } - - fn get_sort_by(&self) -> SortBy { - self.sort_by + }; + sort_suggestions(&String::from_utf8_lossy(&prefix), suggestions, self.sort_by) } } diff --git a/crates/nu-cli/src/completions/directory_completions.rs b/crates/nu-cli/src/completions/directory_completions.rs index 024322f997..61b0439444 100644 --- a/crates/nu-cli/src/completions/directory_completions.rs +++ b/crates/nu-cli/src/completions/directory_completions.rs @@ -1,14 +1,14 @@ use crate::completions::{ completion_common::{adjust_if_intermediate, complete_item, AdjustView}, - Completer, CompletionOptions, SortBy, + Completer, CompletionOptions, }; use nu_ansi_term::Style; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, - levenshtein_distance, Span, + Span, }; use reedline::Suggestion; -use std::path::{Path, MAIN_SEPARATOR as SEP}; +use std::path::Path; use super::SemanticSuggestion; @@ -36,7 +36,7 @@ impl Completer for DirectoryCompletion { // Filter only the folders #[allow(deprecated)] - let output: Vec<_> = directory_completion( + let items: Vec<_> = directory_completion( span, &prefix, &working_set.permanent_state.current_work_dir(), @@ -62,41 +62,11 @@ impl Completer for DirectoryCompletion { }) .collect(); - output - } - - // Sort results prioritizing the non hidden folders - fn sort(&self, items: Vec, prefix: Vec) -> Vec { - let prefix_str = String::from_utf8_lossy(&prefix).to_string(); - - // Sort items - let mut sorted_items = items; - - match self.get_sort_by() { - SortBy::Ascending => { - sorted_items.sort_by(|a, b| { - // Ignore trailing slashes in folder names when sorting - a.suggestion - .value - .trim_end_matches(SEP) - .cmp(b.suggestion.value.trim_end_matches(SEP)) - }); - } - SortBy::LevenshteinDistance => { - sorted_items.sort_by(|a, b| { - let a_distance = levenshtein_distance(&prefix_str, &a.suggestion.value); - let b_distance = levenshtein_distance(&prefix_str, &b.suggestion.value); - a_distance.cmp(&b_distance) - }); - } - _ => (), - } - // Separate the results between hidden and non hidden let mut hidden: Vec = vec![]; let mut non_hidden: Vec = vec![]; - for item in sorted_items.into_iter() { + for item in items.into_iter() { let item_path = Path::new(&item.suggestion.value); if let Some(value) = item_path.file_name() { diff --git a/crates/nu-cli/src/completions/dotnu_completions.rs b/crates/nu-cli/src/completions/dotnu_completions.rs index c939578b41..1b2bbab20c 100644 --- a/crates/nu-cli/src/completions/dotnu_completions.rs +++ b/crates/nu-cli/src/completions/dotnu_completions.rs @@ -1,4 +1,4 @@ -use crate::completions::{file_path_completion, Completer, CompletionOptions, SortBy}; +use crate::completions::{file_path_completion, Completer, CompletionOptions}; use nu_protocol::{ engine::{Stack, StateWorkingSet}, Span, @@ -6,7 +6,7 @@ use nu_protocol::{ use reedline::Suggestion; use std::path::{is_separator, Path, MAIN_SEPARATOR as SEP, MAIN_SEPARATOR_STR}; -use super::SemanticSuggestion; +use super::{completion_common::sort_suggestions, SemanticSuggestion, SortBy}; #[derive(Clone, Default)] pub struct DotNuCompletion {} @@ -131,10 +131,6 @@ impl Completer for DotNuCompletion { }) .collect(); - output - } - - fn get_sort_by(&self) -> SortBy { - SortBy::LevenshteinDistance + sort_suggestions(&prefix_str, output, SortBy::Ascending) } } diff --git a/crates/nu-cli/src/completions/file_completions.rs b/crates/nu-cli/src/completions/file_completions.rs index f6205f6792..10a548e7ab 100644 --- a/crates/nu-cli/src/completions/file_completions.rs +++ b/crates/nu-cli/src/completions/file_completions.rs @@ -1,15 +1,15 @@ use crate::completions::{ completion_common::{adjust_if_intermediate, complete_item, AdjustView}, - Completer, CompletionOptions, SortBy, + Completer, CompletionOptions, }; use nu_ansi_term::Style; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, - levenshtein_distance, Span, + Span, }; use nu_utils::IgnoreCaseExt; use reedline::Suggestion; -use std::path::{Path, MAIN_SEPARATOR as SEP}; +use std::path::Path; use super::SemanticSuggestion; @@ -40,7 +40,7 @@ impl Completer for FileCompletion { } = adjust_if_intermediate(&prefix, working_set, span); #[allow(deprecated)] - let output: Vec<_> = complete_item( + let items: Vec<_> = complete_item( readjusted, span, &prefix, @@ -67,41 +67,13 @@ impl Completer for FileCompletion { }) .collect(); - output - } - - // Sort results prioritizing the non hidden folders - fn sort(&self, items: Vec, prefix: Vec) -> Vec { - let prefix_str = String::from_utf8_lossy(&prefix).to_string(); - - // Sort items - let mut sorted_items = items; - - match self.get_sort_by() { - SortBy::Ascending => { - sorted_items.sort_by(|a, b| { - // Ignore trailing slashes in folder names when sorting - a.suggestion - .value - .trim_end_matches(SEP) - .cmp(b.suggestion.value.trim_end_matches(SEP)) - }); - } - SortBy::LevenshteinDistance => { - sorted_items.sort_by(|a, b| { - let a_distance = levenshtein_distance(&prefix_str, &a.suggestion.value); - let b_distance = levenshtein_distance(&prefix_str, &b.suggestion.value); - a_distance.cmp(&b_distance) - }); - } - _ => (), - } + // Sort results prioritizing the non hidden folders // Separate the results between hidden and non hidden let mut hidden: Vec = vec![]; let mut non_hidden: Vec = vec![]; - for item in sorted_items.into_iter() { + for item in items.into_iter() { let item_path = Path::new(&item.suggestion.value); if let Some(value) = item_path.file_name() { diff --git a/crates/nu-cli/src/completions/flag_completions.rs b/crates/nu-cli/src/completions/flag_completions.rs index b0dcc0963b..ea4ddd6856 100644 --- a/crates/nu-cli/src/completions/flag_completions.rs +++ b/crates/nu-cli/src/completions/flag_completions.rs @@ -1,4 +1,6 @@ -use crate::completions::{Completer, CompletionOptions}; +use crate::completions::{ + completion_common::sort_suggestions, Completer, CompletionOptions, SortBy, +}; use nu_protocol::{ ast::{Expr, Expression}, engine::{Stack, StateWorkingSet}, @@ -90,7 +92,7 @@ impl Completer for FlagCompletion { } } - return output; + return sort_suggestions(&String::from_utf8_lossy(&prefix), output, SortBy::Ascending); } vec![] diff --git a/crates/nu-cli/src/completions/variable_completions.rs b/crates/nu-cli/src/completions/variable_completions.rs index 0572fe93c1..ab04da13ea 100644 --- a/crates/nu-cli/src/completions/variable_completions.rs +++ b/crates/nu-cli/src/completions/variable_completions.rs @@ -9,6 +9,8 @@ use nu_protocol::{ use reedline::Suggestion; use std::str; +use super::{completion_common::sort_suggestions, SortBy}; + #[derive(Clone)] pub struct VariableCompletion { var_context: (Vec, Vec>), // tuple with $var and the sublevels (.b.c.d) @@ -40,6 +42,7 @@ impl Completer for VariableCompletion { end: span.end - offset, }; let sublevels_count = self.var_context.1.len(); + let prefix_str = String::from_utf8_lossy(&prefix); // Completions for the given variable if !var_str.is_empty() { @@ -69,7 +72,7 @@ impl Completer for VariableCompletion { } } - return output; + return sort_suggestions(&prefix_str, output, SortBy::Ascending); } } else { // No nesting provided, return all env vars @@ -93,7 +96,7 @@ impl Completer for VariableCompletion { } } - return output; + return sort_suggestions(&prefix_str, output, SortBy::Ascending); } } @@ -117,7 +120,7 @@ impl Completer for VariableCompletion { } } - return output; + return sort_suggestions(&prefix_str, output, SortBy::Ascending); } } @@ -139,7 +142,7 @@ impl Completer for VariableCompletion { } } - return output; + return sort_suggestions(&prefix_str, output, SortBy::Ascending); } } } diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs index 35e9435b41..03f37925f5 100644 --- a/crates/nu-cli/tests/completions/mod.rs +++ b/crates/nu-cli/tests/completions/mod.rs @@ -11,7 +11,7 @@ use std::{ sync::Arc, }; use support::{ - completions_helpers::{new_partial_engine, new_quote_engine}, + completions_helpers::{new_dotnu_engine, new_partial_engine, new_quote_engine}, file, folder, match_suggestions, new_engine, }; @@ -85,6 +85,27 @@ fn custom_completer() -> NuCompleter { NuCompleter::new(Arc::new(engine), Arc::new(stack)) } +#[fixture] +fn subcommand_completer() -> NuCompleter { + // Create a new engine + let (dir, _, mut engine, mut stack) = new_engine(); + + // Use fuzzy matching, because subcommands are sorted by Levenshtein distance, + // and that's not very useful with prefix matching + let commands = r#" + $env.config.completions.algorithm = "fuzzy" + def foo [] {} + def "foo bar" [] {} + def "foo abaz" [] {} + def "foo aabrr" [] {} + def food [] {} + "#; + assert!(support::merge_input(commands.as_bytes(), &mut engine, &mut stack, dir).is_ok()); + + // Instantiate a new completer + NuCompleter::new(Arc::new(engine), Arc::new(stack)) +} + #[test] fn variables_dollar_sign_with_varialblecompletion() { let (_, _, engine, stack) = new_engine(); @@ -138,43 +159,42 @@ fn variables_customcompletion_subcommands_with_customcompletion_2( #[test] fn dotnu_completions() { // Create a new engine - let (_, _, engine, stack) = new_engine(); + let (_, _, engine, stack) = new_dotnu_engine(); // Instantiate a new completer let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); + let expected = vec![ + "asdf.nu".into(), + "bar.nu".into(), + "bat.nu".into(), + "baz.nu".into(), + #[cfg(windows)] + "dir_module\\".into(), + #[cfg(not(windows))] + "dir_module/".into(), + "foo.nu".into(), + "spam.nu".into(), + "xyzzy.nu".into(), + ]; + // Test source completion let completion_str = "source-env ".to_string(); let suggestions = completer.complete(&completion_str, completion_str.len()); - assert_eq!(2, suggestions.len()); - assert_eq!("custom_completion.nu", suggestions.first().unwrap().value); - #[cfg(windows)] - assert_eq!("directory_completion\\", suggestions.get(1).unwrap().value); - #[cfg(not(windows))] - assert_eq!("directory_completion/", suggestions.get(1).unwrap().value); + match_suggestions(expected.clone(), suggestions); // Test use completion let completion_str = "use ".to_string(); let suggestions = completer.complete(&completion_str, completion_str.len()); - assert_eq!(2, suggestions.len()); - assert_eq!("custom_completion.nu", suggestions.first().unwrap().value); - #[cfg(windows)] - assert_eq!("directory_completion\\", suggestions.get(1).unwrap().value); - #[cfg(not(windows))] - assert_eq!("directory_completion/", suggestions.get(1).unwrap().value); + match_suggestions(expected.clone(), suggestions); // Test overlay use completion let completion_str = "overlay use ".to_string(); let suggestions = completer.complete(&completion_str, completion_str.len()); - assert_eq!(2, suggestions.len()); - assert_eq!("custom_completion.nu", suggestions.first().unwrap().value); - #[cfg(windows)] - assert_eq!("directory_completion\\", suggestions.get(1).unwrap().value); - #[cfg(not(windows))] - assert_eq!("directory_completion/", suggestions.get(1).unwrap().value); + match_suggestions(expected, suggestions); } #[test] @@ -276,9 +296,10 @@ fn partial_completions() { // Create the expected values let expected_paths: Vec = vec![ - folder(dir.join("partial_a")), - folder(dir.join("partial_b")), - folder(dir.join("partial_c")), + folder(dir.join("partial")), + folder(dir.join("partial-a")), + folder(dir.join("partial-b")), + folder(dir.join("partial-c")), ]; // Match the results @@ -292,13 +313,14 @@ fn partial_completions() { // Create the expected values let expected_paths: Vec = vec![ - file(dir.join("partial_a").join("have_ext.exe")), - file(dir.join("partial_a").join("have_ext.txt")), - file(dir.join("partial_a").join("hello")), - file(dir.join("partial_a").join("hola")), - file(dir.join("partial_b").join("hello_b")), - file(dir.join("partial_b").join("hi_b")), - file(dir.join("partial_c").join("hello_c")), + file(dir.join("partial").join("hello.txt")), + file(dir.join("partial-a").join("have_ext.exe")), + file(dir.join("partial-a").join("have_ext.txt")), + file(dir.join("partial-a").join("hello")), + file(dir.join("partial-a").join("hola")), + file(dir.join("partial-b").join("hello_b")), + file(dir.join("partial-b").join("hi_b")), + file(dir.join("partial-c").join("hello_c")), ]; // Match the results @@ -311,14 +333,15 @@ fn partial_completions() { // Create the expected values let expected_paths: Vec = vec![ - file(dir.join("partial_a").join("anotherfile")), - file(dir.join("partial_a").join("have_ext.exe")), - file(dir.join("partial_a").join("have_ext.txt")), - file(dir.join("partial_a").join("hello")), - file(dir.join("partial_a").join("hola")), - file(dir.join("partial_b").join("hello_b")), - file(dir.join("partial_b").join("hi_b")), - file(dir.join("partial_c").join("hello_c")), + file(dir.join("partial").join("hello.txt")), + file(dir.join("partial-a").join("anotherfile")), + file(dir.join("partial-a").join("have_ext.exe")), + file(dir.join("partial-a").join("have_ext.txt")), + file(dir.join("partial-a").join("hello")), + file(dir.join("partial-a").join("hola")), + file(dir.join("partial-b").join("hello_b")), + file(dir.join("partial-b").join("hi_b")), + file(dir.join("partial-c").join("hello_c")), ]; // Match the results @@ -343,19 +366,25 @@ fn partial_completions() { // Create the expected values let expected_paths: Vec = vec![ file( - dir.join("partial_a") + dir.join("partial") .join("..") .join("final_partial") .join("somefile"), ), file( - dir.join("partial_b") + dir.join("partial-a") .join("..") .join("final_partial") .join("somefile"), ), file( - dir.join("partial_c") + dir.join("partial-b") + .join("..") + .join("final_partial") + .join("somefile"), + ), + file( + dir.join("partial-c") .join("..") .join("final_partial") .join("somefile"), @@ -366,28 +395,28 @@ fn partial_completions() { match_suggestions(expected_paths, suggestions); // Test completion for all files under directories whose names begin with "pa" - let file_str = file(dir.join("partial_a").join("have")); + let file_str = file(dir.join("partial-a").join("have")); let target_file = format!("rm {file_str}"); let suggestions = completer.complete(&target_file, target_file.len()); // Create the expected values let expected_paths: Vec = vec![ - file(dir.join("partial_a").join("have_ext.exe")), - file(dir.join("partial_a").join("have_ext.txt")), + file(dir.join("partial-a").join("have_ext.exe")), + file(dir.join("partial-a").join("have_ext.txt")), ]; // Match the results match_suggestions(expected_paths, suggestions); // Test completion for all files under directories whose names begin with "pa" - let file_str = file(dir.join("partial_a").join("have_ext.")); + let file_str = file(dir.join("partial-a").join("have_ext.")); let file_dir = format!("rm {file_str}"); let suggestions = completer.complete(&file_dir, file_dir.len()); // Create the expected values let expected_paths: Vec = vec![ - file(dir.join("partial_a").join("have_ext.exe")), - file(dir.join("partial_a").join("have_ext.txt")), + file(dir.join("partial-a").join("have_ext.exe")), + file(dir.join("partial-a").join("have_ext.txt")), ]; // Match the results @@ -652,6 +681,27 @@ fn command_watch_with_filecompletion() { match_suggestions(expected_paths, suggestions) } +#[rstest] +fn subcommand_completions(mut subcommand_completer: NuCompleter) { + let prefix = "foo br"; + let suggestions = subcommand_completer.complete(prefix, prefix.len()); + match_suggestions( + vec!["foo bar".to_string(), "foo aabrr".to_string()], + suggestions, + ); + + let prefix = "foo b"; + let suggestions = subcommand_completer.complete(prefix, prefix.len()); + match_suggestions( + vec![ + "foo bar".to_string(), + "foo abaz".to_string(), + "foo aabrr".to_string(), + ], + suggestions, + ); +} + #[test] fn file_completion_quoted() { let (_, _, engine, stack) = new_quote_engine(); @@ -662,11 +712,11 @@ fn file_completion_quoted() { let suggestions = completer.complete(target_dir, target_dir.len()); let expected_paths: Vec = vec![ - "\'[a] bc.txt\'".to_string(), "`--help`".to_string(), "`-42`".to_string(), "`-inf`".to_string(), "`4.2`".to_string(), + "\'[a] bc.txt\'".to_string(), "`te st.txt`".to_string(), "`te#st.txt`".to_string(), "`te'st.txt`".to_string(), diff --git a/crates/nu-cli/tests/completions/support/completions_helpers.rs b/crates/nu-cli/tests/completions/support/completions_helpers.rs index 47f46ab00e..027ac9d997 100644 --- a/crates/nu-cli/tests/completions/support/completions_helpers.rs +++ b/crates/nu-cli/tests/completions/support/completions_helpers.rs @@ -68,6 +68,52 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) { (dir, dir_str, engine_state, stack) } +// creates a new engine with the current path into the completions fixtures folder +pub fn new_dotnu_engine() -> (PathBuf, String, EngineState, Stack) { + // Target folder inside assets + let dir = fs::fixtures().join("dotnu_completions"); + let dir_str = dir + .clone() + .into_os_string() + .into_string() + .unwrap_or_default(); + let dir_span = nu_protocol::Span::new(0, dir_str.len()); + + // Create a new engine with default context + let mut engine_state = create_default_context(); + + // Add $nu + engine_state.generate_nu_constant(); + + // New stack + let mut stack = Stack::new(); + + // Add pwd as env var + stack.add_env_var("PWD".to_string(), Value::string(dir_str.clone(), dir_span)); + stack.add_env_var( + "TEST".to_string(), + Value::string("NUSHELL".to_string(), dir_span), + ); + + stack.add_env_var( + "NU_LIB_DIRS".to_string(), + Value::List { + vals: vec![ + Value::string(file(dir.join("lib-dir1")), dir_span), + Value::string(file(dir.join("lib-dir2")), dir_span), + Value::string(file(dir.join("lib-dir3")), dir_span), + ], + internal_span: dir_span, + }, + ); + + // Merge environment into the permanent state + let merge_result = engine_state.merge_env(&mut stack, &dir); + assert!(merge_result.is_ok()); + + (dir, dir_str, engine_state, stack) +} + pub fn new_quote_engine() -> (PathBuf, String, EngineState, Stack) { // Target folder inside assets let dir = fs::fixtures().join("quoted_completions"); @@ -149,9 +195,13 @@ pub fn match_suggestions(expected: Vec, suggestions: Vec) { Expected: {expected:#?}\n" ) } - expected.iter().zip(suggestions).for_each(|it| { - assert_eq!(it.0, &it.1.value); - }); + assert_eq!( + expected, + suggestions + .into_iter() + .map(|it| it.value) + .collect::>() + ); } // append the separator to the converted path diff --git a/tests/fixtures/partial_completions/partial_a/anotherfile b/tests/fixtures/dotnu_completions/dir_module/mod.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_a/anotherfile rename to tests/fixtures/dotnu_completions/dir_module/mod.nu diff --git a/tests/fixtures/partial_completions/partial_a/have_ext.exe b/tests/fixtures/dotnu_completions/foo.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_a/have_ext.exe rename to tests/fixtures/dotnu_completions/foo.nu diff --git a/tests/fixtures/partial_completions/partial_a/have_ext.txt b/tests/fixtures/dotnu_completions/lib-dir1/bar.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_a/have_ext.txt rename to tests/fixtures/dotnu_completions/lib-dir1/bar.nu diff --git a/tests/fixtures/partial_completions/partial_a/hello b/tests/fixtures/dotnu_completions/lib-dir1/baz.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_a/hello rename to tests/fixtures/dotnu_completions/lib-dir1/baz.nu diff --git a/tests/fixtures/partial_completions/partial_a/hola b/tests/fixtures/dotnu_completions/lib-dir1/xyzzy.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_a/hola rename to tests/fixtures/dotnu_completions/lib-dir1/xyzzy.nu diff --git a/tests/fixtures/partial_completions/partial_b/hello_b b/tests/fixtures/dotnu_completions/lib-dir2/asdf.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_b/hello_b rename to tests/fixtures/dotnu_completions/lib-dir2/asdf.nu diff --git a/tests/fixtures/partial_completions/partial_b/hi_b b/tests/fixtures/dotnu_completions/lib-dir2/bat.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_b/hi_b rename to tests/fixtures/dotnu_completions/lib-dir2/bat.nu diff --git a/tests/fixtures/partial_completions/partial_c/hello_c b/tests/fixtures/dotnu_completions/lib-dir3/spam.nu similarity index 100% rename from tests/fixtures/partial_completions/partial_c/hello_c rename to tests/fixtures/dotnu_completions/lib-dir3/spam.nu diff --git a/tests/fixtures/partial_completions/partial-a/anotherfile b/tests/fixtures/partial_completions/partial-a/anotherfile new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial-a/have_ext.exe b/tests/fixtures/partial_completions/partial-a/have_ext.exe new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial-a/have_ext.txt b/tests/fixtures/partial_completions/partial-a/have_ext.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial-a/hello b/tests/fixtures/partial_completions/partial-a/hello new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial-a/hola b/tests/fixtures/partial_completions/partial-a/hola new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial-b/hello_b b/tests/fixtures/partial_completions/partial-b/hello_b new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial-b/hi_b b/tests/fixtures/partial_completions/partial-b/hi_b new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial-c/hello_c b/tests/fixtures/partial_completions/partial-c/hello_c new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/partial_completions/partial/hello.txt b/tests/fixtures/partial_completions/partial/hello.txt new file mode 100644 index 0000000000..e69de29bb2