Slim down cli plugin logic.

crates/nu-cli/src/cli.rs

@@ -1,8 +1,5 @@
 use crate::commands::classified::block::run_block;
 use crate::commands::classified::maybe_text_codec::{MaybeTextCodec, StringOrBinary};
-use crate::commands::plugin::JsonRpc;
-use crate::commands::plugin::{PluginCommand, PluginSink};
-use crate::commands::whole_stream_command;
 use crate::context::Context;
 use crate::git::current_branch;
 use crate::path::canonicalize;
@@ -12,117 +9,31 @@ use crate::EnvironmentSyncer;
 use futures_codec::FramedRead;
 use nu_errors::{ProximateShellError, ShellDiagnostic, ShellError};
 use nu_protocol::hir::{ClassifiedCommand, Expression, InternalCommand, Literal, NamedArguments};
-use nu_protocol::{Primitive, ReturnSuccess, Signature, UntaggedValue, Value};
+use nu_protocol::{Primitive, ReturnSuccess, UntaggedValue, Value};
-#[allow(unused)]
-use nu_source::Tagged;
 
 use log::{debug, trace};
 use rustyline::config::{ColorMode, CompletionType, Config};
 use rustyline::error::ReadlineError;
 use rustyline::{self, config::Configurer, At, Cmd, Editor, KeyPress, Movement, Word};
 use std::error::Error;
-use std::io::{BufRead, BufReader, Write};
 use std::iter::Iterator;
 use std::path::{Path, PathBuf};
-use std::process::{Child, Command, Stdio};
 use std::sync::atomic::Ordering;
 
