Feature/refactor completion options (#5228)

* Copy completion filter to custom completions

* Remove filter function from completer

This function was a no-op for FileCompletion and CommandCompletion.
Flag- and VariableCompletion just filters with `starts_with` which
happens in both completers anyway and should therefore also be a no-op.
The remaining use case in CustomCompletion was moved into the
CustomCompletion source file.

Filtering should probably happen immediately while fetching completions
to avoid unnecessary memory allocations.

* Add get_sort_by() to Completer trait

* Remove CompletionOptions from Completer::fetch()

* Fix clippy lints

* Apply Completer changes to DotNuCompletion

crates/nu-cli/src/completions/base.rs

@@ -1,4 +1,4 @@
-use crate::completions::{CompletionOptions, SortBy};
+use crate::completions::SortBy;
 use nu_protocol::{engine::StateWorkingSet, levenshtein_distance, Span};
 use reedline::Suggestion;
 
@@ -12,50 +12,18 @@ pub trait Completer {
         span: Span,
         offset: usize,
         pos: usize,
-    ) -> (Vec<Suggestion>, CompletionOptions);
+    ) -> Vec<Suggestion>;
 
-    // Filter results using the completion options
+    fn get_sort_by(&self) -> SortBy {
-    fn filter(
+        SortBy::Ascending
-        &self,
-        prefix: Vec<u8>,
-        items: Vec<Suggestion>,
-        options: CompletionOptions,
-    ) -> Vec<Suggestion> {
-        items
-            .into_iter()
-            .filter(|it| {
-                // Minimise clones for new functionality
-                match (options.case_sensitive, options.positional) {
-                    (true, true) => it.value.as_bytes().starts_with(&prefix),
-                    (true, false) => it
-                        .value
-                        .contains(std::str::from_utf8(&prefix).unwrap_or("")),
-                    (false, positional) => {
-                        let value = it.value.to_lowercase();
-                        let prefix = std::str::from_utf8(&prefix).unwrap_or("").to_lowercase();
-                        if positional {
-                            value.starts_with(&prefix)
-                        } else {
-                            value.contains(&prefix)
-                        }
-                    }
-                }
-            })
-            .collect()
     }
 
-    // Sort results using the completion options
+    fn sort(&self, items: Vec<Suggestion>, prefix: Vec<u8>) -> Vec<Suggestion> {
-    fn sort(
-        &self,
-        items: Vec<Suggestion>,
-        prefix: Vec<u8>,
-        options: CompletionOptions,
-    ) -> Vec<Suggestion> {
         let prefix_str = String::from_utf8_lossy(&prefix).to_string();
         let mut filtered_items = items;
 
         // Sort items
-        match options.sort_by {
+        match self.get_sort_by() {
             SortBy::LevenshteinDistance => {
                 filtered_items.sort_by(|a, b| {
                     let a_distance = levenshtein_distance(&prefix_str, &a.value);

crates/nu-cli/src/completions/command_completions.rs

@@ -1,6 +1,4 @@
-use crate::completions::{
+use crate::completions::{file_completions::file_path_completion, Completer, SortBy};
-    file_completions::file_path_completion, Completer, CompletionOptions, SortBy,
-};
 use nu_parser::{trim_quotes, FlatShape};
 use nu_protocol::{
     engine::{EngineState, StateWorkingSet},
@@ -154,7 +152,7 @@ impl Completer for CommandCompletion {
         span: Span,
         offset: usize,
         pos: usize,
-    ) -> (Vec<Suggestion>, CompletionOptions) {
+    ) -> Vec<Suggestion> {
         let last = self
             .flattened
             .iter()
@@ -172,9 +170,6 @@ impl Completer for CommandCompletion {
             })
             .last();
 
-        // Options
-        let options = CompletionOptions::new(true, true, SortBy::LevenshteinDistance);
-
         // The last item here would be the earliest shape that could possible by part of this subcommand
         let subcommands = if let Some(last) = last {
             self.complete_commands(
@@ -191,7 +186,7 @@ impl Completer for CommandCompletion {
         };
 
         if !subcommands.is_empty() {
-            return (subcommands, options);
+            return subcommands;
         }
 
         let commands = if matches!(self.flat_shape, nu_parser::FlatShape::External)
@@ -225,7 +220,8 @@ impl Completer for CommandCompletion {
         };
         // let prefix = working_set.get_span_contents(flat.0);
         let prefix = String::from_utf8_lossy(&prefix).to_string();
-        let output = file_path_completion(span, &prefix, &cwd)
+
+        file_path_completion(span, &prefix, &cwd)
             .into_iter()
             .map(move |x| {
                 if self.flat_idx == 0 {
@@ -262,13 +258,10 @@ impl Completer for CommandCompletion {
             })
             .chain(subcommands.into_iter())
             .chain(commands.into_iter())
-            .collect::<Vec<_>>();
+            .collect::<Vec<_>>()
-
-        (output, options)
     }
 
-    // Replace base filter with no filter once all the results are already based in the current path
+    fn get_sort_by(&self) -> SortBy {
-    fn filter(&self, _: Vec<u8>, items: Vec<Suggestion>, _: CompletionOptions) -> Vec<Suggestion> {
+        SortBy::LevenshteinDistance
-        items
     }
 }

crates/nu-cli/src/completions/completer.rs

@@ -36,14 +36,10 @@ impl NuCompleter {
         pos: usize,
     ) -> Vec<Suggestion> {
         // Fetch
-        let (mut suggestions, options) =
+        let mut suggestions = completer.fetch(working_set, prefix.clone(), new_span, offset, pos);
-            completer.fetch(working_set, prefix.clone(), new_span, offset, pos);
-
-        // Filter
-        suggestions = completer.filter(prefix.clone(), suggestions, options.clone());
 
         // Sort
-        suggestions = completer.sort(suggestions, prefix, options);
+        suggestions = completer.sort(suggestions, prefix);
 
         suggestions
     }

crates/nu-cli/src/completions/completion_options.rs

@@ -1,4 +1,4 @@
-#[derive(Clone)]
+#[derive(Copy, Clone)]
 pub enum SortBy {
     LevenshteinDistance,
     Ascending,
@@ -12,16 +12,6 @@ pub struct CompletionOptions {
     pub sort_by: SortBy,
 }
 
-impl CompletionOptions {
-    pub fn new(case_sensitive: bool, positional: bool, sort_by: SortBy) -> Self {
-        Self {
-            case_sensitive,
-            positional,
-            sort_by,
-        }
-    }
-}
-
 impl Default for CompletionOptions {
     fn default() -> Self {
         Self {

crates/nu-cli/src/completions/custom_completions.rs

@@ -13,6 +13,7 @@ pub struct CustomCompletion {
     stack: Stack,
     decl_id: usize,
     line: String,
+    sort_by: SortBy,
 }
 
 impl CustomCompletion {
@@ -22,6 +23,7 @@ impl CustomCompletion {
             stack,
             decl_id,
             line,
+            sort_by: SortBy::None,
         }
     }
 
@@ -55,11 +57,11 @@ impl Completer for CustomCompletion {
     fn fetch(
         &mut self,
         _: &StateWorkingSet,
-        _: Vec<u8>,
+        prefix: Vec<u8>,
         span: Span,
         offset: usize,
         pos: usize,
-    ) -> (Vec<Suggestion>, CompletionOptions) {
+    ) -> Vec<Suggestion> {
         // Line position
         let line_pos = pos - offset;
 
@@ -113,6 +115,10 @@ impl Completer for CustomCompletion {
                                 .and_then(|val| val.as_bool().ok())
                                 .unwrap_or(false);
 
+                            if should_sort {
+                                self.sort_by = SortBy::Ascending;
+                            }
+
                             CompletionOptions {
                                 case_sensitive: options
                                     .get_data_by_key("case_sensitive")
@@ -144,6 +150,32 @@ impl Completer for CustomCompletion {
             _ => (vec![], CompletionOptions::default()),
         };
 
-        (suggestions, options)
+        filter(&prefix, suggestions, options)
+    }
+
+    fn get_sort_by(&self) -> SortBy {
+        self.sort_by
     }
 }
+
+fn filter(prefix: &[u8], items: Vec<Suggestion>, options: CompletionOptions) -> Vec<Suggestion> {
+    items
+        .into_iter()
+        .filter(|it| {
+            // Minimise clones for new functionality
+            match (options.case_sensitive, options.positional) {
+                (true, true) => it.value.as_bytes().starts_with(prefix),
+                (true, false) => it.value.contains(std::str::from_utf8(prefix).unwrap_or("")),
+                (false, positional) => {
+                    let value = it.value.to_lowercase();
+                    let prefix = std::str::from_utf8(prefix).unwrap_or("").to_lowercase();
+                    if positional {
+                        value.starts_with(&prefix)
+                    } else {
+                        value.contains(&prefix)
+                    }
+                }
+            }
+        })
+        .collect()
+}

crates/nu-cli/src/completions/dotnu_completions.rs

@@ -1,6 +1,4 @@
-use crate::completions::{
+use crate::completions::{file_path_completion, partial_from, Completer, SortBy};
-    file_path_completion, partial_from, Completer, CompletionOptions, SortBy,
-};
 use nu_protocol::{
     engine::{EngineState, StateWorkingSet},
     Span,
@@ -21,11 +19,6 @@ impl DotNuCompletion {
 }
 
 impl Completer for DotNuCompletion {
-    // Replace base filter with no filter once all the results are already filtered
-    fn filter(&self, _: Vec<u8>, items: Vec<Suggestion>, _: CompletionOptions) -> Vec<Suggestion> {
-        items
-    }
-
     fn fetch(
         &mut self,
         _: &StateWorkingSet,
@@ -33,7 +26,7 @@ impl Completer for DotNuCompletion {
         span: Span,
         offset: usize,
         _: usize,
-    ) -> (Vec<Suggestion>, CompletionOptions) {
+    ) -> Vec<Suggestion> {
         let prefix_str = String::from_utf8_lossy(&prefix).to_string();
         let mut search_dirs: Vec<String> = vec![];
         let (base_dir, mut partial) = partial_from(&prefix_str);
@@ -118,9 +111,10 @@ impl Completer for DotNuCompletion {
             })
             .collect();
 
-        // Options
+        output
-        let options = CompletionOptions::new(false, true, SortBy::LevenshteinDistance);
+    }
 
-        (output, options)
+    fn get_sort_by(&self) -> SortBy {
+        SortBy::LevenshteinDistance
     }
 }

crates/nu-cli/src/completions/file_completions.rs

@@ -1,4 +1,4 @@
-use crate::completions::{Completer, CompletionOptions};
+use crate::completions::Completer;
 use nu_protocol::{
     engine::{EngineState, StateWorkingSet},
     levenshtein_distance, Span,
@@ -28,7 +28,7 @@ impl Completer for FileCompletion {
         span: Span,
         offset: usize,
         _: usize,
-    ) -> (Vec<Suggestion>, CompletionOptions) {
+    ) -> Vec<Suggestion> {
         let cwd = if let Some(d) = self.engine_state.env_vars.get("PWD") {
             match d.as_string() {
                 Ok(s) => s,
@@ -51,19 +51,11 @@ impl Completer for FileCompletion {
             })
             .collect();
 
-        // Options
+        output
-        let options = CompletionOptions::default();
-
-        (output, options)
     }
 
     // Sort results prioritizing the non hidden folders
-    fn sort(
+    fn sort(&self, items: Vec<Suggestion>, prefix: Vec<u8>) -> Vec<Suggestion> {
-        &self,
-        items: Vec<Suggestion>,
-        prefix: Vec<u8>,
-        _: CompletionOptions, // Ignore the given options, once it's a custom sorting
-    ) -> Vec<Suggestion> {
         let prefix_str = String::from_utf8_lossy(&prefix).to_string();
 
         // Sort items
@@ -97,11 +89,6 @@ impl Completer for FileCompletion {
 
         non_hidden
     }
-
-    // Replace base filter with no filter once all the results are already based in the current path
-    fn filter(&self, _: Vec<u8>, items: Vec<Suggestion>, _: CompletionOptions) -> Vec<Suggestion> {
-        items
-    }
 }
 
 pub fn partial_from(input: &str) -> (String, String) {

crates/nu-cli/src/completions/flag_completions.rs

@@ -1,4 +1,4 @@
-use crate::completions::{Completer, CompletionOptions};
+use crate::completions::Completer;
 use nu_protocol::{
     ast::{Expr, Expression},
     engine::StateWorkingSet,
@@ -26,7 +26,7 @@ impl Completer for FlagCompletion {
         span: Span,
         offset: usize,
         _: usize,
-    ) -> (Vec<Suggestion>, CompletionOptions) {
+    ) -> Vec<Suggestion> {
         // Check if it's a flag
         if let Expr::Call(call) = &self.expression.expr {
             let decl = working_set.get_decl(call.decl_id);
@@ -73,9 +73,9 @@ impl Completer for FlagCompletion {
                 }
             }
 
-            return (output, CompletionOptions::default());
+            return output;
         }
 
-        (vec![], CompletionOptions::default())
+        vec![]
     }
 }

crates/nu-cli/src/completions/variable_completions.rs

@@ -1,4 +1,4 @@
-use crate::completions::{Completer, CompletionOptions};
+use crate::completions::Completer;
 use nu_protocol::{
     engine::{EngineState, Stack, StateWorkingSet},
     Span, Value,
@@ -35,7 +35,7 @@ impl Completer for VariableCompletion {
         span: Span,
         offset: usize,
         _: usize,
-    ) -> (Vec<Suggestion>, CompletionOptions) {
+    ) -> Vec<Suggestion> {
         let mut output = vec![];
         let builtins = ["$nu", "$in", "$config", "$env", "$nothing"];
         let var_str = std::str::from_utf8(&self.var_context.0)
@@ -52,6 +52,7 @@ impl Completer for VariableCompletion {
             // Completion for $env.<tab>
             if var_str.as_str() == "$env" {
                 for env_var in self.stack.get_env_vars(&self.engine_state) {
+                    if env_var.0.as_bytes().starts_with(&prefix) {
                         output.push(Suggestion {
                             value: env_var.0,
                             description: None,
@@ -59,8 +60,9 @@ impl Completer for VariableCompletion {
                             span: current_span,
                         });
                     }
+                }
 
-                return (output, CompletionOptions::default());
+                return output;
             }
 
             // Completion other variable types
@@ -96,11 +98,11 @@ impl Completer for VariableCompletion {
                                 });
                             }
 
-                            return (output, CompletionOptions::default());
+                            return output;
                         }
 
                         _ => {
-                            return (output, CompletionOptions::default());
+                            return output;
                         }
                     }
                 }
@@ -149,7 +151,7 @@ impl Completer for VariableCompletion {
 
         output.dedup();
 
-        (output, CompletionOptions::default())
+        output
     }
 }