Add method to convert ClassifiedBlock into completion locations. (#2316)

The completion engine maps completion locations to spans on a line, which
indicate whther to complete a command name, flag name, argument, and so on.

Initial implementation is simplistic, with some rough edges, since it relies
heavily on the parser's interpretation. For example

    du -

if asking for completions, `-` is considered a positional argument by the
parser, but the user is likely looking for a flag. These scenarios will be
addressed in a series of progressive enhancements to the engine.

crates/nu-cli/src/cli.rs

@@ -7,7 +7,6 @@ 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;
@@ -787,10 +786,7 @@ pub async fn cli(
 
         let cwd = context.shell_manager.path();
 
-        rl.set_helper(Some(crate::shell::Helper::new(
-            Box::new(<NuCompleter as Default>::default()),
-            context.clone(),
-        )));
+        rl.set_helper(Some(crate::shell::Helper::new(context.clone())));
 
         let colored_prompt = {
             if let Some(prompt) = config.get("prompt") {

crates/nu-cli/src/completion/command.rs

@@ -0,0 +1,115 @@
+use std::fs::{read_dir, DirEntry};
+use std::iter::FromIterator;
+
+#[cfg(all(windows, feature = "ichwh"))]
+use ichwh::{IchwhError, IchwhResult};
+use indexmap::set::IndexSet;
+
+use crate::completion::{Context, Suggestion};
+use crate::context;
+
+pub struct Completer;
+
+impl Completer {
+    pub fn complete(&self, ctx: &Context<'_>, partial: &str) -> Vec<Suggestion> {
+        let context: &context::Context = ctx.as_ref();
+        let mut commands: IndexSet<String> = IndexSet::from_iter(context.registry.names());
+
+        let path_executables = find_path_executables().unwrap_or_default();
+
+        // TODO quote these, if necessary
+        commands.extend(path_executables.into_iter());
+
+        commands
+            .into_iter()
+            .filter(|v| v.starts_with(partial))
+            .map(|v| Suggestion {
+                replacement: format!("{} ", v),
+                display: v,
+            })
+            .collect()
+    }
+}
+
+// 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 {
+    use std::os::unix::fs::PermissionsExt;
+
+    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
+    }
+}
+
+// TODO cache these, but watch for changes to PATH
+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)
+}

crates/nu-cli/src/completion/engine.rs

