From 6d36941e5525898a8281a5503dc02160e88b8eee Mon Sep 17 00:00:00 2001 From: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Date: Mon, 5 Aug 2024 20:30:10 -0400 Subject: [PATCH] Add completions.sort option (#13311) --- .../src/completions/command_completions.rs | 14 ++---- crates/nu-cli/src/completions/completer.rs | 1 + .../src/completions/completion_common.rs | 45 ++++++++++--------- .../src/completions/completion_options.rs | 13 ++---- .../src/completions/custom_completions.rs | 26 +++++------ .../src/completions/dotnu_completions.rs | 4 +- .../src/completions/flag_completions.rs | 6 +-- crates/nu-cli/src/completions/mod.rs | 2 +- .../src/completions/variable_completions.rs | 12 ++--- crates/nu-cli/tests/completions/mod.rs | 35 ++++++++++++--- crates/nu-lsp/src/lib.rs | 22 ++++----- crates/nu-protocol/src/config/completer.rs | 29 ++++++++++++ crates/nu-protocol/src/config/mod.rs | 12 ++++- .../src/sample_config/default_config.nu | 1 + tests/fixtures/lsp/completion/keyword.nu | 2 +- 15 files changed, 139 insertions(+), 85 deletions(-) diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs index f594e3d667..f8107f8ed7 100644 --- a/crates/nu-cli/src/completions/command_completions.rs +++ b/crates/nu-cli/src/completions/command_completions.rs @@ -1,5 +1,5 @@ use crate::{ - completions::{Completer, CompletionOptions, MatchAlgorithm, SortBy}, + completions::{Completer, CompletionOptions, MatchAlgorithm}, SuggestionKind, }; use nu_parser::FlatShape; @@ -193,11 +193,7 @@ impl Completer for CommandCompletion { }; if !subcommands.is_empty() { - return sort_suggestions( - &String::from_utf8_lossy(&prefix), - subcommands, - SortBy::LevenshteinDistance, - ); + return sort_suggestions(&String::from_utf8_lossy(&prefix), subcommands, options); } let config = working_set.get_config(); @@ -222,11 +218,7 @@ impl Completer for CommandCompletion { vec![] }; - sort_suggestions( - &String::from_utf8_lossy(&prefix), - commands, - SortBy::LevenshteinDistance, - ) + sort_suggestions(&String::from_utf8_lossy(&prefix), commands, options) } } diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index e794354f09..91fc1132a5 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -48,6 +48,7 @@ impl NuCompleter { let options = CompletionOptions { case_sensitive: config.case_sensitive_completions, match_algorithm: config.completion_algorithm.into(), + sort: config.completion_sort, ..Default::default() }; diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs index 2b85f6091e..a5d07be57e 100644 --- a/crates/nu-cli/src/completions/completion_common.rs +++ b/crates/nu-cli/src/completions/completion_common.rs @@ -2,17 +2,18 @@ use crate::{ completions::{matches, CompletionOptions}, SemanticSuggestion, }; +use fuzzy_matcher::{skim::SkimMatcherV2, FuzzyMatcher}; 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}, - levenshtein_distance, Span, + CompletionSort, Span, }; use nu_utils::get_ls_colors; use std::path::{is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP}; -use super::{MatchAlgorithm, SortBy}; +use super::MatchAlgorithm; #[derive(Clone, Default)] pub struct PathBuiltFromString { @@ -71,7 +72,7 @@ pub fn complete_rec( } let prefix = partial.first().unwrap_or(&""); - let sorted_entries = sort_completions(prefix, entries, SortBy::Ascending, |(entry, _)| entry); + let sorted_entries = sort_completions(prefix, entries, options, |(entry, _)| entry); for (entry_name, built) in sorted_entries { match partial.split_first() { @@ -305,33 +306,37 @@ pub fn adjust_if_intermediate( pub fn sort_suggestions( prefix: &str, items: Vec, - sort_by: SortBy, + options: &CompletionOptions, ) -> Vec { - sort_completions(prefix, items, sort_by, |it| &it.suggestion.value) + sort_completions(prefix, items, options, |it| &it.suggestion.value) } /// # Arguments -/// * `prefix` - What the user's typed, for sorting by Levenshtein distance +/// * `prefix` - What the user's typed, for sorting by fuzzy matcher score pub fn sort_completions( prefix: &str, mut items: Vec, - sort_by: SortBy, + options: &CompletionOptions, 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 => {} - }; + if options.sort == CompletionSort::Smart && options.match_algorithm == MatchAlgorithm::Fuzzy { + let mut matcher = SkimMatcherV2::default(); + if options.case_sensitive { + matcher = matcher.respect_case(); + } else { + matcher = matcher.ignore_case(); + }; + items.sort_by(|a, b| { + let a_str = get_value(a); + let b_str = get_value(b); + let a_score = matcher.fuzzy_match(a_str, prefix).unwrap_or_default(); + let b_score = matcher.fuzzy_match(b_str, prefix).unwrap_or_default(); + b_score.cmp(&a_score).then(a_str.cmp(b_str)) + }); + } else { + items.sort_by(|a, b| get_value(a).cmp(get_value(b))); + } items } diff --git a/crates/nu-cli/src/completions/completion_options.rs b/crates/nu-cli/src/completions/completion_options.rs index a414aafedf..880aa54fb1 100644 --- a/crates/nu-cli/src/completions/completion_options.rs +++ b/crates/nu-cli/src/completions/completion_options.rs @@ -1,17 +1,10 @@ use fuzzy_matcher::{skim::SkimMatcherV2, FuzzyMatcher}; use nu_parser::trim_quotes_str; -use nu_protocol::CompletionAlgorithm; +use nu_protocol::{CompletionAlgorithm, CompletionSort}; use std::fmt::Display; -#[derive(Copy, Clone)] -pub enum SortBy { - LevenshteinDistance, - Ascending, - None, -} - /// Describes how suggestions should be matched. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum MatchAlgorithm { /// Only show suggestions which begin with the given input /// @@ -96,6 +89,7 @@ pub struct CompletionOptions { pub case_sensitive: bool, pub positional: bool, pub match_algorithm: MatchAlgorithm, + pub sort: CompletionSort, } impl Default for CompletionOptions { @@ -104,6 +98,7 @@ impl Default for CompletionOptions { case_sensitive: true, positional: true, match_algorithm: MatchAlgorithm::Prefix, + sort: Default::default(), } } } diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs index 0d2c674ef9..f21f942501 100644 --- a/crates/nu-cli/src/completions/custom_completions.rs +++ b/crates/nu-cli/src/completions/custom_completions.rs @@ -1,13 +1,13 @@ use crate::completions::{ completer::map_value_completions, Completer, CompletionOptions, MatchAlgorithm, - SemanticSuggestion, SortBy, + SemanticSuggestion, }; use nu_engine::eval_call; use nu_protocol::{ ast::{Argument, Call, Expr, Expression}, debugger::WithoutDebug, engine::{Stack, StateWorkingSet}, - PipelineData, Span, Type, Value, + CompletionSort, PipelineData, Span, Type, Value, }; use nu_utils::IgnoreCaseExt; use std::collections::HashMap; @@ -18,7 +18,6 @@ pub struct CustomCompletion { stack: Stack, decl_id: usize, line: String, - sort_by: SortBy, } impl CustomCompletion { @@ -27,7 +26,6 @@ impl CustomCompletion { stack, decl_id, line, - sort_by: SortBy::None, } } } @@ -93,10 +91,6 @@ impl Completer for CustomCompletion { .and_then(|val| val.as_bool().ok()) .unwrap_or(false); - if should_sort { - self.sort_by = SortBy::Ascending; - } - custom_completion_options = Some(CompletionOptions { case_sensitive: options .get("case_sensitive") @@ -114,6 +108,11 @@ impl Completer for CustomCompletion { .unwrap_or(MatchAlgorithm::Prefix), None => completion_options.match_algorithm, }, + sort: if should_sort { + CompletionSort::Alphabetical + } else { + CompletionSort::Smart + }, }); } @@ -124,12 +123,11 @@ impl Completer for CustomCompletion { }) .unwrap_or_default(); - let suggestions = if let Some(custom_completion_options) = custom_completion_options { - filter(&prefix, suggestions, &custom_completion_options) - } else { - filter(&prefix, suggestions, completion_options) - }; - sort_suggestions(&String::from_utf8_lossy(&prefix), suggestions, self.sort_by) + let options = custom_completion_options + .as_ref() + .unwrap_or(completion_options); + let suggestions = filter(&prefix, suggestions, completion_options); + sort_suggestions(&String::from_utf8_lossy(&prefix), suggestions, options) } } diff --git a/crates/nu-cli/src/completions/dotnu_completions.rs b/crates/nu-cli/src/completions/dotnu_completions.rs index 6b4be16769..4a241a57b9 100644 --- a/crates/nu-cli/src/completions/dotnu_completions.rs +++ b/crates/nu-cli/src/completions/dotnu_completions.rs @@ -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::{completion_common::sort_suggestions, SemanticSuggestion, SortBy}; +use super::{completion_common::sort_suggestions, SemanticSuggestion}; #[derive(Clone, Default)] pub struct DotNuCompletion {} @@ -130,6 +130,6 @@ impl Completer for DotNuCompletion { }) .collect(); - sort_suggestions(&prefix_str, output, SortBy::Ascending) + sort_suggestions(&prefix_str, output, options) } } diff --git a/crates/nu-cli/src/completions/flag_completions.rs b/crates/nu-cli/src/completions/flag_completions.rs index c4ccfd6048..590929d5a0 100644 --- a/crates/nu-cli/src/completions/flag_completions.rs +++ b/crates/nu-cli/src/completions/flag_completions.rs @@ -1,6 +1,4 @@ -use crate::completions::{ - completion_common::sort_suggestions, Completer, CompletionOptions, SortBy, -}; +use crate::completions::{completion_common::sort_suggestions, Completer, CompletionOptions}; use nu_protocol::{ ast::{Expr, Expression}, engine::{Stack, StateWorkingSet}, @@ -90,7 +88,7 @@ impl Completer for FlagCompletion { } } - return sort_suggestions(&String::from_utf8_lossy(&prefix), output, SortBy::Ascending); + return sort_suggestions(&String::from_utf8_lossy(&prefix), output, options); } vec![] diff --git a/crates/nu-cli/src/completions/mod.rs b/crates/nu-cli/src/completions/mod.rs index 20a61ee6d9..ed1510cdfb 100644 --- a/crates/nu-cli/src/completions/mod.rs +++ b/crates/nu-cli/src/completions/mod.rs @@ -13,7 +13,7 @@ mod variable_completions; pub use base::{Completer, SemanticSuggestion, SuggestionKind}; pub use command_completions::CommandCompletion; pub use completer::NuCompleter; -pub use completion_options::{CompletionOptions, MatchAlgorithm, SortBy}; +pub use completion_options::{CompletionOptions, MatchAlgorithm}; pub use custom_completions::CustomCompletion; pub use directory_completions::DirectoryCompletion; pub use dotnu_completions::DotNuCompletion; diff --git a/crates/nu-cli/src/completions/variable_completions.rs b/crates/nu-cli/src/completions/variable_completions.rs index 418dcec064..5e8f117810 100644 --- a/crates/nu-cli/src/completions/variable_completions.rs +++ b/crates/nu-cli/src/completions/variable_completions.rs @@ -9,7 +9,7 @@ use nu_protocol::{ use reedline::Suggestion; use std::str; -use super::{completion_common::sort_suggestions, SortBy}; +use super::completion_common::sort_suggestions; #[derive(Clone)] pub struct VariableCompletion { @@ -72,7 +72,7 @@ impl Completer for VariableCompletion { } } - return sort_suggestions(&prefix_str, output, SortBy::Ascending); + return sort_suggestions(&prefix_str, output, options); } } else { // No nesting provided, return all env vars @@ -93,7 +93,7 @@ impl Completer for VariableCompletion { } } - return sort_suggestions(&prefix_str, output, SortBy::Ascending); + return sort_suggestions(&prefix_str, output, options); } } @@ -117,7 +117,7 @@ impl Completer for VariableCompletion { } } - return sort_suggestions(&prefix_str, output, SortBy::Ascending); + return sort_suggestions(&prefix_str, output, options); } } @@ -139,7 +139,7 @@ impl Completer for VariableCompletion { } } - return sort_suggestions(&prefix_str, output, SortBy::Ascending); + return sort_suggestions(&prefix_str, output, options); } } } @@ -217,7 +217,7 @@ impl Completer for VariableCompletion { } } - output = sort_suggestions(&prefix_str, output, SortBy::Ascending); + output = sort_suggestions(&prefix_str, output, options); output.dedup(); // TODO: Removes only consecutive duplicates, is it intended? diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs index ad5dcaa70e..e1714f78e2 100644 --- a/crates/nu-cli/tests/completions/mod.rs +++ b/crates/nu-cli/tests/completions/mod.rs @@ -89,14 +89,12 @@ 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 "foo aabcrr" [] {} def food [] {} "#; assert!(support::merge_input(commands.as_bytes(), &mut engine, &mut stack, dir).is_ok()); @@ -105,6 +103,22 @@ fn subcommand_completer() -> NuCompleter { NuCompleter::new(Arc::new(engine), Arc::new(stack)) } +/// Use fuzzy completions but sort in alphabetical order +#[fixture] +fn fuzzy_alpha_sort_completer() -> NuCompleter { + // Create a new engine + let (dir, _, mut engine, mut stack) = new_engine(); + + let config = r#" + $env.config.completions.algorithm = "fuzzy" + $env.config.completions.sort = "alphabetical" + "#; + assert!(support::merge_input(config.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_variablecompletion() { let (_, _, engine, stack) = new_engine(); @@ -774,7 +788,7 @@ 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()], + &vec!["foo bar".to_string(), "foo aabcrr".to_string()], &suggestions, ); @@ -783,8 +797,8 @@ fn subcommand_completions(mut subcommand_completer: NuCompleter) { match_suggestions( &vec![ "foo bar".to_string(), + "foo aabcrr".to_string(), "foo abaz".to_string(), - "foo aabrr".to_string(), ], &suggestions, ); @@ -1270,6 +1284,17 @@ fn custom_completer_triggers_cursor_after_word(mut custom_completer: NuCompleter match_suggestions(&expected, &suggestions); } +#[rstest] +fn sort_fuzzy_completions_in_alphabetical_order(mut fuzzy_alpha_sort_completer: NuCompleter) { + let suggestions = fuzzy_alpha_sort_completer.complete("ls nu", 5); + // Even though "nushell" is a better match, it should come second because + // the completions should be sorted in alphabetical order + match_suggestions( + &vec!["custom_completion.nu".into(), "nushell".into()], + &suggestions, + ); +} + #[ignore = "was reverted, still needs fixing"] #[rstest] fn alias_offset_bug_7648() { diff --git a/crates/nu-lsp/src/lib.rs b/crates/nu-lsp/src/lib.rs index aee2ab240d..6841749fc9 100644 --- a/crates/nu-lsp/src/lib.rs +++ b/crates/nu-lsp/src/lib.rs @@ -1195,17 +1195,17 @@ mod tests { assert_json_include!( actual: result, expected: serde_json::json!([ - { - "label": "def", - "textEdit": { - "newText": "def", - "range": { - "start": { "character": 0, "line": 0 }, - "end": { "character": 2, "line": 0 } - } - }, - "kind": 14 - } + { + "label": "overlay", + "textEdit": { + "newText": "overlay", + "range": { + "start": { "character": 0, "line": 0 }, + "end": { "character": 2, "line": 0 } + } + }, + "kind": 14 + }, ]) ); } diff --git a/crates/nu-protocol/src/config/completer.rs b/crates/nu-protocol/src/config/completer.rs index 67bde52e27..921057c082 100644 --- a/crates/nu-protocol/src/config/completer.rs +++ b/crates/nu-protocol/src/config/completer.rs @@ -35,6 +35,35 @@ impl ReconstructVal for CompletionAlgorithm { } } +#[derive(Serialize, Deserialize, Clone, Copy, Debug, Default, PartialEq)] +pub enum CompletionSort { + #[default] + Smart, + Alphabetical, +} + +impl FromStr for CompletionSort { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + match s.to_ascii_lowercase().as_str() { + "smart" => Ok(Self::Smart), + "alphabetical" => Ok(Self::Alphabetical), + _ => Err("expected either 'smart' or 'alphabetical'"), + } + } +} + +impl ReconstructVal for CompletionSort { + fn reconstruct_value(&self, span: Span) -> Value { + let str = match self { + Self::Smart => "smart", + Self::Alphabetical => "alphabetical", + }; + Value::string(str, span) + } +} + pub(super) fn reconstruct_external_completer(config: &Config, span: Span) -> Value { if let Some(closure) = config.external_completer.as_ref() { Value::closure(closure.clone(), span) diff --git a/crates/nu-protocol/src/config/mod.rs b/crates/nu-protocol/src/config/mod.rs index eda3d9a15e..59cc70fb79 100644 --- a/crates/nu-protocol/src/config/mod.rs +++ b/crates/nu-protocol/src/config/mod.rs @@ -11,7 +11,7 @@ use crate::{record, ShellError, Span, Value}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; -pub use self::completer::CompletionAlgorithm; +pub use self::completer::{CompletionAlgorithm, CompletionSort}; pub use self::helper::extract_value; pub use self::hooks::Hooks; pub use self::output::ErrorStyle; @@ -69,6 +69,7 @@ pub struct Config { pub quick_completions: bool, pub partial_completions: bool, pub completion_algorithm: CompletionAlgorithm, + pub completion_sort: CompletionSort, pub edit_mode: EditBindings, pub history: HistoryConfig, pub keybindings: Vec, @@ -141,6 +142,7 @@ impl Default for Config { quick_completions: true, partial_completions: true, completion_algorithm: CompletionAlgorithm::default(), + completion_sort: CompletionSort::default(), enable_external_completion: true, max_external_completion_results: 100, recursion_limit: 50, @@ -341,6 +343,13 @@ impl Value { "case_sensitive" => { process_bool_config(value, &mut errors, &mut config.case_sensitive_completions); } + "sort" => { + process_string_enum( + &mut config.completion_sort, + &[key, key2], + value, + &mut errors); + } "external" => { if let Value::Record { val, .. } = value { val.to_mut().retain_mut(|key3, value| @@ -401,6 +410,7 @@ impl Value { "partial" => Value::bool(config.partial_completions, span), "algorithm" => config.completion_algorithm.reconstruct_value(span), "case_sensitive" => Value::bool(config.case_sensitive_completions, span), + "sort" => config.completion_sort.reconstruct_value(span), "external" => reconstruct_external(&config, span), "use_ls_colors" => Value::bool(config.use_ls_colors_completions, span), }, diff --git a/crates/nu-utils/src/sample_config/default_config.nu b/crates/nu-utils/src/sample_config/default_config.nu index 0c78cf9ca1..b807244cee 100644 --- a/crates/nu-utils/src/sample_config/default_config.nu +++ b/crates/nu-utils/src/sample_config/default_config.nu @@ -206,6 +206,7 @@ $env.config = { quick: true # set this to false to prevent auto-selecting completions when only one remains partial: true # set this to false to prevent partial filling of the prompt algorithm: "prefix" # prefix or fuzzy + sort: "smart" # "smart" (alphabetical for prefix matching, fuzzy score for fuzzy matching) or "alphabetical" external: { enable: true # set to false to prevent nushell looking into $env.PATH to find more suggestions, `false` recommended for WSL users as this look up may be very slow max_results: 100 # setting it lower can improve completion performance at the cost of omitting some options diff --git a/tests/fixtures/lsp/completion/keyword.nu b/tests/fixtures/lsp/completion/keyword.nu index 7673daa944..b4226322e1 100644 --- a/tests/fixtures/lsp/completion/keyword.nu +++ b/tests/fixtures/lsp/completion/keyword.nu @@ -1 +1 @@ -de +ov