Pass Stack ref to Completer::fetch (nushell#12783)

# Description Adds an additional `&Stack` parameter to `Completer::fetch` so that the completers don't have to store a `Stack` themselves. I also removed unnecessary `EngineState`s from the completers, since the same `EngineState` is available in the `working_set.permanent_state` also passed to `Completer::fetch`.
sholderbach · May 9, 2024 · 7271ad7 · 7271ad7
1 parent 3b3f482
commit 7271ad7
Show file tree

Hide file tree

Showing 13 changed files with 146 additions and 183 deletions.
diff --git a/crates/nu-cli/src/completions/base.rs b/crates/nu-cli/src/completions/base.rs
@@ -1,13 +1,18 @@
 use crate::completions::{CompletionOptions, SortBy};
-use nu_protocol::{engine::StateWorkingSet, levenshtein_distance, Span};
+use nu_protocol::{
+    engine::{Stack, StateWorkingSet},
+    levenshtein_distance, Span,
+};
 use reedline::Suggestion;
 
 // Completer trait represents the three stages of the completion
 // fetch, filter and sort
 pub trait Completer {
+    #[allow(clippy::too_many_arguments)]
     fn fetch(
         &mut self,
         working_set: &StateWorkingSet,
+        stack: &Stack,
         prefix: Vec<u8>,
         span: Span,
         offset: usize,

diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs
@@ -4,31 +4,26 @@ use crate::{
 };
 use nu_parser::FlatShape;
 use nu_protocol::{
-    engine::{CachedFile, EngineState, StateWorkingSet},
+    engine::{CachedFile, Stack, StateWorkingSet},
     Span,
 };
 use reedline::Suggestion;
-use std::sync::Arc;
 
 use super::SemanticSuggestion;
 
 pub struct CommandCompletion {
-    engine_state: Arc<EngineState>,
     flattened: Vec<(Span, FlatShape)>,
     flat_shape: FlatShape,
     force_completion_after_space: bool,
 }
 
 impl CommandCompletion {
     pub fn new(
-        engine_state: Arc<EngineState>,
-        _: &StateWorkingSet,
         flattened: Vec<(Span, FlatShape)>,
         flat_shape: FlatShape,
         force_completion_after_space: bool,
     ) -> Self {
         Self {
-            engine_state,
             flattened,
             flat_shape,
             force_completion_after_space,
@@ -37,13 +32,14 @@ impl CommandCompletion {
 
     fn external_command_completion(
         &self,
+        working_set: &StateWorkingSet,
         prefix: &str,
         match_algorithm: MatchAlgorithm,
     ) -> Vec<String> {
         let mut executables = vec![];
 
         // os agnostic way to get the PATH env var
-        let paths = self.engine_state.get_path_env_var();
+        let paths = working_set.permanent_state.get_path_env_var();
 
         if let Some(paths) = paths {
             if let Ok(paths) = paths.as_list() {
@@ -52,7 +48,10 @@ impl CommandCompletion {
 
                     if let Ok(mut contents) = std::fs::read_dir(path.as_ref()) {
                         while let Some(Ok(item)) = contents.next() {
-                            if self.engine_state.config.max_external_completion_results
+                            if working_set
+                                .permanent_state
+                                .config
+                                .max_external_completion_results
                                 > executables.len() as i64
                                 && !executables.contains(
                                     &item
@@ -114,7 +113,7 @@ impl CommandCompletion {
 
         if find_externals {
             let results_external = self
-                .external_command_completion(&partial, match_algorithm)
+                .external_command_completion(working_set, &partial, match_algorithm)
                 .into_iter()
                 .map(move |x| SemanticSuggestion {
                     suggestion: Suggestion {
@@ -161,6 +160,7 @@ impl Completer for CommandCompletion {
     fn fetch(
         &mut self,
         working_set: &StateWorkingSet,
+        _stack: &Stack,
         _prefix: Vec<u8>,
         span: Span,
         offset: usize,
@@ -266,6 +266,8 @@ pub fn is_passthrough_command(working_set_file_contents: &[CachedFile]) -> bool
 #[cfg(test)]
 mod command_completions_tests {
     use super::*;
+    use nu_protocol::engine::EngineState;
+    use std::sync::Arc;
 
     #[test]
     fn test_find_non_whitespace_index() {

diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs
@@ -22,10 +22,10 @@ pub struct NuCompleter {
 }
 
 impl NuCompleter {
-    pub fn new(engine_state: Arc<EngineState>, stack: Stack) -> Self {
+    pub fn new(engine_state: Arc<EngineState>, stack: Arc<Stack>) -> Self {
         Self {
             engine_state,
-            stack: stack.reset_out_dest().capture(),
+            stack: Stack::with_parent(stack).reset_out_dest().capture(),
         }
     }
 
@@ -52,8 +52,15 @@ impl NuCompleter {
         };
 
         // Fetch
-        let mut suggestions =
-            completer.fetch(working_set, prefix.clone(), new_span, offset, pos, &options);
+        let mut suggestions = completer.fetch(
+            working_set,
+            &self.stack,
+            prefix.clone(),
+            new_span,
+            offset,
+            pos,
+            &options,
+        );
 
         // Sort
         suggestions = completer.sort(suggestions, prefix);
@@ -175,11 +182,8 @@ impl NuCompleter {
 
                         // Variables completion
                         if prefix.starts_with(b"$") || most_left_var.is_some() {
-                            let mut completer = VariableCompletion::new(
-                                self.engine_state.clone(),
-                                self.stack.clone(),
-                                most_left_var.unwrap_or((vec![], vec![])),
-                            );
+                            let mut completer =
+                                VariableCompletion::new(most_left_var.unwrap_or((vec![], vec![])));
 
                             return self.process_completion(
                                 &mut completer,
@@ -224,8 +228,6 @@ impl NuCompleter {
                             || (flat_idx == 0 && working_set.get_span_contents(new_span).is_empty())
                         {
                             let mut completer = CommandCompletion::new(
-                                self.engine_state.clone(),
-                                &working_set,
                                 flattened.clone(),
                                 // flat_idx,
                                 FlatShape::String,
@@ -253,10 +255,7 @@ impl NuCompleter {
                                     || prev_expr_str == b"overlay use"
                                     || prev_expr_str == b"source-env"
                                 {
-                                    let mut completer = DotNuCompletion::new(
-                                        self.engine_state.clone(),
-                                        self.stack.clone(),
-                                    );
+                                    let mut completer = DotNuCompletion::new();
 
                                     return self.process_completion(
                                         &mut completer,
@@ -267,10 +266,7 @@ impl NuCompleter {
                                         pos,
                                     );
                                 } else if prev_expr_str == b"ls" {
-                                    let mut completer = FileCompletion::new(
-                                        self.engine_state.clone(),
-                                        self.stack.clone(),
-                                    );
+                                    let mut completer = FileCompletion::new();
 
                                     return self.process_completion(
                                         &mut completer,
@@ -288,7 +284,6 @@ impl NuCompleter {
                         match &flat.1 {
                             FlatShape::Custom(decl_id) => {
                                 let mut completer = CustomCompletion::new(
-                                    self.engine_state.clone(),
                                     self.stack.clone(),
                                     *decl_id,
                                     initial_line,
@@ -304,10 +299,7 @@ impl NuCompleter {
                                 );
                             }
                             FlatShape::Directory => {
-                                let mut completer = DirectoryCompletion::new(
-                                    self.engine_state.clone(),
-                                    self.stack.clone(),
-                                );
+                                let mut completer = DirectoryCompletion::new();
 
                                 return self.process_completion(
                                     &mut completer,
@@ -319,10 +311,7 @@ impl NuCompleter {
                                 );
                             }
                             FlatShape::Filepath | FlatShape::GlobPattern => {
-                                let mut completer = FileCompletion::new(
-                                    self.engine_state.clone(),
-                                    self.stack.clone(),
-                                );
+                                let mut completer = FileCompletion::new();
 
                                 return self.process_completion(
                                     &mut completer,
@@ -335,8 +324,6 @@ impl NuCompleter {
                             }
                             flat_shape => {
                                 let mut completer = CommandCompletion::new(
-                                    self.engine_state.clone(),
-                                    &working_set,
                                     flattened.clone(),
                                     // flat_idx,
                                     flat_shape.clone(),
@@ -369,10 +356,7 @@ impl NuCompleter {
                                 }
 
                                 // Check for file completion
-                                let mut completer = FileCompletion::new(
-                                    self.engine_state.clone(),
-                                    self.stack.clone(),
-                                );
+                                let mut completer = FileCompletion::new();
                                 out = self.process_completion(
                                     &mut completer,
                                     &working_set,
@@ -557,7 +541,7 @@ mod completer_tests {
             result.err().unwrap()
         );
 
-        let mut completer = NuCompleter::new(engine_state.into(), Stack::new());
+        let mut completer = NuCompleter::new(engine_state.into(), Arc::new(Stack::new()));
         let dataset = [
             ("sudo", false, "", Vec::new()),
             ("sudo l", true, "l", vec!["ls", "let", "lines", "loop"]),

diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs
@@ -6,25 +6,23 @@ use nu_engine::eval_call;
 use nu_protocol::{
     ast::{Argument, Call, Expr, Expression},
     debugger::WithoutDebug,
-    engine::{EngineState, Stack, StateWorkingSet},
+    engine::{Stack, StateWorkingSet},
     PipelineData, Span, Type, Value,
 };
 use nu_utils::IgnoreCaseExt;
-use std::{collections::HashMap, sync::Arc};
+use std::collections::HashMap;
 
 pub struct CustomCompletion {
-    engine_state: Arc<EngineState>,
     stack: Stack,
     decl_id: usize,
     line: String,
     sort_by: SortBy,
 }
 
 impl CustomCompletion {
-    pub fn new(engine_state: Arc<EngineState>, stack: Stack, decl_id: usize, line: String) -> Self {
+    pub fn new(stack: Stack, decl_id: usize, line: String) -> Self {
         Self {
-            engine_state,
-            stack: stack.reset_out_dest().capture(),
+            stack,
             decl_id,
             line,
             sort_by: SortBy::None,
@@ -35,7 +33,8 @@ impl CustomCompletion {
 impl Completer for CustomCompletion {
     fn fetch(
         &mut self,
-        _: &StateWorkingSet,
+        working_set: &StateWorkingSet,
+        _stack: &Stack,
         prefix: Vec<u8>,
         span: Span,
         offset: usize,
@@ -47,7 +46,7 @@ impl Completer for CustomCompletion {
 
         // Call custom declaration
         let result = eval_call::<WithoutDebug>(
-            &self.engine_state,
+            working_set.permanent_state,
             &mut self.stack,
             &Call {
                 decl_id: self.decl_id,

diff --git a/crates/nu-cli/src/completions/directory_completions.rs b/crates/nu-cli/src/completions/directory_completions.rs
@@ -8,36 +8,28 @@ use nu_protocol::{
     levenshtein_distance, Span,
 };
 use reedline::Suggestion;
-use std::{
-    path::{Path, MAIN_SEPARATOR as SEP},
-    sync::Arc,
-};
+use std::path::{Path, MAIN_SEPARATOR as SEP};
 
 use super::SemanticSuggestion;
 
-#[derive(Clone)]
-pub struct DirectoryCompletion {
-    engine_state: Arc<EngineState>,
-    stack: Stack,
-}
+#[derive(Clone, Default)]
+pub struct DirectoryCompletion {}
 
 impl DirectoryCompletion {
-    pub fn new(engine_state: Arc<EngineState>, stack: Stack) -> Self {
-        Self {
-            engine_state,
-            stack,
-        }
+    pub fn new() -> Self {
+        Self::default()
     }
 }
 
 impl Completer for DirectoryCompletion {
     fn fetch(
         &mut self,
         working_set: &StateWorkingSet,
+        stack: &Stack,
         prefix: Vec<u8>,
         span: Span,
         offset: usize,
-        _: usize,
+        _pos: usize,
         options: &CompletionOptions,
     ) -> Vec<SemanticSuggestion> {
         let AdjustView { prefix, span, .. } = adjust_if_intermediate(&prefix, working_set, span);
@@ -47,10 +39,10 @@ impl Completer for DirectoryCompletion {
         let output: Vec<_> = directory_completion(
             span,
             &prefix,
-            &self.engine_state.current_work_dir(),
+            &working_set.permanent_state.current_work_dir(),
             options,
-            self.engine_state.as_ref(),
-            &self.stack,
+            working_set.permanent_state,
+            stack,
         )
         .into_iter()
         .map(move |x| SemanticSuggestion {