-use rayon::prelude::*;
+fn register_plugins(context: &mut Context) -> Result<(), ShellError> {
+    if let Ok(plugins) = crate::plugin::scan() {
+        context.add_commands(
+            plugins
+                .into_iter()
+                .filter(|p| !context.is_command_registered(p.name()))
+                .collect(),
+        );
+    }
 
-fn load_plugin(path: &std::path::Path, context: &mut Context) -> Result<(), ShellError> {
+    Ok(())
-    let ext = path.extension();
-    let ps1_file = match ext {
-        Some(ext) => ext == "ps1",
-        None => false,
-    };
-
-    let mut child: Child = if ps1_file {
-        Command::new("pwsh")
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .args(&[
-                "-NoLogo",
-                "-NoProfile",
-                "-ExecutionPolicy",
-                "Bypass",
-                "-File",
-                &path.to_string_lossy(),
-            ])
-            .spawn()
-            .expect("Failed to spawn PowerShell process")
-    } else {
-        Command::new(path)
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .spawn()
-            .expect("Failed to spawn child process")
-    };
-
-    let stdin = child.stdin.as_mut().expect("Failed to open stdin");
-    let stdout = child.stdout.as_mut().expect("Failed to open stdout");
-
-    let mut reader = BufReader::new(stdout);
-
-    let request = JsonRpc::new("config", Vec::<Value>::new());
-    let request_raw = serde_json::to_string(&request)?;
-    trace!(target: "nu::load", "plugin infrastructure config -> path {:#?}, request {:?}", &path, &request_raw);
-    stdin.write_all(format!("{}\n", request_raw).as_bytes())?;
-    let path = dunce::canonicalize(path)?;
-
-    let mut input = String::new();
-    let result = match reader.read_line(&mut input) {
-        Ok(count) => {
-            trace!(target: "nu::load", "plugin infrastructure -> config response for {:#?}", &path);
-            trace!(target: "nu::load", "plugin infrastructure -> processing response ({} bytes)", count);
-            trace!(target: "nu::load", "plugin infrastructure -> response: {}", input);
-
-            let response = serde_json::from_str::<JsonRpc<Result<Signature, ShellError>>>(&input);
-            match response {
-                Ok(jrpc) => match jrpc.params {
-                    Ok(params) => {
-                        let fname = path.to_string_lossy();
-
-                        trace!(target: "nu::load", "plugin infrastructure -> processing {:?}", params);
-
-                        let name = params.name.clone();
-                        let fname = fname.to_string();
-
-                        if context.get_command(&name).is_some() {
-                            trace!(target: "nu::load", "plugin infrastructure -> {:?} already loaded.", &name);
-                        } else if params.is_filter {
-                            context.add_commands(vec![whole_stream_command(PluginCommand::new(
-                                name, fname, params,
-                            ))]);
-                        } else {
-                            context.add_commands(vec![whole_stream_command(PluginSink::new(
-                                name, fname, params,
-                            ))]);
-                        }
-                        Ok(())
-                    }
-                    Err(e) => Err(e),
-                },
-                Err(e) => {
-                    trace!(target: "nu::load", "plugin infrastructure -> incompatible {:?}", input);
-                    Err(ShellError::untagged_runtime_error(format!(
-                        "Error: {:?}",
-                        e
-                    )))
-                }
-            }
-        }
-        Err(e) => Err(ShellError::untagged_runtime_error(format!(
-            "Error: {:?}",
-            e
-        ))),
-    };
-
-    let _ = child.wait();
-
-    result
 }
 
-fn search_paths() -> Vec<std::path::PathBuf> {
+pub fn search_paths() -> Vec<std::path::PathBuf> {
     use std::env;
 
     let mut search_paths = Vec::new();
@@ -153,78 +64,6 @@ fn search_paths() -> Vec<std::path::PathBuf> {
     search_paths
 }
 
-pub fn load_plugins(context: &mut Context) -> Result<(), ShellError> {
-    let opts = glob::MatchOptions {
-        case_sensitive: false,
-        require_literal_separator: false,
-        require_literal_leading_dot: false,
-    };
-
-    for path in search_paths() {
-        let mut pattern = path.to_path_buf();
-
-        pattern.push(std::path::Path::new("nu_plugin_[a-z0-9][a-z0-9]*"));
-
-        let plugs: Vec<_> = glob::glob_with(&pattern.to_string_lossy(), opts)?
-            .filter_map(|x| x.ok())
-            .collect();
-
-        let _failures: Vec<_> = plugs
-            .par_iter()
-            .map(|path| {
-                let bin_name = {
-                    if let Some(name) = path.file_name() {
-                        name.to_str().unwrap_or("")
-                    } else {
-                        ""
-                    }
-                };
-
-                // allow plugins with extensions on all platforms
-                let is_valid_name = {
-                    bin_name
-                        .chars()
-                        .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.')
-                };
-
-                let is_executable = {
-                    #[cfg(windows)]
-                    {
-                        bin_name.ends_with(".exe") 
-                        || bin_name.ends_with(".bat")
-                        || bin_name.ends_with(".cmd")
-                        || bin_name.ends_with(".py")
-                        || bin_name.ends_with(".ps1")
-                    }
-
-                    #[cfg(not(windows))]
-                    {
-                        !bin_name.contains('.')
-                        || (bin_name.ends_with('.')
-                        || bin_name.ends_with(".py")
-                        || bin_name.ends_with(".rb")
-                        || bin_name.ends_with(".sh")
-                        || bin_name.ends_with(".bash")
-                        || bin_name.ends_with(".zsh")
-                        || bin_name.ends_with(".pl")
-                        || bin_name.ends_with(".awk")
-                        || bin_name.ends_with(".ps1"))
-                    }
-                };
-
-                if is_valid_name && is_executable {
-                    trace!(target: "nu::load", "plugin infrastructure -> Trying {:?}", path.display());
-
-                    // we are ok if this plugin load fails
-                    let _ = load_plugin(&path, &mut context.clone());
-                }
-            })
-            .collect();
-    }
-
-    Ok(())
-}
-
 pub struct History;
 
 impl History {
@@ -489,7 +328,7 @@ pub async fn run_vec_of_pipelines(
     let mut syncer = crate::EnvironmentSyncer::new();
     let mut context = create_default_context(&mut syncer, false)?;
 
-    let _ = crate::load_plugins(&mut context);
+    let _ = register_plugins(&mut context);
 
     #[cfg(feature = "ctrlc")]
     {
@@ -755,7 +594,7 @@ pub async fn cli(
     mut syncer: EnvironmentSyncer,
     mut context: Context,
 ) -> Result<(), Box<dyn Error>> {
-    let _ = load_plugins(&mut context);
+    let _ = register_plugins(&mut context);
 
     let (mut rl, config) = set_rustyline_configuration();
 

crates/nu-cli/src/commands.rs

@@ -79,7 +79,6 @@ pub(crate) mod open;
 pub(crate) mod parse;
 pub(crate) mod path;
 pub(crate) mod pivot;
-pub(crate) mod plugin;
 pub(crate) mod prepend;
 pub(crate) mod prev;
 pub(crate) mod pwd;

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

@@ -4,6 +4,7 @@ pub(crate) mod expr;
 pub(crate) mod external;
 pub(crate) mod internal;
 pub(crate) mod maybe_text_codec;
+pub(crate) mod plugin;
 
 #[allow(unused_imports)]
 pub(crate) use dynamic::Command as DynamicCommand;

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

@@ -1,8 +1,9 @@
-use crate::commands::WholeStreamCommand;
+use crate::commands::command::{whole_stream_command, WholeStreamCommand};
 use crate::prelude::*;
 use derive_new::new;
 use log::trace;
 use nu_errors::ShellError;
+use nu_plugin::jsonrpc::JsonRpc;
 use nu_protocol::{Primitive, ReturnValue, Signature, UntaggedValue, Value};
 use serde::{self, Deserialize, Serialize};
 use std::io::prelude::*;
@@ -11,23 +12,6 @@ use std::io::Write;
 use std::path::Path;
 use std::process::{Child, Command, Stdio};
 
-#[derive(Debug, Serialize, Deserialize)]
-pub struct JsonRpc<T> {
-    jsonrpc: String,
-    pub method: String,
-    pub params: T,
-}
-
-impl<T> JsonRpc<T> {
-    pub fn new<U: Into<String>>(method: U, params: T) -> Self {
-        JsonRpc {
-            jsonrpc: "2.0".into(),
-            method: method.into(),
-            params,
-        }
-    }
-}
-
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(tag = "method")]
 #[allow(non_camel_case_types)]
@@ -37,15 +21,77 @@ pub enum NuResult {
     },
 }
 
+enum PluginCommand {
+    Filter(PluginFilter),
+    Sink(PluginSink),
+}
+
+impl PluginCommand {
+    fn command(self) -> Result<crate::commands::Command, ShellError> {
+        match self {
+            PluginCommand::Filter(cmd) => Ok(whole_stream_command(cmd)),
+            PluginCommand::Sink(cmd) => Ok(whole_stream_command(cmd)),
+        }
+    }
+}
+
+enum PluginMode {
+    Filter,
+    Sink,
+}
+
+pub struct PluginCommandBuilder {
+    mode: PluginMode,
+    name: String,
+    path: String,
+    config: Signature,
+}
+
+impl PluginCommandBuilder {
+    pub fn new(
+        name: impl Into<String>,
+        path: impl Into<String>,
+        config: impl Into<Signature>,
+    ) -> Self {
+        let config = config.into();
+
+        PluginCommandBuilder {
+            mode: if config.is_filter {
+                PluginMode::Filter
+            } else {
+                PluginMode::Sink
+            },
+            name: name.into(),
+            path: path.into(),
+            config,
+        }
+    }
+
+    pub fn build(&self) -> Result<crate::commands::Command, ShellError> {
+        let mode = &self.mode;
+
+        let name = self.name.clone();
+        let path = self.path.clone();
+        let config = self.config.clone();
+
+        let cmd = match mode {
+            PluginMode::Filter => PluginCommand::Filter(PluginFilter { name, path, config }),
+            PluginMode::Sink => PluginCommand::Sink(PluginSink { name, path, config }),
+        };
+
+        cmd.command()
+    }
+}
+
 #[derive(new)]
-pub struct PluginCommand {
+pub struct PluginFilter {
     name: String,
     path: String,
     config: Signature,
 }
 
 #[async_trait]
-impl WholeStreamCommand for PluginCommand {
+impl WholeStreamCommand for PluginFilter {
     fn name(&self) -> &str {
         &self.name
     }
@@ -63,11 +109,11 @@ impl WholeStreamCommand for PluginCommand {
         args: CommandArgs,
         registry: &CommandRegistry,
     ) -> Result<OutputStream, ShellError> {
-        filter_plugin(self.path.clone(), args, registry).await
+        run_filter(self.path.clone(), args, registry).await
     }
 }
 
-pub async fn filter_plugin(
+async fn run_filter(
     path: String,
     args: CommandArgs,
     registry: &CommandRegistry,
@@ -349,11 +395,11 @@ impl WholeStreamCommand for PluginSink {
         args: CommandArgs,
         registry: &CommandRegistry,
     ) -> Result<OutputStream, ShellError> {
-        sink_plugin(self.path.clone(), args, registry).await
+        run_sink(self.path.clone(), args, registry).await
     }
 }
 
-pub async fn sink_plugin(
+async fn run_sink(
     path: String,
     args: CommandArgs,
     registry: &CommandRegistry,

crates/nu-cli/src/context.rs

@@ -228,10 +228,15 @@ impl Context {
         }
     }
 
+    #[allow(unused)]
     pub(crate) fn get_command(&self, name: &str) -> Option<Command> {
         self.registry.get_command(name)
     }
 
+    pub(crate) fn is_command_registered(&self, name: &str) -> bool {
+        self.registry.has(name)
+    }
+
     pub(crate) fn expect_command(&self, name: &str) -> Result<Command, ShellError> {
         self.registry.expect_command(name)
     }

crates/nu-cli/src/lib.rs

@@ -26,6 +26,7 @@ mod futures;
 mod git;
 mod keybinding;
 mod path;
+mod plugin;
 mod shell;
 mod stream;
 pub mod utils;
@@ -34,8 +35,8 @@ pub mod utils;
 mod examples;
 
 pub use crate::cli::{
-    cli, create_default_context, load_plugins, parse_and_eval, process_line,
+    cli, create_default_context, parse_and_eval, process_line, run_pipeline_standalone,
-    run_pipeline_standalone, run_vec_of_pipelines, LineResult,
+    run_vec_of_pipelines, LineResult,
 };
 pub use crate::commands::command::{
     whole_stream_command, CommandArgs, EvaluatedWholeStreamCommandArgs, Example, WholeStreamCommand,

crates/nu-cli/src/plugin.rs

@@ -0,0 +1,170 @@
+use crate::commands::classified::plugin::PluginCommandBuilder;
+use log::trace;
+use nu_errors::ShellError;
+use nu_plugin::jsonrpc::JsonRpc;
+use nu_protocol::{Signature, Value};
+use std::io::{BufRead, BufReader, Write};
+use std::process::{Child, Command, Stdio};
+
+use rayon::prelude::*;
+
+pub fn build_plugin_command(
+    path: &std::path::Path,
+) -> Result<Option<PluginCommandBuilder>, ShellError> {
+    let ext = path.extension();
+    let ps1_file = match ext {
+        Some(ext) => ext == "ps1",
+        None => false,
+    };
+
+    let mut child: Child = if ps1_file {
+        Command::new("pwsh")
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .args(&[
+                "-NoLogo",
+                "-NoProfile",
+                "-ExecutionPolicy",
+                "Bypass",
+                "-File",
+                &path.to_string_lossy(),
+            ])
+            .spawn()
+            .expect("Failed to spawn PowerShell process")
+    } else {
+        Command::new(path)
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .spawn()
+            .expect("Failed to spawn child process")
+    };
+
+    let stdin = child.stdin.as_mut().expect("Failed to open stdin");
+    let stdout = child.stdout.as_mut().expect("Failed to open stdout");
+
+    let mut reader = BufReader::new(stdout);
+
+    let request = JsonRpc::new("config", Vec::<Value>::new());
+    let request_raw = serde_json::to_string(&request)?;
+    trace!(target: "nu::load", "plugin infrastructure config -> path {:#?}, request {:?}", &path, &request_raw);
+    stdin.write_all(format!("{}\n", request_raw).as_bytes())?;
+    let path = dunce::canonicalize(path)?;
+
+    let mut input = String::new();
+    let result = match reader.read_line(&mut input) {
+        Ok(count) => {
+            trace!(target: "nu::load", "plugin infrastructure -> config response for {:#?}", &path);
+            trace!(target: "nu::load", "plugin infrastructure -> processing response ({} bytes)", count);
+            trace!(target: "nu::load", "plugin infrastructure -> response: {}", input);
+
+            let response = serde_json::from_str::<JsonRpc<Result<Signature, ShellError>>>(&input);
+            match response {
+                Ok(jrpc) => match jrpc.params {
+                    Ok(params) => {
+                        let fname = path.to_string_lossy();
+
+                        trace!(target: "nu::load", "plugin infrastructure -> processing {:?}", params);
+
+                        let name = params.name.clone();
+
+                        let fname = fname.to_string();
+
+                        Ok(Some(PluginCommandBuilder::new(&name, &fname, params)))
+                    }
+                    Err(e) => Err(e),
+                },
+                Err(e) => {
+                    trace!(target: "nu::load", "plugin infrastructure -> incompatible {:?}", input);
+                    Err(ShellError::untagged_runtime_error(format!(
+                        "Error: {:?}",
+                        e
+                    )))
+                }
+            }
+        }
+        Err(e) => Err(ShellError::untagged_runtime_error(format!(
+            "Error: {:?}",
+            e
+        ))),
+    };
+
+    let _ = child.wait();
+
+    result
+}
+
+pub fn scan() -> Result<Vec<crate::commands::Command>, ShellError> {
+    let mut plugins = vec![];
+
+    let opts = glob::MatchOptions {
+        case_sensitive: false,
+        require_literal_separator: false,
+        require_literal_leading_dot: false,
+    };
+
+    for path in crate::cli::search_paths() {
+        let mut pattern = path.to_path_buf();
+
+        pattern.push(std::path::Path::new("nu_plugin_[a-z0-9][a-z0-9]*"));
+
+        let plugs: Vec<_> = glob::glob_with(&pattern.to_string_lossy(), opts)?
+            .filter_map(|x| x.ok())
+            .collect();
+
+        let plugs: Vec<_> = plugs
+            .par_iter()
+            .filter_map(|path| {
+                let bin_name = {
+                    if let Some(name) = path.file_name() {
+                        name.to_str().unwrap_or("")
+                    } else {
+                        ""
+                    }
+                };
+
+                // allow plugins with extensions on all platforms
+                let is_valid_name = {
+                    bin_name
+                        .chars()
+                        .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.')
+                };
+
+                let is_executable = {
+                    #[cfg(windows)]
+                    {
+                        bin_name.ends_with(".exe") 
+                        || bin_name.ends_with(".bat")
+                        || bin_name.ends_with(".cmd")
+                        || bin_name.ends_with(".py")
+                        || bin_name.ends_with(".ps1")
+                    }
+
+                    #[cfg(not(windows))]
+                    {
+                        !bin_name.contains('.')
+                        || (bin_name.ends_with('.')
+                        || bin_name.ends_with(".py")
+                        || bin_name.ends_with(".rb")
+                        || bin_name.ends_with(".sh")
+                        || bin_name.ends_with(".bash")
+                        || bin_name.ends_with(".zsh")
+                        || bin_name.ends_with(".pl")
+                        || bin_name.ends_with(".awk")
+                        || bin_name.ends_with(".ps1"))
+                    }
+                };
+
+                if is_valid_name && is_executable {
+                    trace!(target: "nu::load", "plugin infrastructure -> Trying {:?}", path.display());
+                    build_plugin_command(&path).unwrap_or_else(|_| None)
+                } else {
+                    None
+                }
+            }).map(|p| p.build())
+            .filter_map(Result::ok)
+            .collect::<Vec<crate::commands::Command>>();
+        plugins.extend(plugs);
+    }
+
+    Ok(plugins)
+}

crates/nu-plugin/src/jsonrpc.rs

@@ -0,0 +1,41 @@
+use nu_protocol::{outln, CallInfo, Value};
+use serde::{Deserialize, Serialize};
+
+#[derive(Debug, Serialize, Deserialize)]
+pub struct JsonRpc<T> {
+    jsonrpc: String,
+    pub method: String,
+    pub params: T,
+}
+
+impl<T> JsonRpc<T> {
+    pub fn new<U: Into<String>>(method: U, params: T) -> Self {
+        JsonRpc {
+            jsonrpc: "2.0".into(),
+            method: method.into(),
+            params,
+        }
+    }
+}
+
+pub fn send_response<T: Serialize>(result: T) {
+    let response = JsonRpc::new("response", result);
+    let response_raw = serde_json::to_string(&response);
+
+    match response_raw {
+        Ok(response) => outln!("{}", response),
+        Err(err) => outln!("{}", err),
+    }
+}
+
+#[derive(Debug, Serialize, Deserialize)]
+#[serde(tag = "method")]
+#[allow(non_camel_case_types)]
+pub enum NuCommand {
+    config,
+    begin_filter { params: CallInfo },
+    filter { params: Value },
+    end_filter,
+    sink { params: (CallInfo, Vec<Value>) },
+    quit,
+}

crates/nu-plugin/src/lib.rs

@@ -1,4 +1,6 @@
+pub mod jsonrpc;
 mod plugin;
+
 pub mod test_helpers;
 
 pub use crate::plugin::{serve_plugin, Plugin};

crates/nu-plugin/src/plugin.rs

@@ -1,6 +1,6 @@
+use crate::jsonrpc::{send_response, NuCommand};
 use nu_errors::ShellError;
-use nu_protocol::{outln, CallInfo, ReturnValue, Signature, Value};
+use nu_protocol::{CallInfo, ReturnValue, Signature, Value};
-use serde::{Deserialize, Serialize};
 use std::io;
 
 /// The `Plugin` trait defines the API which plugins may use to "hook" into nushell.
@@ -134,40 +134,3 @@ pub fn serve_plugin(plugin: &mut dyn Plugin) {
         }
     }
 }
-
-#[derive(Debug, Serialize, Deserialize)]
-pub struct JsonRpc<T> {
-    jsonrpc: String,
-    pub method: String,
-    pub params: T,
-}
-impl<T> JsonRpc<T> {
-    pub fn new<U: Into<String>>(method: U, params: T) -> Self {
-        JsonRpc {
-            jsonrpc: "2.0".into(),
-            method: method.into(),
-            params,
-        }
-    }
-}
-
-fn send_response<T: Serialize>(result: T) {
-    let response = JsonRpc::new("response", result);
-    let response_raw = serde_json::to_string(&response);
-
-    match response_raw {
-        Ok(response) => outln!("{}", response),
-        Err(err) => outln!("{}", err),
-    }
-}
-#[derive(Debug, Serialize, Deserialize)]
-#[serde(tag = "method")]
-#[allow(non_camel_case_types)]
-pub enum NuCommand {
-    config,
-    begin_filter { params: CallInfo },
-    filter { params: Value },
-    end_filter,
-    sink { params: (CallInfo, Vec<Value>) },
-    quit,
-}