@@ -0,0 +1,334 @@
+use nu_protocol::hir::*;
+use nu_source::{Span, Spanned, SpannedItem};
+
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum LocationType {
+    Command,
+    Flag(String),                             // command name
+    Argument(Option<String>, Option<String>), // command name, argument name
+    Variable,
+}
+
+pub type CompletionLocation = Spanned<LocationType>;
+
+// TODO The below is very similar to shapes / expression_to_flat_shape. Check back October 2020
+//      to see if we're close enough to just make use of those.
+
+struct Flatten<'s> {
+    line: &'s str,
+    command: Option<String>,
+    flag: Option<String>,
+}
+
+impl<'s> Flatten<'s> {
+    /// Converts a SpannedExpression into a completion location for use in NuCompleter
+    fn expression(&self, e: &SpannedExpression) -> Vec<CompletionLocation> {
+        match &e.expr {
+            Expression::Block(block) => self.completion_locations(block),
+            Expression::Invocation(block) => self.completion_locations(block),
+            Expression::List(exprs) => exprs.iter().flat_map(|v| self.expression(v)).collect(),
+            Expression::Command(span) => vec![LocationType::Command.spanned(*span)],
+            Expression::Path(path) => self.expression(&path.head),
+            Expression::Variable(_) => vec![LocationType::Variable.spanned(e.span)],
+
+            Expression::Boolean(_)
+            | Expression::FilePath(_)
+            | Expression::Literal(Literal::ColumnPath(_))
+            | Expression::Literal(Literal::GlobPattern(_))
+            | Expression::Literal(Literal::Number(_))
+            | Expression::Literal(Literal::Size(_, _))
+            | Expression::Literal(Literal::String(_)) => {
+                vec![
+                    LocationType::Argument(self.command.clone(), self.flag.clone()).spanned(e.span),
+                ]
+            }
+
+            Expression::Binary(binary) => {
+                let mut result = Vec::new();
+                result.append(&mut self.expression(&binary.left));
+                result.append(&mut self.expression(&binary.right));
+                result
+            }
+            Expression::Range(range) => {
+                let mut result = Vec::new();
+                result.append(&mut self.expression(&range.left));
+                result.append(&mut self.expression(&range.right));
+                result
+            }
+
+            Expression::ExternalWord
+            | Expression::ExternalCommand(_)
+            | Expression::Synthetic(_)
+            | Expression::Literal(Literal::Operator(_))
+            | Expression::Literal(Literal::Bare(_))
+            | Expression::Garbage => Vec::new(),
+        }
+    }
+
+    fn internal_command(&self, internal: &InternalCommand) -> Vec<CompletionLocation> {
+        let mut result = Vec::new();
+
+        match internal.args.head.expr {
+            Expression::Command(_) => {
+                result.push(LocationType::Command.spanned(internal.name_span));
+            }
+            Expression::Literal(Literal::String(_)) => {
+                result.push(LocationType::Command.spanned(internal.name_span));
+            }
+            _ => (),
+        }
+
+        if let Some(positionals) = &internal.args.positional {
+            let mut positionals = positionals.iter();
+
+            if internal.name == "run_external" {
+                if let Some(external_command) = positionals.next() {
+                    result.push(LocationType::Command.spanned(external_command.span));
+                }
+            }
+
+            result.extend(positionals.flat_map(|positional| match positional.expr {
+                Expression::Garbage => {
+                    let garbage = positional.span.slice(self.line);
+                    let location = if garbage.starts_with('-') {
+                        LocationType::Flag(internal.name.clone())
+                    } else {
+                        // TODO we may be able to map this to the name of a positional,
+                        //      but we'll need a signature
+                        LocationType::Argument(Some(internal.name.clone()), None)
+                    };
+
+                    vec![location.spanned(positional.span)]
+                }
+
+                _ => self.expression(positional),
+            }));
+        }
+
+        if let Some(named) = &internal.args.named {
+            for (name, kind) in &named.named {
+                match kind {
+                    NamedValue::PresentSwitch(span) => {
+                        result.push(LocationType::Flag(internal.name.clone()).spanned(*span));
+                    }
+
+                    NamedValue::Value(span, expr) => {
+                        result.push(LocationType::Flag(internal.name.clone()).spanned(*span));
+                        result.append(&mut self.with_flag(name.clone()).expression(expr));
+                    }
+
+                    _ => (),
+                }
+            }
+        }
+
+        result
+    }
+
+    fn pipeline(&self, pipeline: &Commands) -> Vec<CompletionLocation> {
+        let mut result = Vec::new();
+
+        for command in &pipeline.list {
+            match command {
+                ClassifiedCommand::Internal(internal) => {
+                    let engine = self.with_command(internal.name.clone());
+                    result.append(&mut engine.internal_command(internal));
+                }
+
+                ClassifiedCommand::Expr(expr) => result.append(&mut self.expression(expr)),
+                _ => (),
+            }
+        }
+
+        result
+    }
+
+    /// Flattens the block into a Vec of completion locations
+    pub fn completion_locations(&self, block: &Block) -> Vec<CompletionLocation> {
+        block.block.iter().flat_map(|v| self.pipeline(v)).collect()
+    }
+
+    pub fn new(line: &'s str) -> Flatten<'s> {
+        Flatten {
+            line,
+            command: None,
+            flag: None,
+        }
+    }
+
+    pub fn with_command(&self, command: String) -> Flatten<'s> {
+        Flatten {
+            line: self.line,
+            command: Some(command),
+            flag: None,
+        }
+    }
+
+    pub fn with_flag(&self, flag: String) -> Flatten<'s> {
+        Flatten {
+            line: self.line,
+            command: self.command.clone(),
+            flag: Some(flag),
+        }
+    }
+}
+
+/// Characters that precede a command name
+const BEFORE_COMMAND_CHARS: &[char] = &['|', '(', ';'];
+
+/// Determines the completion location for a given block at the given cursor position
+pub fn completion_location(line: &str, block: &Block, pos: usize) -> Option<CompletionLocation> {
+    let completion_engine = Flatten::new(line);
+    let locations = completion_engine.completion_locations(block);
+
+    if locations.is_empty() {
+        Some(LocationType::Command.spanned(Span::unknown()))
+    } else {
+        let mut prev = None;
+        for loc in locations {
+            // We don't use span.contains because we want to include the end. This handles the case
+            // where the cursor is just after the text (i.e., no space between cursor and text)
+            if loc.span.start() <= pos && pos <= loc.span.end() {
+                return Some(loc);
+            } else if pos < loc.span.start() {
+                break;
+            }
+
+            prev = Some(loc);
+        }
+
+        if let Some(prev) = prev {
+            // Cursor is between locations (or at the end). Look at the line to see if the cursor
+            // is after some character that would imply we're in the command position.
+            let start = prev.span.end();
+            if line[start..pos].contains(BEFORE_COMMAND_CHARS) {
+                Some(LocationType::Command.spanned(Span::unknown()))
+            } else {
+                // TODO this should be able to be mapped to a command
+                Some(LocationType::Argument(None, None).spanned(Span::unknown()))
+            }
+        } else {
+            // Cursor is before any possible completion location, so must be a command
+            Some(LocationType::Command.spanned(Span::unknown()))
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use nu_parser::SignatureRegistry;
+    use nu_protocol::{Signature, SyntaxShape};
+
+    #[derive(Clone, Debug)]
+    struct VecRegistry(Vec<Signature>);
+
+    impl From<Vec<Signature>> for VecRegistry {
+        fn from(v: Vec<Signature>) -> Self {
+            VecRegistry(v)
+        }
+    }
+
+    impl SignatureRegistry for VecRegistry {
+        fn has(&self, name: &str) -> bool {
+            self.0.iter().any(|v| &v.name == name)
+        }
+
+        fn get(&self, name: &str) -> Option<nu_protocol::Signature> {
+            self.0.iter().find(|v| &v.name == name).map(Clone::clone)
+        }
+
+        fn clone_box(&self) -> Box<dyn SignatureRegistry> {
+            Box::new(self.clone())
+        }
+    }
+
+    mod completion_location {
+        use super::*;
+
+        use nu_parser::{classify_block, lite_parse, SignatureRegistry};
+
+        fn completion_location(
+            line: &str,
+            registry: &dyn SignatureRegistry,
+            pos: usize,
+        ) -> Option<LocationType> {
+            let lite_block = lite_parse(line, 0).expect("lite_parse");
+            let block = classify_block(&lite_block, registry);
+            super::completion_location(line, &block.block, pos).map(|v| v.item)
+        }
+
+        #[test]
+        fn completes_internal_command_names() {
+            let registry: VecRegistry =
+                vec![Signature::build("echo").rest(SyntaxShape::Any, "the values to echo")].into();
+            let line = "echo 1 | echo 2";
+
+            assert_eq!(
+                completion_location(line, &registry, 10),
+                Some(LocationType::Command),
+            );
+        }
+
+        #[test]
+        fn completes_external_command_names() {
+            let registry: VecRegistry = Vec::new().into();
+            let line = "echo 1 | echo 2";
+
+            assert_eq!(
+                completion_location(line, &registry, 10),
+                Some(LocationType::Command),
+            );
+        }
+
+        #[test]
+        fn completes_command_names_when_cursor_immediately_after_command_name() {
+            let registry: VecRegistry = Vec::new().into();
+            let line = "echo 1 | echo 2";
+
+            assert_eq!(
+                completion_location(line, &registry, 4),
+                Some(LocationType::Command),
+            );
+        }
+
+        #[test]
+        fn completes_variables() {
+            let registry: VecRegistry = Vec::new().into();
+            let line = "echo $nu.env.";
+
+            assert_eq!(
+                completion_location(line, &registry, 13),
+                Some(LocationType::Variable),
+            );
+        }
+
+        #[test]
+        fn completes_flags() {
+            let registry: VecRegistry = vec![Signature::build("du")
+                .switch("recursive", "the values to echo", None)
+                .rest(SyntaxShape::Any, "blah")]
+            .into();
+
+            let line = "du --recurs";
+
+            assert_eq!(
+                completion_location(line, &registry, 7),
+                Some(LocationType::Flag("du".to_string())),
+            );
+        }
+
+        #[test]
+        fn completes_arguments() {
+            let registry: VecRegistry =
+                vec![Signature::build("echo").rest(SyntaxShape::Any, "the values to echo")].into();
+            let line = "echo 1 | echo 2";
+
+            assert_eq!(
+                completion_location(line, &registry, 6),
+                Some(LocationType::Argument(Some("echo".to_string()), None)),
+            );
+        }
+    }
+}

crates/nu-cli/src/completion/flag.rs

@@ -0,0 +1,33 @@
+use crate::completion::{Context, Suggestion};
+use crate::context;
+
+pub struct Completer;
+
+impl Completer {
+    pub fn complete(&self, ctx: &Context<'_>, cmd: String, partial: &str) -> Vec<Suggestion> {
+        let context: &context::Context = ctx.as_ref();
+
+        if let Some(cmd) = context.registry.get_command(&cmd) {
+            let sig = cmd.signature();
+            let mut suggestions = Vec::new();
+            for (name, (named_type, _desc)) in sig.named.iter() {
+                suggestions.push(format!("--{}", name));
+
+                if let Some(c) = named_type.get_short() {
+                    suggestions.push(format!("-{}", c));
+                }
+            }
+
+            suggestions
+                .into_iter()
+                .filter(|v| v.starts_with(partial))
+                .map(|v| Suggestion {
+                    replacement: format!("{} ", v),
+                    display: v,
+                })
+                .collect()
+        } else {
+            Vec::new()
+        }
+    }
+}

crates/nu-cli/src/completion/mod.rs

@@ -1,3 +1,8 @@
+pub(crate) mod command;
+pub(crate) mod engine;
+pub(crate) mod flag;
+pub(crate) mod path;
+
 use nu_errors::ShellError;
 
 use crate::context;
@@ -35,6 +40,4 @@ pub trait Completer {
         pos: usize,
         ctx: &Context<'_>,
     ) -> Result<(usize, Vec<Suggestion>), ShellError>;
-
-    fn hint(&self, line: &str, pos: usize, ctx: &Context<'_>) -> Option<String>;
 }

crates/nu-cli/src/completion/path.rs

@@ -0,0 +1,31 @@
+use rustyline::completion::FilenameCompleter;
+
+use crate::completion::{Context, Suggestion};
+
+pub struct Completer {
+    inner: FilenameCompleter,
+}
+
+impl Completer {
+    pub fn new() -> Completer {
+        Completer {
+            inner: FilenameCompleter::new(),
+        }
+    }
+
+    pub fn complete(&self, _ctx: &Context<'_>, partial: &str) -> Vec<Suggestion> {
+        let expanded = nu_parser::expand_ndots(partial);
+
+        if let Ok((_pos, pairs)) = self.inner.complete_path(&expanded, expanded.len()) {
+            pairs
+                .into_iter()
+                .map(|v| Suggestion {
+                    replacement: v.replacement,
+                    display: v.display,
+                })
+                .collect()
+        } else {
+            Vec::new()
+        }
+    }
+}

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

@@ -1,364 +1,72 @@
-use std::fs::{read_dir, DirEntry};
-
-#[cfg(unix)]
-use std::os::unix::fs::PermissionsExt;
-
-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::completion::{self, Suggestion};
 use crate::context;
-use crate::prelude::*;
-use nu_data::config;
 
-pub(crate) struct NuCompleter {
-    file_completer: FilenameCompleter,
-    hinter: HistoryHinter,
-}
+pub(crate) struct NuCompleter {}
 
-#[derive(PartialEq, Eq, Debug)]
-enum ReplacementLocation {
-    Command,
-    Other,
-}
+impl NuCompleter {}
 
 impl NuCompleter {
-    fn complete_internal(
+    pub fn complete(
         &self,
         line: &str,
         pos: usize,
         context: &completion::Context,
-    ) -> rustyline::Result<(usize, Vec<rustyline::completion::Pair>)> {
-        let line_chars: Vec<_> = line[..pos].chars().collect();
+    ) -> (usize, Vec<Suggestion>) {
+        use crate::completion::engine::LocationType;
 
-        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 = get_matching_arguments(
-                    context.as_ref(),
-                    &lite_block,
-                    &line_chars,
-                    line,
-                    replace_pos,
-                    pos,
-                );
-            } else {
-                completions = self.file_completer.complete(line, pos, context.as_ref())?.1;
-            }
-        } else {
-            completions = self.file_completer.complete(line, pos, context.as_ref())?.1;
-        }
-
-        // 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();
-
-            let complete_from_path = config::config(Tag::unknown())
-                .map(|conf| {
-                    conf.get("complete_from_path")
-                        .map(|v| v.is_true())
-                        .unwrap_or(true)
-                })
-                .unwrap_or(true);
-
-            if complete_from_path {
-                let path_executables = find_path_executables().unwrap_or_default();
-                for path_exe in path_executables {
-                    all_executables.insert(path_exe);
-                }
-            };
-
-            for exe in all_executables.iter() {
-                let mut pos = replace_pos;
-                let mut matched = false;
-                if pos < line_chars.len() {
-                    for chr in exe.chars() {
-                        if line_chars[pos] != chr {
-                            break;
-                        }
-
-                        pos += 1;
-                        if pos == line_chars.len() {
-                            matched = true;
-                            break;
-                        }
-                    }
-                }
-
-                if matched {
-                    completions.push(rustyline::completion::Pair {
-                        display: exe.to_string(),
-                        replacement: exe.to_string(),
-                    });
-                }
-            }
-        }
-
-        // Adjust replacement to deal with a quote already at the cursor. Specifically, if there's
-        // already a quote at the cursor, but the replacement doesn't have one, we need to ensure
-        // one exists (to be safe, even if the completion doesn't need it).
-        for completion in &mut completions {
-            let cursor_char = line.chars().nth(replace_pos);
-            if cursor_char.unwrap_or(' ') == '"' && !completion.replacement.starts_with('"') {
-                completion.replacement.insert(0, '"');
-            }
-        }
-
-        Ok((replace_pos, completions))
-    }
-}
-
-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
+        let nu_context: &context::Context = context.as_ref();
+        let lite_block = match nu_parser::lite_parse(line, 0) {
+            Ok(block) => Some(block),
+            Err(result) => result.partial,
         };
 
