Refactor all completion logic into NuCompleter (#2252)

* Refactor all completion logic into `NuCompleter`

This is the next step to improving completions. Previously, completion logic was
scattered about (`FilesystemShell`, `NuCompleter`, `Helper`, and `ShellManager`).
By unifying the core logic into a central location, it will be easier to take the
next steps in improving completion.

* Update context.rs

Co-authored-by: Jonathan Turner <jonathandturner@users.noreply.github.com>

crates/nu-cli/src/cli.rs

@@ -7,6 +7,7 @@ use crate::context::Context;
 use crate::git::current_branch;
 use crate::path::canonicalize;
 use crate::prelude::*;
+use crate::shell::completer::NuCompleter;
 use crate::shell::Helper;
 use crate::EnvironmentSyncer;
 use futures_codec::FramedRead;
@@ -778,7 +779,10 @@ pub async fn cli(
 
         let cwd = context.shell_manager.path();
 
-        rl.set_helper(Some(crate::shell::Helper::new(context.clone())));
+        rl.set_helper(Some(crate::shell::Helper::new(
+            Box::new(<NuCompleter as Default>::default()),
+            context.clone(),
+        )));
 
         let colored_prompt = {
             if use_starship {

crates/nu-cli/src/commands/classified/internal.rs

@@ -182,10 +182,7 @@ pub(crate) async fn run_internal_command(
                             }
                             CommandAction::EnterShell(location) => {
                                 context.shell_manager.insert_at_current(Box::new(
-                                    match FilesystemShell::with_location(
-                                        location,
-                                        context.registry().clone(),
-                                    ) {
+                                    match FilesystemShell::with_location(location) {
                                         Ok(v) => v,
                                         Err(err) => {
                                             return InputStream::one(

crates/nu-cli/src/completion.rs

@@ -1,16 +1,30 @@
 use nu_errors::ShellError;
 
+use crate::context;
+
 #[derive(Debug, Eq, PartialEq)]
 pub struct Suggestion {
     pub display: String,
     pub replacement: String,
 }
 
-pub struct Context<'a>(pub &'a rustyline::Context<'a>);
+pub struct Context<'a>(&'a context::Context, &'a rustyline::Context<'a>);
+
+impl<'a> Context<'a> {
+    pub fn new(a: &'a context::Context, b: &'a rustyline::Context<'a>) -> Context<'a> {
+        Context(a, b)
+    }
+}
+
+impl<'a> AsRef<context::Context> for Context<'a> {
+    fn as_ref(&self) -> &context::Context {
+        self.0
+    }
+}
 
 impl<'a> AsRef<rustyline::Context<'a>> for Context<'a> {
     fn as_ref(&self) -> &rustyline::Context<'a> {
-        self.0
+        self.1
     }
 }
 

crates/nu-cli/src/context.rs

@@ -150,14 +150,14 @@ impl Context {
         #[cfg(windows)]
         {
             Ok(Context {
-                registry: registry.clone(),
+                registry,
                 host: Arc::new(parking_lot::Mutex::new(Box::new(
                     crate::env::host::BasicHost,
                 ))),
                 current_errors: Arc::new(Mutex::new(vec![])),
                 ctrl_c: Arc::new(AtomicBool::new(false)),
                 user_recently_used_autoenv_untrust: false,
-                shell_manager: ShellManager::basic(registry)?,
+                shell_manager: ShellManager::basic()?,
                 windows_drives_previous_cwd: Arc::new(Mutex::new(std::collections::HashMap::new())),
                 raw_input: String::default(),
             })
@@ -166,14 +166,14 @@ impl Context {
         #[cfg(not(windows))]
         {
             Ok(Context {
-                registry: registry.clone(),
+                registry,
                 host: Arc::new(parking_lot::Mutex::new(Box::new(
                     crate::env::host::BasicHost,
                 ))),
                 current_errors: Arc::new(Mutex::new(vec![])),
                 ctrl_c: Arc::new(AtomicBool::new(false)),
                 user_recently_used_autoenv_untrust: false,
-                shell_manager: ShellManager::basic(registry)?,
+                shell_manager: ShellManager::basic()?,
                 raw_input: String::default(),
             })
         }

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

@@ -1,24 +1,24 @@
-use crate::context::CommandRegistry;
-
-use crate::data::config;
-use crate::prelude::*;
-use derive_new::new;
-#[cfg(all(windows, feature = "ichwh"))]
-use ichwh::IchwhError;
-#[cfg(all(windows, feature = "ichwh"))]
-use ichwh::IchwhResult;
-use indexmap::set::IndexSet;
-use rustyline::completion::{Completer, FilenameCompleter};
 use std::fs::{read_dir, DirEntry};
+
 #[cfg(unix)]
 use std::os::unix::fs::PermissionsExt;
-use std::path::PathBuf;
 
-#[derive(new)]
+use indexmap::set::IndexSet;
+use nu_errors::ShellError;
+use rustyline::completion::{Completer as _, FilenameCompleter};
+use rustyline::hint::{Hinter as _, HistoryHinter};
+
+#[cfg(all(windows, feature = "ichwh"))]
+use ichwh::{IchwhError, IchwhResult};
+
+use crate::completion::{self, Completer};
+use crate::context;
+use crate::data::config;
+use crate::prelude::*;
+
 pub(crate) struct NuCompleter {
-    pub file_completer: FilenameCompleter,
-    pub commands: CommandRegistry,
-    pub homedir: Option<PathBuf>,
+    file_completer: FilenameCompleter,
+    hinter: HistoryHinter,
 }
 
 #[derive(PartialEq, Eq, Debug)]
@@ -28,30 +28,33 @@ enum ReplacementLocation {
 }
 
 impl NuCompleter {
-    pub fn complete(
+    fn complete_internal(
         &self,
         line: &str,
         pos: usize,
-        context: &rustyline::Context,
+        context: &completion::Context,
     ) -> rustyline::Result<(usize, Vec<rustyline::completion::Pair>)> {
-        let commands: Vec<String> = self.commands.names();
-
         let line_chars: Vec<_> = line[..pos].chars().collect();
 
-        let (replace_pos, replace_loc) = self.get_replace_pos(line, pos);
-
-        let mut completions;
+        let (replace_pos, replace_loc) = get_replace_pos(line, pos);
 
         // See if we're a flag
+        let mut completions;
         if pos > 0 && replace_pos < line_chars.len() && line_chars[replace_pos] == '-' {
             if let Ok(lite_block) = nu_parser::lite_parse(line, 0) {
-                completions =
-                    self.get_matching_arguments(&lite_block, &line_chars, line, replace_pos, pos);
+                completions = get_matching_arguments(
+                    context.as_ref(),
+                    &lite_block,
+                    &line_chars,
+                    line,
+                    replace_pos,
+                    pos,
+                );
             } else {
-                completions = self.file_completer.complete(line, pos, context)?.1;
+                completions = self.file_completer.complete(line, pos, context.as_ref())?.1;
             }
         } else {
-            completions = self.file_completer.complete(line, pos, context)?.1;
+            completions = self.file_completer.complete(line, pos, context.as_ref())?.1;
 
             for completion in &mut completions {
                 if completion.replacement.contains("\\ ") {
@@ -70,7 +73,7 @@ impl NuCompleter {
                     }
                 }
             }
-        };
+        }
 
         let complete_from_path = match config::config(Tag::unknown()) {
             Ok(conf) => match conf.get("complete_from_path") {
@@ -83,9 +86,11 @@ impl NuCompleter {
         // Only complete executables or commands if the thing we're completing
         // is syntactically a command
         if replace_loc == ReplacementLocation::Command {
+            let context: &context::Context = context.as_ref();
+            let commands: Vec<String> = context.registry.names();
             let mut all_executables: IndexSet<_> = commands.iter().map(|x| x.to_string()).collect();
             if complete_from_path {
-                let path_executables = self.find_path_executables().unwrap_or_default();
+                let path_executables = find_path_executables().unwrap_or_default();
                 for path_exe in path_executables {
                     all_executables.insert(path_exe);
                 }
@@ -126,8 +131,191 @@ impl NuCompleter {
 
         Ok((replace_pos, completions))
     }
+}
 
-    fn get_replace_pos(&self, line: &str, pos: usize) -> (usize, ReplacementLocation) {
+impl Completer for NuCompleter {
+    fn complete(
+        &self,
+        line: &str,
+        pos: usize,
+        context: &completion::Context,
+    ) -> Result<(usize, Vec<completion::Suggestion>), ShellError> {
+        let expanded = nu_parser::expand_ndots(&line);
+
+        // Find the first not-matching char position, if there is one
+        let differ_pos = line
+            .chars()
+            .zip(expanded.chars())
+            .enumerate()
+            .find(|(_index, (a, b))| a != b)
+            .map(|(differ_pos, _)| differ_pos);
+
+        let pos = if let Some(differ_pos) = differ_pos {
+            if differ_pos < pos {
+                pos + (expanded.len() - line.len())
+            } else {
+                pos
+            }
+        } else {
+            pos
+        };
+
+        self.complete_internal(&expanded, pos, context)
+            .map_err(|e| ShellError::untagged_runtime_error(format!("{}", e)))
+            .map(requote)
+            .map(|(pos, completions)| {
+                (
+                    pos,
+                    completions
+                        .into_iter()
+                        .map(|pair| completion::Suggestion {
+                            display: pair.display,
+                            replacement: pair.replacement,
+                        })
+                        .collect(),
+                )
+            })
+    }
+
+    fn hint(&self, line: &str, pos: usize, ctx: &completion::Context<'_>) -> Option<String> {
+        self.hinter.hint(line, pos, &ctx.as_ref())
+    }
+}
+
+impl Default for NuCompleter {
+    fn default() -> NuCompleter {
+        NuCompleter {
+            file_completer: FilenameCompleter::new(),
+            hinter: HistoryHinter {},
+        }
+    }
+}
+
+fn get_matching_arguments(
+    context: &context::Context,
+    lite_block: &nu_parser::LiteBlock,
+    line_chars: &[char],
+    line: &str,
+    replace_pos: usize,
+    pos: usize,
+) -> Vec<rustyline::completion::Pair> {
+    let mut matching_arguments = vec![];
+
+    let mut line_copy = line.to_string();
+    let substring = line_chars[replace_pos..pos].iter().collect::<String>();
+    let replace_string = (replace_pos..pos).map(|_| " ").collect::<String>();
+    line_copy.replace_range(replace_pos..pos, &replace_string);
+
+    let result = nu_parser::classify_block(&lite_block, &context.registry);
+
+    for pipeline in &result.block.block {
+        for command in &pipeline.list {
+            if let nu_protocol::hir::ClassifiedCommand::Internal(
+                nu_protocol::hir::InternalCommand { args, .. },
+            ) = command
+            {
+                if replace_pos >= args.span.start() && replace_pos <= args.span.end() {
+                    if let Some(named) = &args.named {
+                        for (name, _) in named.iter() {
+                            let full_flag = format!("--{}", name);
+
+                            if full_flag.starts_with(&substring) {
+                                matching_arguments.push(rustyline::completion::Pair {
+                                    display: full_flag.clone(),
+                                    replacement: full_flag,
+                                });
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    matching_arguments
+}
+
+// These is_executable/pathext implementations are copied from ichwh and modified
+// to not be async
+
+#[cfg(windows)]
+fn pathext() -> IchwhResult<Vec<String>> {
+    Ok(std::env::var_os("PATHEXT")
+        .ok_or(IchwhError::PathextNotDefined)?
+        .to_string_lossy()
+        .split(';')
+        // Cut off the leading '.' character
+        .map(|ext| ext[1..].to_string())
+        .collect::<Vec<_>>())
+}
+
+#[cfg(windows)]
+fn is_executable(file: &DirEntry) -> bool {
+    if let Ok(metadata) = file.metadata() {
+        let file_type = metadata.file_type();
+
+        // If the entry isn't a file, it cannot be executable
+        if !(file_type.is_file() || file_type.is_symlink()) {
+            return false;
+        }
+
+        if let Some(extension) = file.path().extension() {
+            if let Ok(exts) = pathext() {
+                exts.iter()
+                    .any(|ext| extension.to_string_lossy().eq_ignore_ascii_case(ext))
+            } else {
+                false
+            }
+        } else {
+            false
+        }
+    } else {
+        false
+    }
+}
+
+#[cfg(target_arch = "wasm32")]
+fn is_executable(file: &DirEntry) -> bool {
+    false
+}
+
+#[cfg(unix)]
+fn is_executable(file: &DirEntry) -> bool {
+    let metadata = file.metadata();
+
+    if let Ok(metadata) = metadata {
+        let filetype = metadata.file_type();
+        let permissions = metadata.permissions();
+
+        // The file is executable if it is a directory or a symlink and the permissions are set for
+        // owner, group, or other
+        (filetype.is_file() || filetype.is_symlink()) && (permissions.mode() & 0o111 != 0)
+    } else {
+        false
+    }
+}
+
+fn find_path_executables() -> Option<IndexSet<String>> {
+    let path_var = std::env::var_os("PATH")?;
+    let paths: Vec<_> = std::env::split_paths(&path_var).collect();
+
+    let mut executables: IndexSet<String> = IndexSet::new();
+    for path in paths {
+        if let Ok(mut contents) = read_dir(path) {
+            while let Some(Ok(item)) = contents.next() {
+                if is_executable(&item) {
+                    if let Ok(name) = item.file_name().into_string() {
+                        executables.insert(name);
+                    }
+                }
+            }
+        }
+    }
+
+    Some(executables)
+}
+
+fn get_replace_pos(line: &str, pos: usize) -> (usize, ReplacementLocation) {
     let line_chars: Vec<_> = line[..pos].chars().collect();
     let mut replace_pos = line_chars.len();
     let mut parsed_pos = false;
@@ -165,129 +353,26 @@ impl NuCompleter {
     }
 
     (replace_pos, loc)
-    }
+}
 
-    fn get_matching_arguments(
-        &self,
-        lite_block: &nu_parser::LiteBlock,
-        line_chars: &[char],
-        line: &str,
-        replace_pos: usize,
-        pos: usize,
-    ) -> Vec<rustyline::completion::Pair> {
-        let mut matching_arguments = vec![];
+fn requote(
+    items: (usize, Vec<rustyline::completion::Pair>),
+) -> (usize, Vec<rustyline::completion::Pair>) {
+    let mut new_items = Vec::with_capacity(items.1.len());
 
-        let mut line_copy = line.to_string();
-        let substring = line_chars[replace_pos..pos].iter().collect::<String>();
-        let replace_string = (replace_pos..pos).map(|_| " ").collect::<String>();
-        line_copy.replace_range(replace_pos..pos, &replace_string);
+    for item in items.1 {
+        let unescaped = rustyline::completion::unescape(&item.replacement, Some('\\'));
+        let maybe_quote = if unescaped != item.replacement {
+            "\""
+        } else {
+            ""
+        };
 
-        let result = nu_parser::classify_block(&lite_block, &self.commands);
-
-        for pipeline in &result.block.block {
-            for command in &pipeline.list {
-                if let nu_protocol::hir::ClassifiedCommand::Internal(
-                    nu_protocol::hir::InternalCommand { args, .. },
-                ) = command
-                {
-                    if replace_pos >= args.span.start() && replace_pos <= args.span.end() {
-                        if let Some(named) = &args.named {
-                            for (name, _) in named.iter() {
-                                let full_flag = format!("--{}", name);
-
-                                if full_flag.starts_with(&substring) {
-                                    matching_arguments.push(rustyline::completion::Pair {
-                                        display: full_flag.clone(),
-                                        replacement: full_flag,
+        new_items.push(rustyline::completion::Pair {
+            display: item.display,
+            replacement: format!("{}{}{}", maybe_quote, unescaped, maybe_quote),
         });
     }
-                            }
-                        }
-                    }
-                }
-            }
-        }
 
-        matching_arguments
-    }
-
-    // These is_executable/pathext implementations are copied from ichwh and modified
-    // to not be async
-
-    #[cfg(windows)]
-    fn pathext(&self) -> IchwhResult<Vec<String>> {
-        Ok(std::env::var_os("PATHEXT")
-            .ok_or(IchwhError::PathextNotDefined)?
-            .to_string_lossy()
-            .split(';')
-            // Cut off the leading '.' character
-            .map(|ext| ext[1..].to_string())
-            .collect::<Vec<_>>())
-    }
-
-    #[cfg(windows)]
-    fn is_executable(&self, file: &DirEntry) -> bool {
-        if let Ok(metadata) = file.metadata() {
-            let file_type = metadata.file_type();
-
-            // If the entry isn't a file, it cannot be executable
-            if !(file_type.is_file() || file_type.is_symlink()) {
-                return false;
-            }
-
-            if let Some(extension) = file.path().extension() {
-                if let Ok(exts) = self.pathext() {
-                    exts.iter()
-                        .any(|ext| extension.to_string_lossy().eq_ignore_ascii_case(ext))
-                } else {
-                    false
-                }
-            } else {
-                false
-            }
-        } else {
-            false
-        }
-    }
-
-    #[cfg(target_arch = "wasm32")]
-    fn is_executable(&self, file: &DirEntry) -> bool {
-        false
-    }
-
-    #[cfg(unix)]
-    fn is_executable(&self, file: &DirEntry) -> bool {
-        let metadata = file.metadata();
-
-        if let Ok(metadata) = metadata {
-            let filetype = metadata.file_type();
-            let permissions = metadata.permissions();
-
-            // The file is executable if it is a directory or a symlink and the permissions are set for
-            // owner, group, or other
-            (filetype.is_file() || filetype.is_symlink()) && (permissions.mode() & 0o111 != 0)
-        } else {
-            false
-        }
-    }
-
-    fn find_path_executables(&self) -> Option<IndexSet<String>> {
-        let path_var = std::env::var_os("PATH")?;
-        let paths: Vec<_> = std::env::split_paths(&path_var).collect();
-
-        let mut executables: IndexSet<String> = IndexSet::new();
-        for path in paths {
-            if let Ok(mut contents) = read_dir(path) {
-                while let Some(Ok(item)) = contents.next() {
-                    if self.is_executable(&item) {
-                        if let Ok(name) = item.file_name().into_string() {
-                            executables.insert(name);
-                        }
-                    }
-                }
-            }
-        }
-
-        Some(executables)
-    }
+    (items.0, new_items)
 }

crates/nu-cli/src/shell/filesystem_shell.rs

@@ -5,16 +5,12 @@ use crate::commands::ls::LsArgs;
 use crate::commands::mkdir::MkdirArgs;
 use crate::commands::move_::mv::Arguments as MvArgs;
 use crate::commands::rm::RemoveArgs;
-use crate::completion;
 use crate::data::dir_entry_dict;
 use crate::path::canonicalize;
 use crate::prelude::*;
-use crate::shell::completer::NuCompleter;
 use crate::shell::shell::Shell;
 use crate::utils::FileStructure;
 
-use rustyline::completion::FilenameCompleter;
-use rustyline::hint::{Hinter, HistoryHinter};
 use std::collections::HashMap;
 use std::io::{Error, ErrorKind};
 use std::path::{Path, PathBuf};
@@ -28,15 +24,12 @@ use futures_util::TryStreamExt;
 use std::os::unix::fs::PermissionsExt;
 
 use nu_errors::ShellError;
-use nu_parser::expand_ndots;
 use nu_protocol::{Primitive, ReturnSuccess, UntaggedValue};
 use nu_source::Tagged;
 
 pub struct FilesystemShell {
     pub(crate) path: String,
     pub(crate) last_path: String,
-    completer: NuCompleter,
-    hinter: HistoryHinter,
 }
 
 impl std::fmt::Debug for FilesystemShell {
@@ -50,18 +43,12 @@ impl Clone for FilesystemShell {
         FilesystemShell {
             path: self.path.clone(),
             last_path: self.path.clone(),
-            completer: NuCompleter {
-                file_completer: FilenameCompleter::new(),
-                commands: self.completer.commands.clone(),
-                homedir: self.homedir(),
-            },
-            hinter: HistoryHinter {},
         }
     }
 }
 
 impl FilesystemShell {
-    pub fn basic(commands: CommandRegistry) -> Result<FilesystemShell, Error> {
+    pub fn basic() -> Result<FilesystemShell, Error> {
         let path = match std::env::current_dir() {
             Ok(path) => path,
             Err(_) => PathBuf::from("/"),
@@ -70,33 +57,15 @@ impl FilesystemShell {
         Ok(FilesystemShell {
             path: path.to_string_lossy().to_string(),
             last_path: path.to_string_lossy().to_string(),
-            completer: NuCompleter {
-                file_completer: FilenameCompleter::new(),
-                commands,
-                homedir: homedir_if_possible(),
-            },
-            hinter: HistoryHinter {},
         })
     }
 
-    pub fn with_location(
-        path: String,
-        commands: CommandRegistry,
-    ) -> Result<FilesystemShell, std::io::Error> {
+    pub fn with_location(path: String) -> Result<FilesystemShell, std::io::Error> {
         let path = canonicalize(std::env::current_dir()?, &path)?;
         let path = path.display().to_string();
         let last_path = path.clone();
 
-        Ok(FilesystemShell {
-            path,
-            last_path,
-            completer: NuCompleter {
-                file_completer: FilenameCompleter::new(),
-                commands,
-                homedir: homedir_if_possible(),
-            },
-            hinter: HistoryHinter {},
-        })
+        Ok(FilesystemShell { path, last_path })
     }
 }
 
@@ -740,56 +709,6 @@ impl Shell for FilesystemShell {
     }
 }
 
-impl completion::Completer for FilesystemShell {
-    fn complete(
-        &self,
-        line: &str,
-        pos: usize,
-        ctx: &completion::Context<'_>,
-    ) -> Result<(usize, Vec<completion::Suggestion>), ShellError> {
-        let expanded = expand_ndots(&line);
-
-        // Find the first not-matching char position, if there is one
-        let differ_pos = line
-            .chars()
-            .zip(expanded.chars())
-            .enumerate()
-            .find(|(_index, (a, b))| a != b)
-            .map(|(differ_pos, _)| differ_pos);
-
-        let pos = if let Some(differ_pos) = differ_pos {
-            if differ_pos < pos {
-                pos + (expanded.len() - line.len())
-            } else {
-                pos
-            }
-        } else {
-            pos
-        };
-
-        self.completer
-            .complete(&expanded, pos, ctx.as_ref())
-            .map_err(|e| ShellError::untagged_runtime_error(format!("{}", e)))
-            .map(requote)
-            .map(|(pos, completions)| {
-                (
-                    pos,
-                    completions
-                        .into_iter()
-                        .map(|pair| completion::Suggestion {
-                            display: pair.display,
-                            replacement: pair.replacement,
-                        })
-                        .collect(),
-                )
-            })
-    }
-
-    fn hint(&self, line: &str, pos: usize, ctx: &completion::Context<'_>) -> Option<String> {
-        self.hinter.hint(line, pos, ctx.as_ref())
-    }
-}
-
 struct TaggedPathBuf<'a>(&'a PathBuf, &'a Tag);
 
 fn move_file(from: TaggedPathBuf, to: TaggedPathBuf) -> Result<(), ShellError> {
@@ -876,25 +795,3 @@ fn is_hidden_dir(dir: impl AsRef<Path>) -> bool {
             .unwrap_or(false)
     }
 }
-
-fn requote(
-    items: (usize, Vec<rustyline::completion::Pair>),
-) -> (usize, Vec<rustyline::completion::Pair>) {
-    let mut new_items = Vec::with_capacity(items.1.len());
-
-    for item in items.1 {
-        let unescaped = rustyline::completion::unescape(&item.replacement, Some('\\'));
-        let maybe_quote = if unescaped != item.replacement {
-            "\""
-        } else {
-            ""
-        };
-
-        new_items.push(rustyline::completion::Pair {
-            display: item.display,
-            replacement: format!("{}{}{}", maybe_quote, unescaped, maybe_quote),
-        });
-    }
-
-    (items.0, new_items)
-}

crates/nu-cli/src/shell/helper.rs

@@ -1,4 +1,4 @@
-use crate::completion::{self, Completer as _};
+use crate::completion::{self, Completer};
 use crate::context::Context;
 use crate::shell::palette::{DefaultPalette, Palette};
 
@@ -6,19 +6,19 @@ use ansi_term::{Color, Style};
 use nu_parser::SignatureRegistry;
 use nu_protocol::hir::FlatShape;
 use nu_source::{Spanned, Tag, Tagged};
-use rustyline::completion::Completer;
-use rustyline::highlight::Highlighter;
 use rustyline::hint::Hinter;
 use std::borrow::Cow::{self, Owned};
 
 pub struct Helper {
+    completer: Box<dyn Completer>,
     context: Context,
     pub colored_prompt: String,
 }
 
 impl Helper {
-    pub(crate) fn new(context: Context) -> Helper {
+    pub(crate) fn new(completer: Box<dyn Completer>, context: Context) -> Helper {
         Helper {
+            completer,
             context,
             colored_prompt: String::new(),
         }
@@ -35,7 +35,7 @@ impl rustyline::completion::Candidate for completion::Suggestion {
     }
 }
 
-impl Completer for Helper {
+impl rustyline::completion::Completer for Helper {
     type Candidate = completion::Suggestion;
 
     fn complete(
@@ -44,9 +44,8 @@ impl Completer for Helper {
         pos: usize,
         ctx: &rustyline::Context<'_>,
     ) -> Result<(usize, Vec<Self::Candidate>), rustyline::error::ReadlineError> {
-        let ctx = completion::Context(ctx);
-        self.context
-            .shell_manager
+        let ctx = completion::Context::new(&self.context, ctx);
+        self.completer
             .complete(line, pos, &ctx)
             .map_err(|_| rustyline::error::ReadlineError::Eof)
     }
@@ -54,12 +53,12 @@ impl Completer for Helper {
 
 impl Hinter for Helper {
     fn hint(&self, line: &str, pos: usize, ctx: &rustyline::Context<'_>) -> Option<String> {
-        let ctx = completion::Context(ctx);
-        self.context.shell_manager.hint(line, pos, &ctx)
+        let ctx = completion::Context::new(&self.context, ctx);
+        self.completer.hint(line, pos, &ctx)
     }
 }
 
-impl Highlighter for Helper {
+impl rustyline::highlight::Highlighter for Helper {
     fn highlight_prompt<'b, 's: 'b, 'p: 'b>(
         &'s self,
         prompt: &'p str,

crates/nu-cli/src/shell/shell.rs

@@ -6,7 +6,6 @@ use crate::commands::ls::LsArgs;
 use crate::commands::mkdir::MkdirArgs;
 use crate::commands::move_::mv::Arguments as MvArgs;
 use crate::commands::rm::RemoveArgs;
-use crate::completion;
 use crate::prelude::*;
 use crate::stream::OutputStream;
 
@@ -14,7 +13,7 @@ use encoding_rs::Encoding;
 use nu_errors::ShellError;
 use std::path::PathBuf;
 
-pub trait Shell: completion::Completer + std::fmt::Debug {
+pub trait Shell: std::fmt::Debug {
     fn name(&self) -> String;
     fn homedir(&self) -> Option<PathBuf>;
 

crates/nu-cli/src/shell/shell_manager.rs

@@ -6,7 +6,6 @@ use crate::commands::ls::LsArgs;
 use crate::commands::mkdir::MkdirArgs;
 use crate::commands::move_::mv::Arguments as MvArgs;
 use crate::commands::rm::RemoveArgs;
-use crate::completion::{self, Completer};
 use crate::prelude::*;
 use crate::shell::filesystem_shell::FilesystemShell;
 use crate::shell::shell::Shell;
@@ -27,12 +26,10 @@ pub struct ShellManager {
 }
 
 impl ShellManager {
-    pub fn basic(commands: CommandRegistry) -> Result<ShellManager, Box<dyn Error>> {
+    pub fn basic() -> Result<ShellManager, Box<dyn Error>> {
         Ok(ShellManager {
             current_shell: Arc::new(AtomicUsize::new(0)),
-            shells: Arc::new(Mutex::new(vec![Box::new(FilesystemShell::basic(
-                commands,
-            )?)])),
+            shells: Arc::new(Mutex::new(vec![Box::new(FilesystemShell::basic()?)])),
         })
     }
 
@@ -181,24 +178,3 @@ impl ShellManager {
         shells[self.current_shell()].mv(args, name, &path)
     }
 }
-
-impl Completer for ShellManager {
-    fn complete(
-        &self,
-        line: &str,
-        pos: usize,
-        ctx: &completion::Context<'_>,
-    ) -> Result<(usize, Vec<completion::Suggestion>), ShellError> {
-        self.shells.lock()[self.current_shell()].complete(line, pos, ctx)
-    }
-
-    fn hint(
-        &self,
-        line: &str,
-        pos: usize,
-        ctx: &completion::Context<'_>,
-        //context: ExpandContext,
-    ) -> Option<String> {
-        self.shells.lock()[self.current_shell()].hint(line, pos, ctx)
-    }
-}