Add completions.sort option (#13311)

This commit is contained in:
Yash Thakur 2024-08-05 20:30:10 -04:00 committed by GitHub
parent 2f44801414
commit 6d36941e55
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 139 additions and 85 deletions

View file

@ -1,5 +1,5 @@
use crate::{ use crate::{
completions::{Completer, CompletionOptions, MatchAlgorithm, SortBy}, completions::{Completer, CompletionOptions, MatchAlgorithm},
SuggestionKind, SuggestionKind,
}; };
use nu_parser::FlatShape; use nu_parser::FlatShape;
@ -193,11 +193,7 @@ impl Completer for CommandCompletion {
}; };
if !subcommands.is_empty() { if !subcommands.is_empty() {
return sort_suggestions( return sort_suggestions(&String::from_utf8_lossy(&prefix), subcommands, options);
&String::from_utf8_lossy(&prefix),
subcommands,
SortBy::LevenshteinDistance,
);
} }
let config = working_set.get_config(); let config = working_set.get_config();
@ -222,11 +218,7 @@ impl Completer for CommandCompletion {
vec![] vec![]
}; };
sort_suggestions( sort_suggestions(&String::from_utf8_lossy(&prefix), commands, options)
&String::from_utf8_lossy(&prefix),
commands,
SortBy::LevenshteinDistance,
)
} }
} }

View file