-        self.complete_internal(&expanded, pos, context)
-            .map_err(|e| ShellError::untagged_runtime_error(format!("{}", e)))
+        let location = lite_block
+            .map(|block| nu_parser::classify_block(&block, &nu_context.registry))
+            .and_then(|block| {
+                crate::completion::engine::completion_location(line, &block.block, pos)
+            });
+
+        if let Some(location) = location {
+            let partial = location.span.slice(line);
+
+            let suggestions = match location.item {
+                LocationType::Command => {
+                    let command_completer = crate::completion::command::Completer {};
+                    command_completer.complete(context, partial)
+                }
+
+                LocationType::Flag(cmd) => {
+                    let flag_completer = crate::completion::flag::Completer {};
+                    flag_completer.complete(context, cmd, partial)
+                }
+
+                LocationType::Argument(_cmd, _arg_name) => {
+                    // TODO use cmd and arg_name to narrow things down further
+                    let path_completer = crate::completion::path::Completer::new();
+                    path_completer.complete(context, partial)
+                }
+
+                LocationType::Variable => Vec::new(),
+            }
+            .into_iter()
             .map(requote)
-            .map(|(pos, completions)| {
-                (
-                    pos,
-                    completions
-                        .into_iter()
-                        .map(|pair| completion::Suggestion {
-                            display: pair.display,
-                            replacement: pair.replacement,
-                        })
-                        .collect(),
-                )
-            })
-    }
+            .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
-            }
+            (location.span.start(), suggestions)
         } 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);
