From 6535ae3d6e3053c2ca95389e7c55911ff125c724 Mon Sep 17 00:00:00 2001
From: Jason Gedge <jason.gedge@shopify.com>
Date: Sat, 29 Aug 2020 23:31:42 -0400
Subject: [PATCH] Show directories and executable for command completion.
 (#2446)

* Show directories and executable for command completion.

Previously we chose from two sets for completing the command position:

1. internal commands, and
2. executables relative to the PATH environment variable.

We now also show directories/executables that match the relative/absolute path
that has been partially typed.

* Fix for Windows
---
 crates/nu-cli/src/completion/command.rs | 43 +++++++++++++++++--------
 crates/nu-cli/src/shell/completer.rs    |  2 +-
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/crates/nu-cli/src/completion/command.rs b/crates/nu-cli/src/completion/command.rs
index c9ca2e045..077baee05 100644
--- a/crates/nu-cli/src/completion/command.rs
+++ b/crates/nu-cli/src/completion/command.rs
@@ -1,5 +1,5 @@
-use std::fs::{read_dir, DirEntry};
 use std::iter::FromIterator;
+use std::path::Path;
 
 use indexmap::set::IndexSet;
 
@@ -13,22 +13,41 @@ impl Completer {
         let context: &context::Context = ctx.as_ref();
         let mut commands: IndexSet<String> = IndexSet::from_iter(context.registry.names());
 
+        // Command suggestions can come from three possible sets:
+        //   1. internal command names,
+        //   2. external command names relative to PATH env var, and
+        //   3. any other executable (that matches what's been typed so far).
+
         let path_executables = find_path_executables().unwrap_or_default();
 
         // TODO quote these, if necessary
         commands.extend(path_executables.into_iter());
 
-        commands
+        let mut suggestions: Vec<_> = commands
             .into_iter()
             .filter(|v| v.starts_with(partial))
             .map(|v| Suggestion {
                 replacement: format!("{} ", v),
                 display: v,
             })
-            .collect()
+            .collect();
+
+        if partial != "" {
+            let path_completer = crate::completion::path::Completer::new();
+            let path_results = path_completer.complete(ctx, partial);
+            suggestions.extend(path_results.into_iter().filter(|suggestion| {
+                let path = Path::new(&suggestion.replacement);
+                path.is_dir() || is_executable(&path)
+            }));
+        }
+
+        suggestions
     }
 }
 
+// TODO create a struct for "is executable" and store this information in it so we don't recompute
+//      on every dir entry
+
 #[cfg(windows)]
 fn pathext() -> Option<Vec<String>> {
     std::env::var_os("PATHEXT").map(|v| {
@@ -41,8 +60,8 @@ fn pathext() -> Option<Vec<String>> {
 }
 
 #[cfg(windows)]
-fn is_executable(file: &DirEntry) -> bool {
-    if let Ok(metadata) = file.metadata() {
+fn is_executable(path: &Path) -> bool {
+    if let Ok(metadata) = path.metadata() {
         let file_type = metadata.file_type();
 
         // If the entry isn't a file, it cannot be executable
@@ -50,7 +69,7 @@ fn is_executable(file: &DirEntry) -> bool {
             return false;
         }
 
-        if let Some(extension) = file.path().extension() {
+        if let Some(extension) = path.extension() {
             if let Some(exts) = pathext() {
                 exts.iter()
                     .any(|ext| extension.to_string_lossy().eq_ignore_ascii_case(ext))
@@ -66,17 +85,15 @@ fn is_executable(file: &DirEntry) -> bool {
 }
 
 #[cfg(target_arch = "wasm32")]
-fn is_executable(_file: &DirEntry) -> bool {
+fn is_executable(_path: &Path) -> bool {
     false
 }
 
 #[cfg(unix)]
-fn is_executable(file: &DirEntry) -> bool {
+fn is_executable(path: &Path) -> bool {
     use std::os::unix::fs::PermissionsExt;
 
-    let metadata = file.metadata();
-
-    if let Ok(metadata) = metadata {
+    if let Ok(metadata) = path.metadata() {
         let filetype = metadata.file_type();
         let permissions = metadata.permissions();
 
@@ -95,9 +112,9 @@ fn find_path_executables() -> Option<IndexSet<String>> {
 
     let mut executables: IndexSet<String> = IndexSet::new();
     for path in paths {
-        if let Ok(mut contents) = read_dir(path) {
+        if let Ok(mut contents) = std::fs::read_dir(path) {
             while let Some(Ok(item)) = contents.next() {
-                if is_executable(&item) {
+                if is_executable(&item.path()) {
                     if let Ok(name) = item.file_name().into_string() {
                         executables.insert(name);
                     }
diff --git a/crates/nu-cli/src/shell/completer.rs b/crates/nu-cli/src/shell/completer.rs
index 25551efad..46fda9447 100644
--- a/crates/nu-cli/src/shell/completer.rs
+++ b/crates/nu-cli/src/shell/completer.rs
@@ -36,7 +36,7 @@ impl NuCompleter {
                     let partial = location.span.slice(line);
                     match location.item {
                         LocationType::Command => {
-                            let command_completer = crate::completion::command::Completer {};
+                            let command_completer = crate::completion::command::Completer;
                             command_completer.complete(context, partial)
                         }