@ -48,6 +48,7 @@ impl NuCompleter {
let options = CompletionOptions { let options = CompletionOptions {
case_sensitive: config.case_sensitive_completions, case_sensitive: config.case_sensitive_completions,
match_algorithm: config.completion_algorithm.into(), match_algorithm: config.completion_algorithm.into(),
sort: config.completion_sort,
..Default::default() ..Default::default()
}; };

View file

@ -2,17 +2,18 @@ use crate::{
completions::{matches, CompletionOptions}, completions::{matches, CompletionOptions},
SemanticSuggestion, SemanticSuggestion,
}; };
use fuzzy_matcher::{skim::SkimMatcherV2, FuzzyMatcher};
use nu_ansi_term::Style; use nu_ansi_term::Style;
use nu_engine::env_to_string; use nu_engine::env_to_string;
use nu_path::{expand_to_real_path, home_dir}; use nu_path::{expand_to_real_path, home_dir};
use nu_protocol::{ use nu_protocol::{
engine::{EngineState, Stack, StateWorkingSet}, engine::{EngineState, Stack, StateWorkingSet},
levenshtein_distance, Span, CompletionSort, Span,
}; };
use nu_utils::get_ls_colors; use nu_utils::get_ls_colors;
use std::path::{is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP}; use std::path::{is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP};
use super::{MatchAlgorithm, SortBy}; use super::MatchAlgorithm;
#[derive(Clone, Default)] #[derive(Clone, Default)]
pub struct PathBuiltFromString { pub struct PathBuiltFromString {
@ -71,7 +72,7 @@ pub fn complete_rec(
} }
let prefix = partial.first().unwrap_or(&""); 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 { for (entry_name, built) in sorted_entries {
match partial.split_first() { match partial.split_first() {
@ -305,33 +306,37 @@ pub fn adjust_if_intermediate(
pub fn sort_suggestions( pub fn sort_suggestions(
prefix: &str, prefix: &str,
items: Vec<SemanticSuggestion>, items: Vec<SemanticSuggestion>,
sort_by: SortBy, options: &CompletionOptions,
) -> Vec<SemanticSuggestion> { ) -> Vec<SemanticSuggestion> {
sort_completions(prefix, items, sort_by, |it| &it.suggestion.value) sort_completions(prefix, items, options, |it| &it.suggestion.value)
} }
/// # Arguments /// # 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<T>( pub fn sort_completions<T>(
prefix: &str, prefix: &str,
mut items: Vec<T>, mut items: Vec<T>,
sort_by: SortBy, options: &CompletionOptions,
get_value: fn(&T) -> &str, get_value: fn(&T) -> &str,
) -> Vec<T> { ) -> Vec<T> {
// Sort items // Sort items
match sort_by { if options.sort == CompletionSort::Smart && options.match_algorithm == MatchAlgorithm::Fuzzy {
SortBy::LevenshteinDistance => { let mut matcher = SkimMatcherV2::default();
if options.case_sensitive {
matcher = matcher.respect_case();
} else {
matcher = matcher.ignore_case();
};
items.sort_by(|a, b| { items.sort_by(|a, b| {
let a_distance = levenshtein_distance(prefix, get_value(a)); let a_str = get_value(a);
let b_distance = levenshtein_distance(prefix, get_value(b)); let b_str = get_value(b);
a_distance.cmp(&b_distance) 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 {
SortBy::Ascending => {
items.sort_by(|a, b| get_value(a).cmp(get_value(b))); items.sort_by(|a, b| get_value(a).cmp(get_value(b)));
} }
SortBy::None => {}
};
items items
} }

View file

@ -1,17 +1,10 @@
use fuzzy_matcher::{skim::SkimMatcherV2, FuzzyMatcher}; use fuzzy_matcher::{skim::SkimMatcherV2, FuzzyMatcher};
use nu_parser::trim_quotes_str; use nu_parser::trim_quotes_str;
use nu_protocol::CompletionAlgorithm; use nu_protocol::{CompletionAlgorithm, CompletionSort};
use std::fmt::Display; use std::fmt::Display;
#[derive(Copy, Clone)]
pub enum SortBy {
LevenshteinDistance,
Ascending,
None,
}
/// Describes how suggestions should be matched. /// Describes how suggestions should be matched.
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug, PartialEq)]
pub enum MatchAlgorithm { pub enum MatchAlgorithm {
/// Only show suggestions which begin with the given input /// Only show suggestions which begin with the given input
/// ///
@ -96,6 +89,7 @@ pub struct CompletionOptions {
pub case_sensitive: bool, pub case_sensitive: bool,
pub positional: bool, pub positional: bool,
pub match_algorithm: MatchAlgorithm, pub match_algorithm: MatchAlgorithm,
pub sort: CompletionSort,
} }
impl Default for CompletionOptions { impl Default for CompletionOptions {
@ -104,6 +98,7 @@ impl Default for CompletionOptions {
case_sensitive: true, case_sensitive: true,
positional: true, positional: true,
match_algorithm: MatchAlgorithm::Prefix, match_algorithm: MatchAlgorithm::Prefix,
sort: Default::default(),
} }
} }
} }

View file

@ -1,13 +1,13 @@
use crate::completions::{ use crate::completions::{
completer::map_value_completions, Completer, CompletionOptions, MatchAlgorithm, completer::map_value_completions, Completer, CompletionOptions, MatchAlgorithm,
SemanticSuggestion, SortBy, SemanticSuggestion,
}; };
use nu_engine::eval_call; use nu_engine::eval_call;
use nu_protocol::{ use nu_protocol::{
ast::{Argument, Call, Expr, Expression}, ast::{Argument, Call, Expr, Expression},
debugger::WithoutDebug, debugger::WithoutDebug,
engine::{Stack, StateWorkingSet}, engine::{Stack, StateWorkingSet},
PipelineData, Span, Type, Value, CompletionSort, PipelineData, Span, Type, Value,
}; };
use nu_utils::IgnoreCaseExt; use nu_utils::IgnoreCaseExt;
use std::collections::HashMap; use std::collections::HashMap;
@ -18,7 +18,6 @@ pub struct CustomCompletion {
stack: Stack, stack: Stack,
decl_id: usize, decl_id: usize,
line: String, line: String,
sort_by: SortBy,
} }
impl CustomCompletion { impl CustomCompletion {
@ -27,7 +26,6 @@ impl CustomCompletion {
stack, stack,
decl_id, decl_id,
line, line,
sort_by: SortBy::None,
} }
} }
} }
@ -93,10 +91,6 @@ impl Completer for CustomCompletion {
.and_then(|val| val.as_bool().ok()) .and_then(|val| val.as_bool().ok())
.unwrap_or(false); .unwrap_or(false);
if should_sort {
self.sort_by = SortBy::Ascending;
}
custom_completion_options = Some(CompletionOptions { custom_completion_options = Some(CompletionOptions {
case_sensitive: options case_sensitive: options
.get("case_sensitive") .get("case_sensitive")
@ -114,6 +108,11 @@ impl Completer for CustomCompletion {
.unwrap_or(MatchAlgorithm::Prefix), .unwrap_or(MatchAlgorithm::Prefix),
None => completion_options.match_algorithm, 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(); .unwrap_or_default();
let suggestions = if let Some(custom_completion_options) = custom_completion_options { let options = custom_completion_options
filter(&prefix, suggestions, &custom_completion_options) .as_ref()
} else { .unwrap_or(completion_options);
filter(&prefix, suggestions, completion_options) let suggestions = filter(&prefix, suggestions, completion_options);
}; sort_suggestions(&String::from_utf8_lossy(&prefix), suggestions, options)
sort_suggestions(&String::from_utf8_lossy(&prefix), suggestions, self.sort_by)
} }
} }

View file

@ -6,7 +6,7 @@ use nu_protocol::{
use reedline::Suggestion; use reedline::Suggestion;
use std::path::{is_separator, Path, MAIN_SEPARATOR as SEP, MAIN_SEPARATOR_STR}; 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)] #[derive(Clone, Default)]
pub struct DotNuCompletion {} pub struct DotNuCompletion {}
@ -130,6 +130,6 @@ impl Completer for DotNuCompletion {
}) })
.collect(); .collect();
sort_suggestions(&prefix_str, output, SortBy::Ascending) sort_suggestions(&prefix_str, output, options)
} }
} }