-                    }
-                }
-            }
+            (pos, Vec::new())
         }
     }
-
-    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;
-    let mut loc = ReplacementLocation::Other;
-    if let Ok(lite_block) = nu_parser::lite_parse(line, 0) {
-        'outer: for pipeline in lite_block.block.iter() {
-            for command in pipeline.commands.iter() {
-                let name_span = command.name.span;
-                if name_span.start() <= pos && name_span.end() >= pos {
-                    replace_pos = name_span.start();
-                    parsed_pos = true;
-                    loc = ReplacementLocation::Command;
-                    break 'outer;
-                }
-
-                for arg in command.args.iter() {
-                    if arg.span.start() <= pos && arg.span.end() >= pos {
-                        replace_pos = arg.span.start();
-                        parsed_pos = true;
-                        break 'outer;
-                    }
-                }
-            }
-        }
-    }
-
-    if !parsed_pos {
-        // If the command won't parse, naively detect the completion start point
-        while replace_pos > 0 {
-            if line_chars[replace_pos - 1] == ' ' {
-                break;
-            }
-            replace_pos -= 1;
-        }
-    }
-
-    (replace_pos, loc)
-}
-
-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 {
+fn requote(item: Suggestion) -> Suggestion {
+    let unescaped = rustyline::completion::unescape(&item.replacement, Some('\\'));
+    if unescaped != item.replacement {
+        Suggestion {
             display: item.display,
-            replacement: format!("{}{}{}", maybe_quote, unescaped, maybe_quote),
-        });
+            replacement: format!("\"{}\"", unescaped),
+        }
+    } else {
+        item
     }
-
-    (items.0, new_items)
 }

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

@@ -282,8 +282,4 @@ impl completion::Completer for HelpShell {
         }
         Ok((replace_pos, completions))
     }
-
-    fn hint(&self, _line: &str, _pos: usize, _ctx: &completion::Context<'_>) -> Option<String> {
-        None
-    }
 }

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

@@ -1,24 +1,27 @@
-use crate::completion::{self, Completer};
-use crate::context::Context;
-use crate::shell::palette::{DefaultPalette, Palette};
+use std::borrow::Cow::{self, Owned};
 
 use ansi_term::{Color, Style};
 use nu_parser::SignatureRegistry;
 use nu_protocol::hir::FlatShape;
 use nu_source::{Spanned, Tag, Tagged};
-use rustyline::hint::Hinter;
-use std::borrow::Cow::{self, Owned};
+
+use crate::completion;
+use crate::context::Context;
+use crate::shell::completer::NuCompleter;
+use crate::shell::palette::{DefaultPalette, Palette};
 
 pub struct Helper {
-    completer: Box<dyn Completer>,
+    completer: NuCompleter,
+    hinter: rustyline::hint::HistoryHinter,
     context: Context,
     pub colored_prompt: String,
 }
 
 impl Helper {
-    pub(crate) fn new(completer: Box<dyn Completer>, context: Context) -> Helper {
+    pub(crate) fn new(context: Context) -> Helper {
         Helper {
-            completer,
+            completer: NuCompleter {},
+            hinter: rustyline::hint::HistoryHinter {},
             context,
             colored_prompt: String::new(),
         }
@@ -45,16 +48,13 @@ impl rustyline::completion::Completer for Helper {
         ctx: &rustyline::Context<'_>,
     ) -> Result<(usize, Vec<Self::Candidate>), rustyline::error::ReadlineError> {
         let ctx = completion::Context::new(&self.context, ctx);
-        self.completer
-            .complete(line, pos, &ctx)
-            .map_err(|_| rustyline::error::ReadlineError::Eof)
+        Ok(self.completer.complete(line, pos, &ctx))
     }
 }
 
-impl Hinter for Helper {
+impl rustyline::hint::Hinter for Helper {
     fn hint(&self, line: &str, pos: usize, ctx: &rustyline::Context<'_>) -> Option<String> {
-        let ctx = completion::Context::new(&self.context, ctx);
-        self.completer.hint(line, pos, &ctx)
+        self.hinter.hint(line, pos, &ctx)
     }
 }
 

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

@@ -307,8 +307,4 @@ impl completion::Completer for ValueShell {
         }
         Ok((replace_pos, completions))
     }
-
-    fn hint(&self, _line: &str, _pos: usize, _context: &completion::Context<'_>) -> Option<String> {
-        None
-    }
 }

crates/nu-parser/src/errors.rs

@@ -4,10 +4,10 @@ use std::fmt::Debug;
 #[derive(Debug)]
 pub struct ParseError<T: Debug> {
     /// An informative cause for this parse error
-    pub(crate) cause: nu_errors::ParseError,
+    pub cause: nu_errors::ParseError,
 
     /// What has been successfully parsed, if anything
-    pub(crate) partial: Option<T>,
+    pub partial: Option<T>,
 }
 
 pub type ParseResult<T> = Result<T, ParseError<T>>;

crates/nu-parser/src/lite_parse.rs

@@ -133,11 +133,6 @@ fn bare(src: &mut Input, span_offset: usize) -> ParseResult<Spanned<String>> {
         // correct information from the non-lite parse.
         bare.push(delimiter);
 
-        let span = Span::new(
-            start_offset + span_offset,
-            start_offset + span_offset + bare.len(),
-        );
-
         return Err(ParseError {
             cause: nu_errors::ParseError::unexpected_eof(delimiter.to_string(), span),
             partial: Some(bare.spanned(span)),

crates/nu-parser/src/parse.rs

@@ -1358,7 +1358,7 @@ fn classify_pipeline(
                     }),
                     positional: Some(args),
                     named: None,
-                    span: Span::unknown(),
+                    span: name_span,
                     external_redirection: if iter.peek().is_none() {
                         ExternalRedirection::None
                     } else {
@@ -1448,7 +1448,7 @@ fn classify_pipeline(
                     }),
                     positional: Some(args),
                     named: None,
-                    span: Span::unknown(),
+                    span: name_span,
                     external_redirection: if iter.peek().is_none() {
                         ExternalRedirection::None
                     } else {

crates/nu-source/src/meta.rs

@@ -526,10 +526,11 @@ impl Span {
     /// let span = Span::new(2, 8);
     ///
     /// assert_eq!(span.contains(5), true);
+    /// assert_eq!(span.contains(8), false);
     /// assert_eq!(span.contains(100), false);
     /// ```
     pub fn contains(&self, pos: usize) -> bool {
-        self.start <= pos && self.end >= pos
+        self.start <= pos && pos < self.end
     }
 
     /// Returns a new Span by merging an earlier Span with the current Span.