View file

@ -1,6 +1,4 @@
use crate::completions::{ use crate::completions::{completion_common::sort_suggestions, Completer, CompletionOptions};
completion_common::sort_suggestions, Completer, CompletionOptions, SortBy,
};
use nu_protocol::{ use nu_protocol::{
ast::{Expr, Expression}, ast::{Expr, Expression},
engine::{Stack, StateWorkingSet}, 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![] vec![]

View file

@ -13,7 +13,7 @@ mod variable_completions;
pub use base::{Completer, SemanticSuggestion, SuggestionKind}; pub use base::{Completer, SemanticSuggestion, SuggestionKind};
pub use command_completions::CommandCompletion; pub use command_completions::CommandCompletion;
pub use completer::NuCompleter; pub use completer::NuCompleter;
pub use completion_options::{CompletionOptions, MatchAlgorithm, SortBy}; pub use completion_options::{CompletionOptions, MatchAlgorithm};
pub use custom_completions::CustomCompletion; pub use custom_completions::CustomCompletion;
pub use directory_completions::DirectoryCompletion; pub use directory_completions::DirectoryCompletion;
pub use dotnu_completions::DotNuCompletion; pub use dotnu_completions::DotNuCompletion;

View file

@ -9,7 +9,7 @@ use nu_protocol::{
use reedline::Suggestion; use reedline::Suggestion;
use std::str; use std::str;
use super::{completion_common::sort_suggestions, SortBy}; use super::completion_common::sort_suggestions;
#[derive(Clone)] #[derive(Clone)]
pub struct VariableCompletion { 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 { } else {
// No nesting provided, return all env vars // 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? output.dedup(); // TODO: Removes only consecutive duplicates, is it intended?

View file

@ -89,14 +89,12 @@ fn subcommand_completer() -> NuCompleter {
// Create a new engine // Create a new engine
let (dir, _, mut engine, mut stack) = 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#" let commands = r#"
$env.config.completions.algorithm = "fuzzy" $env.config.completions.algorithm = "fuzzy"
def foo [] {} def foo [] {}
def "foo bar" [] {} def "foo bar" [] {}
def "foo abaz" [] {} def "foo abaz" [] {}
def "foo aabrr" [] {} def "foo aabcrr" [] {}
def food [] {} def food [] {}
"#; "#;
assert!(support::merge_input(commands.as_bytes(), &mut engine, &mut stack, dir).is_ok()); 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)) 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] #[test]
fn variables_dollar_sign_with_variablecompletion() { fn variables_dollar_sign_with_variablecompletion() {
let (_, _, engine, stack) = new_engine(); let (_, _, engine, stack) = new_engine();
@ -774,7 +788,7 @@ fn subcommand_completions(mut subcommand_completer: NuCompleter) {
let prefix = "foo br"; let prefix = "foo br";
let suggestions = subcommand_completer.complete(prefix, prefix.len()); let suggestions = subcommand_completer.complete(prefix, prefix.len());
match_suggestions( match_suggestions(
&vec!["foo bar".to_string(), "foo aabrr".to_string()], &vec!["foo bar".to_string(), "foo aabcrr".to_string()],
&suggestions, &suggestions,
); );
@ -783,8 +797,8 @@ fn subcommand_completions(mut subcommand_completer: NuCompleter) {
match_suggestions( match_suggestions(
&vec![ &vec![
"foo bar".to_string(), "foo bar".to_string(),
"foo aabcrr".to_string(),
"foo abaz".to_string(), "foo abaz".to_string(),
"foo aabrr".to_string(),
], ],
&suggestions, &suggestions,
); );
@ -1270,6 +1284,17 @@ fn custom_completer_triggers_cursor_after_word(mut custom_completer: NuCompleter
match_suggestions(&expected, &suggestions); 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"] #[ignore = "was reverted, still needs fixing"]
#[rstest] #[rstest]
fn alias_offset_bug_7648() { fn alias_offset_bug_7648() {

View file

@ -1196,16 +1196,16 @@ mod tests {
actual: result, actual: result,
expected: serde_json::json!([ expected: serde_json::json!([
{ {
"label": "def", "label": "overlay",
"textEdit": { "textEdit": {
"newText": "def", "newText": "overlay",
"range": { "range": {
"start": { "character": 0, "line": 0 }, "start": { "character": 0, "line": 0 },
"end": { "character": 2, "line": 0 } "end": { "character": 2, "line": 0 }
} }
}, },
"kind": 14 "kind": 14
} },
]) ])
); );
} }

View file

@ -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<Self, Self::Err> {
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 { pub(super) fn reconstruct_external_completer(config: &Config, span: Span) -> Value {
if let Some(closure) = config.external_completer.as_ref() { if let Some(closure) = config.external_completer.as_ref() {
Value::closure(closure.clone(), span) Value::closure(closure.clone(), span)

View file

@ -11,7 +11,7 @@ use crate::{record, ShellError, Span, Value};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::collections::HashMap; use std::collections::HashMap;
pub use self::completer::CompletionAlgorithm; pub use self::completer::{CompletionAlgorithm, CompletionSort};
pub use self::helper::extract_value; pub use self::helper::extract_value;
pub use self::hooks::Hooks; pub use self::hooks::Hooks;
pub use self::output::ErrorStyle; pub use self::output::ErrorStyle;
@ -69,6 +69,7 @@ pub struct Config {
pub quick_completions: bool, pub quick_completions: bool,
pub partial_completions: bool, pub partial_completions: bool,
pub completion_algorithm: CompletionAlgorithm, pub completion_algorithm: CompletionAlgorithm,
pub completion_sort: CompletionSort,
pub edit_mode: EditBindings, pub edit_mode: EditBindings,
pub history: HistoryConfig, pub history: HistoryConfig,
pub keybindings: Vec<ParsedKeybinding>, pub keybindings: Vec<ParsedKeybinding>,
@ -141,6 +142,7 @@ impl Default for Config {
quick_completions: true, quick_completions: true,
partial_completions: true, partial_completions: true,
completion_algorithm: CompletionAlgorithm::default(), completion_algorithm: CompletionAlgorithm::default(),
completion_sort: CompletionSort::default(),
enable_external_completion: true, enable_external_completion: true,
max_external_completion_results: 100, max_external_completion_results: 100,
recursion_limit: 50, recursion_limit: 50,
@ -341,6 +343,13 @@ impl Value {
"case_sensitive" => { "case_sensitive" => {
process_bool_config(value, &mut errors, &mut config.case_sensitive_completions); 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" => { "external" => {
if let Value::Record { val, .. } = value { if let Value::Record { val, .. } = value {
val.to_mut().retain_mut(|key3, value| val.to_mut().retain_mut(|key3, value|
@ -401,6 +410,7 @@ impl Value {
"partial" => Value::bool(config.partial_completions, span), "partial" => Value::bool(config.partial_completions, span),
"algorithm" => config.completion_algorithm.reconstruct_value(span), "algorithm" => config.completion_algorithm.reconstruct_value(span),
"case_sensitive" => Value::bool(config.case_sensitive_completions, span), "case_sensitive" => Value::bool(config.case_sensitive_completions, span),
"sort" => config.completion_sort.reconstruct_value(span),
"external" => reconstruct_external(&config, span), "external" => reconstruct_external(&config, span),
"use_ls_colors" => Value::bool(config.use_ls_colors_completions, span), "use_ls_colors" => Value::bool(config.use_ls_colors_completions, span),
}, },

View file

@ -206,6 +206,7 @@ $env.config = {
quick: true # set this to false to prevent auto-selecting completions when only one remains 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 partial: true # set this to false to prevent partial filling of the prompt
algorithm: "prefix" # prefix or fuzzy algorithm: "prefix" # prefix or fuzzy
sort: "smart" # "smart" (alphabetical for prefix matching, fuzzy score for fuzzy matching) or "alphabetical"
external: { 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 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 max_results: 100 # setting it lower can improve completion performance at the cost of omitting some options

View file

@ -1 +1 @@
de ov