diff --git a/crates/nu-cmd-plugin/src/commands/plugin/add.rs b/crates/nu-cmd-plugin/src/commands/plugin/add.rs index c12f486668..c603ddbb8c 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/add.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/add.rs @@ -119,7 +119,7 @@ apparent the next time `nu` is next launched with that plugin registry file. let metadata = interface.get_metadata()?; let commands = interface.get_signature()?; - modify_plugin_file(engine_state, stack, call.head, custom_path, |contents| { + modify_plugin_file(engine_state, stack, call.head, &custom_path, |contents| { // Update the file with the received metadata and signatures let item = PluginRegistryItem::new(plugin.identity(), metadata, commands); contents.upsert_plugin(item); diff --git a/crates/nu-cmd-plugin/src/commands/plugin/list.rs b/crates/nu-cmd-plugin/src/commands/plugin/list.rs index 3fbf55b23c..367a532ef3 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/list.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/list.rs @@ -1,5 +1,8 @@ -use itertools::Itertools; +use itertools::{EitherOrBoth, Itertools}; use nu_engine::command_prelude::*; +use nu_protocol::{IntoValue, PluginRegistryItemData}; + +use crate::util::read_plugin_file; #[derive(Clone)] pub struct PluginList; @@ -17,7 +20,7 @@ impl Command for PluginList { [ ("name".into(), Type::String), ("version".into(), Type::String), - ("is_running".into(), Type::Bool), + ("status".into(), Type::String), ("pid".into(), Type::Int), ("filename".into(), Type::String), ("shell".into(), Type::String), @@ -26,11 +29,54 @@ impl Command for PluginList { .into(), ), ) + .named( + "plugin-config", + SyntaxShape::Filepath, + "Use a plugin registry file other than the one set in `$nu.plugin-path`", + None, + ) + .switch( + "engine", + "Show info for plugins that are loaded into the engine only.", + Some('e'), + ) + .switch( + "registry", + "Show info for plugins from the registry file only.", + Some('r'), + ) .category(Category::Plugin) } fn description(&self) -> &str { - "List installed plugins." + "List loaded and installed plugins." + } + + fn extra_description(&self) -> &str { + r#" +The `status` column will contain one of the following values: + +- `added`: The plugin is present in the plugin registry file, but not in + the engine. +- `loaded`: The plugin is present both in the plugin registry file and in + the engine, but is not running. +- `running`: The plugin is currently running, and the `pid` column should + contain its process ID. +- `modified`: The plugin state present in the plugin registry file is different + from the state in the engine. +- `removed`: The plugin is still loaded in the engine, but is not present in + the plugin registry file. +- `invalid`: The data in the plugin registry file couldn't be deserialized, + and the plugin most likely needs to be added again. + +`running` takes priority over any other status. Unless `--registry` is used +or the plugin has not been loaded yet, the values of `version`, `filename`, +`shell`, and `commands` reflect the values in the engine and not the ones in +the plugin registry file. + +See also: `plugin use` +"# + .trim() } fn search_terms(&self) -> Vec<&str> { @@ -45,7 +91,7 @@ impl Command for PluginList { result: Some(Value::test_list(vec![Value::test_record(record! { "name" => Value::test_string("inc"), "version" => Value::test_string(env!("CARGO_PKG_VERSION")), - "is_running" => Value::test_bool(true), + "status" => Value::test_string("running"), "pid" => Value::test_int(106480), "filename" => if cfg!(windows) { Value::test_string(r"C:\nu\plugins\nu_plugin_inc.exe") @@ -67,58 +113,189 @@ impl Command for PluginList { fn run( &self, engine_state: &EngineState, - _stack: &mut Stack, + stack: &mut Stack, call: &Call, _input: PipelineData, ) -> Result { - let head = call.head; + let custom_path = call.get_flag(engine_state, stack, "plugin-config")?; + let engine_mode = call.has_flag(engine_state, stack, "engine")?; + let registry_mode = call.has_flag(engine_state, stack, "registry")?; - // Group plugin decls by plugin identity - let decls = engine_state.plugin_decls().into_group_map_by(|decl| { - decl.plugin_identity() - .expect("plugin decl should have identity") - }); + let plugins_info = match (engine_mode, registry_mode) { + // --engine and --registry together is equivalent to the default. + (false, false) | (true, true) => { + if engine_state.plugin_path.is_some() || custom_path.is_some() { + let plugins_in_engine = get_plugins_in_engine(engine_state); + let plugins_in_registry = + get_plugins_in_registry(engine_state, stack, call.head, &custom_path)?; + merge_plugin_info(plugins_in_engine, plugins_in_registry) + } else { + // Don't produce error when running nu --no-config-file + get_plugins_in_engine(engine_state) + } + } + (true, false) => get_plugins_in_engine(engine_state), + (false, true) => get_plugins_in_registry(engine_state, stack, call.head, &custom_path)?, + }; - // Build plugins list - let list = engine_state.plugins().iter().map(|plugin| { - // Find commands that belong to the plugin - let commands = decls.get(plugin.identity()) - .into_iter() - .flat_map(|decls| { - decls.iter().map(|decl| Value::string(decl.name(), head)) - }) - .collect(); - - let pid = plugin - .pid() - .map(|p| Value::int(p as i64, head)) - .unwrap_or(Value::nothing(head)); - - let shell = plugin - .identity() - .shell() - .map(|s| Value::string(s.to_string_lossy(), head)) - .unwrap_or(Value::nothing(head)); - - let metadata = plugin.metadata(); - let version = metadata - .and_then(|m| m.version) - .map(|s| Value::string(s, head)) - .unwrap_or(Value::nothing(head)); - - let record = record! { - "name" => Value::string(plugin.identity().name(), head), - "version" => version, - "is_running" => Value::bool(plugin.is_running(), head), - "pid" => pid, - "filename" => Value::string(plugin.identity().filename().to_string_lossy(), head), - "shell" => shell, - "commands" => Value::list(commands, head), - }; - - Value::record(record, head) - }).collect(); - - Ok(Value::list(list, head).into_pipeline_data()) + Ok(plugins_info.into_value(call.head).into_pipeline_data()) + } +} + +#[derive(Debug, Clone, IntoValue, PartialOrd, Ord, PartialEq, Eq)] +struct PluginInfo { + name: String, + version: Option, + status: PluginStatus, + pid: Option, + filename: String, + shell: Option, + commands: Vec, +} + +#[derive(Debug, Clone, Copy, IntoValue, PartialOrd, Ord, PartialEq, Eq)] +#[nu_value(rename_all = "snake_case")] +enum PluginStatus { + Added, + Loaded, + Running, + Modified, + Removed, + Invalid, +} + +fn get_plugins_in_engine(engine_state: &EngineState) -> Vec { + // Group plugin decls by plugin identity + let decls = engine_state.plugin_decls().into_group_map_by(|decl| { + decl.plugin_identity() + .expect("plugin decl should have identity") + }); + + // Build plugins list + engine_state + .plugins() + .iter() + .map(|plugin| { + // Find commands that belong to the plugin + let commands = decls + .get(plugin.identity()) + .into_iter() + .flat_map(|decls| decls.iter().map(|decl| decl.name().to_owned())) + .sorted() + .collect(); + + PluginInfo { + name: plugin.identity().name().into(), + version: plugin.metadata().and_then(|m| m.version), + status: if plugin.pid().is_some() { + PluginStatus::Running + } else { + PluginStatus::Loaded + }, + pid: plugin.pid(), + filename: plugin.identity().filename().to_string_lossy().into_owned(), + shell: plugin + .identity() + .shell() + .map(|path| path.to_string_lossy().into_owned()), + commands, + } + }) + .sorted() + .collect() +} + +fn get_plugins_in_registry( + engine_state: &EngineState, + stack: &mut Stack, + span: Span, + custom_path: &Option>, +) -> Result, ShellError> { + let plugin_file_contents = read_plugin_file(engine_state, stack, span, custom_path)?; + + let plugins_info = plugin_file_contents + .plugins + .into_iter() + .map(|plugin| { + let mut info = PluginInfo { + name: plugin.name, + version: None, + status: PluginStatus::Added, + pid: None, + filename: plugin.filename.to_string_lossy().into_owned(), + shell: plugin.shell.map(|path| path.to_string_lossy().into_owned()), + commands: vec![], + }; + + if let PluginRegistryItemData::Valid { metadata, commands } = plugin.data { + info.version = metadata.version; + info.commands = commands + .into_iter() + .map(|command| command.sig.name) + .sorted() + .collect(); + } else { + info.status = PluginStatus::Invalid; + } + info + }) + .sorted() + .collect(); + + Ok(plugins_info) +} + +/// If no options are provided, the command loads from both the plugin list in the engine and what's +/// in the registry file. We need to reconcile the two to set the proper states and make sure that +/// new plugins that were added to the plugin registry file show up. +fn merge_plugin_info( + from_engine: Vec, + from_registry: Vec, +) -> Vec { + from_engine + .into_iter() + .merge_join_by(from_registry, |info_a, info_b| { + info_a.name.cmp(&info_b.name) + }) + .map(|either_or_both| match either_or_both { + // Exists in the engine, but not in the registry file + EitherOrBoth::Left(info) => PluginInfo { + status: match info.status { + PluginStatus::Running => info.status, + // The plugin is not in the registry file, so it should be marked as `removed` + _ => PluginStatus::Removed, + }, + ..info + }, + // Exists in the registry file, but not in the engine + EitherOrBoth::Right(info) => info, + // Exists in both + EitherOrBoth::Both(info_engine, info_registry) => PluginInfo { + status: match (info_engine.status, info_registry.status) { + // Above all, `running` should be displayed if the plugin is running + (PluginStatus::Running, _) => PluginStatus::Running, + // `invalid` takes precedence over other states because the user probably wants + // to fix it + (_, PluginStatus::Invalid) => PluginStatus::Invalid, + // Display `modified` if the state in the registry is different somehow + _ if info_engine.is_modified(&info_registry) => PluginStatus::Modified, + // Otherwise, `loaded` (it's not running) + _ => PluginStatus::Loaded, + }, + ..info_engine + }, + }) + .sorted() + .collect() +} + +impl PluginInfo { + /// True if the plugin info shows some kind of change (other than status/pid) relative to the + /// other + fn is_modified(&self, other: &PluginInfo) -> bool { + self.name != other.name + || self.filename != other.filename + || self.shell != other.shell + || self.commands != other.commands } } diff --git a/crates/nu-cmd-plugin/src/commands/plugin/rm.rs b/crates/nu-cmd-plugin/src/commands/plugin/rm.rs index 33557836c0..a82b41524a 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/rm.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/rm.rs @@ -87,7 +87,7 @@ fixed with `plugin add`. let filename = canonicalize_possible_filename_arg(engine_state, stack, &name.item); - modify_plugin_file(engine_state, stack, call.head, custom_path, |contents| { + modify_plugin_file(engine_state, stack, call.head, &custom_path, |contents| { if let Some(index) = contents .plugins .iter() diff --git a/crates/nu-cmd-plugin/src/util.rs b/crates/nu-cmd-plugin/src/util.rs index 6808a74a3c..80d1a766b4 100644 --- a/crates/nu-cmd-plugin/src/util.rs +++ b/crates/nu-cmd-plugin/src/util.rs @@ -6,18 +6,17 @@ use std::{ path::PathBuf, }; -pub(crate) fn modify_plugin_file( +fn get_plugin_registry_file_path( engine_state: &EngineState, stack: &mut Stack, span: Span, - custom_path: Option>, - operate: impl FnOnce(&mut PluginRegistryFile) -> Result<(), ShellError>, -) -> Result<(), ShellError> { + custom_path: &Option>, +) -> Result { #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; - let plugin_registry_file_path = if let Some(ref custom_path) = custom_path { - nu_path::expand_path_with(&custom_path.item, cwd, true) + if let Some(ref custom_path) = custom_path { + Ok(nu_path::expand_path_with(&custom_path.item, cwd, true)) } else { engine_state .plugin_path @@ -28,8 +27,53 @@ pub(crate) fn modify_plugin_file( span: Some(span), help: Some("you may be running `nu` with --no-config-file".into()), inner: vec![], - })? - }; + }) + } +} + +pub(crate) fn read_plugin_file( + engine_state: &EngineState, + stack: &mut Stack, + span: Span, + custom_path: &Option>, +) -> Result { + let plugin_registry_file_path = + get_plugin_registry_file_path(engine_state, stack, span, custom_path)?; + + let file_span = custom_path.as_ref().map(|p| p.span).unwrap_or(span); + + // Try to read the plugin file if it exists + if fs::metadata(&plugin_registry_file_path).is_ok_and(|m| m.len() > 0) { + PluginRegistryFile::read_from( + File::open(&plugin_registry_file_path).map_err(|err| ShellError::IOErrorSpanned { + msg: format!( + "failed to read `{}`: {}", + plugin_registry_file_path.display(), + err + ), + span: file_span, + })?, + Some(file_span), + ) + } else if let Some(path) = custom_path { + Err(ShellError::FileNotFound { + file: path.item.clone(), + span: path.span, + }) + } else { + Ok(PluginRegistryFile::default()) + } +} + +pub(crate) fn modify_plugin_file( + engine_state: &EngineState, + stack: &mut Stack, + span: Span, + custom_path: &Option>, + operate: impl FnOnce(&mut PluginRegistryFile) -> Result<(), ShellError>, +) -> Result<(), ShellError> { + let plugin_registry_file_path = + get_plugin_registry_file_path(engine_state, stack, span, custom_path)?; let file_span = custom_path.as_ref().map(|p| p.span).unwrap_or(span); diff --git a/tests/plugin_persistence/mod.rs b/tests/plugin_persistence/mod.rs index 6729c657c7..587e4d480f 100644 --- a/tests/plugin_persistence/mod.rs +++ b/tests/plugin_persistence/mod.rs @@ -12,7 +12,7 @@ fn plugin_list_shows_installed_plugins() { plugins: [("nu_plugin_inc"), ("nu_plugin_custom_values")], r#"(plugin list).name | str join ','"# ); - assert_eq!("inc,custom_values", out.out); + assert_eq!("custom_values,inc", out.out); assert!(out.status.success()); } @@ -34,15 +34,15 @@ fn plugin_keeps_running_after_calling_it() { plugin: ("nu_plugin_inc"), r#" plugin stop inc - (plugin list).0.is_running | print + (plugin list).0.status == running | print print ";" "2.0.0" | inc -m | ignore - (plugin list).0.is_running | print + (plugin list).0.status == running | print "# ); assert_eq!( "false;true", out.out, - "plugin list didn't show is_running = true" + "plugin list didn't show status = running" ); assert!(out.status.success()); } @@ -244,7 +244,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_immediately() { $env.config.plugin_gc = { default: { stop_after: 0sec } } "2.3.0" | inc -M sleep 100ms - (plugin list | where name == inc).0.is_running + (plugin list | where name == inc).0.status == running "# ); assert!(out.status.success()); @@ -261,7 +261,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_immediately() { } "2.3.0" | inc -M sleep 100ms - (plugin list | where name == inc).0.is_running + (plugin list | where name == inc).0.status == running "# ); assert!(out.status.success()); @@ -281,7 +281,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_after_delay() { while $cond { sleep 100ms $cond = ( - (plugin list | where name == inc).0.is_running and + (plugin list | where name == inc).0.status == running and ((date now) - $start) < 5sec ) } @@ -310,7 +310,7 @@ fn plugin_gc_can_be_configured_to_stop_plugins_after_delay() { while $cond { sleep 100ms $cond = ( - (plugin list | where name == inc).0.is_running and + (plugin list | where name == inc).0.status == running and ((date now) - $start) < 5sec ) } @@ -333,7 +333,7 @@ fn plugin_gc_can_be_configured_as_disabled() { r#" $env.config.plugin_gc = { default: { enabled: false, stop_after: 0sec } } "2.3.0" | inc -M - (plugin list | where name == inc).0.is_running + (plugin list | where name == inc).0.status == running "# ); assert!(out.status.success()); @@ -350,7 +350,7 @@ fn plugin_gc_can_be_configured_as_disabled() { } } "2.3.0" | inc -M - (plugin list | where name == inc).0.is_running + (plugin list | where name == inc).0.status == running "# ); assert!(out.status.success()); @@ -367,7 +367,7 @@ fn plugin_gc_can_be_disabled_by_plugin() { $env.config.plugin_gc = { default: { stop_after: 0sec } } example one 1 foo | ignore # ensure we've run the plugin with the new config sleep 100ms - (plugin list | where name == example).0.is_running + (plugin list | where name == example).0.status == running "# ); assert!(out.status.success()); diff --git a/tests/plugins/registry_file.rs b/tests/plugins/registry_file.rs index 2cb1473b6f..a2ca8735f0 100644 --- a/tests/plugins/registry_file.rs +++ b/tests/plugins/registry_file.rs @@ -37,7 +37,7 @@ fn plugin_add_then_restart_nu() { --config $nu.config-path --env-config $nu.env-path --plugin-config $nu.plugin-path - --commands 'plugin list | get name | to json --raw' + --commands 'plugin list --engine | get name | to json --raw' ) ", example_plugin_path().display()) ); @@ -69,7 +69,7 @@ fn plugin_add_in_nu_plugin_dirs_const() { --config $nu.config-path --env-config $nu.env-path --plugin-config $nu.plugin-path - --commands 'plugin list | get name | to json --raw' + --commands 'plugin list --engine | get name | to json --raw' ) "#, dirname.display(), @@ -103,7 +103,7 @@ fn plugin_add_in_nu_plugin_dirs_env() { --config $nu.config-path --env-config $nu.env-path --plugin-config $nu.plugin-path - --commands 'plugin list | get name | to json --raw' + --commands 'plugin list --engine | get name | to json --raw' ) "#, dirname.display(), @@ -199,7 +199,7 @@ fn plugin_rm_then_restart_nu() { "--plugin-config", "test-plugin-file.msgpackz", "--commands", - "plugin list | get name | to json --raw", + "plugin list --engine | get name | to json --raw", ]) .assert() .success() @@ -364,7 +364,7 @@ fn warning_on_invalid_plugin_item() { "--plugin-config", "test-plugin-file.msgpackz", "--commands", - "plugin list | get name | to json --raw", + "plugin list --engine | get name | to json --raw", ]) .output() .expect("failed to run nu"); @@ -412,6 +412,43 @@ fn plugin_use_error_not_found() { }) } +#[test] +fn plugin_shows_up_in_default_plugin_list_after_add() { + let example_plugin_path = example_plugin_path(); + let result = nu_with_plugins!( + cwd: ".", + plugins: [], + &format!(r#" + plugin add '{}' + plugin list | get status | to json --raw + "#, example_plugin_path.display()) + ); + assert!(result.status.success()); + assert_eq!(r#"["added"]"#, result.out); +} + +#[test] +fn plugin_shows_removed_after_removing() { + let example_plugin_path = example_plugin_path(); + let result = nu_with_plugins!( + cwd: ".", + plugins: [], + &format!(r#" + plugin add '{}' + plugin list | get status | to json --raw + ( + ^$nu.current-exe + --config $nu.config-path + --env-config $nu.env-path + --plugin-config $nu.plugin-path + --commands 'plugin rm example; plugin list | get status | to json --raw' + ) + "#, example_plugin_path.display()) + ); + assert!(result.status.success()); + assert_eq!(r#"["removed"]"#, result.out); +} + #[test] fn plugin_add_and_then_use() { let example_plugin_path = example_plugin_path(); @@ -425,7 +462,7 @@ fn plugin_add_and_then_use() { --config $nu.config-path --env-config $nu.env-path --plugin-config $nu.plugin-path - --commands 'plugin use example; plugin list | get name | to json --raw' + --commands 'plugin use example; plugin list --engine | get name | to json --raw' ) "#, example_plugin_path.display()) ); @@ -446,7 +483,7 @@ fn plugin_add_and_then_use_by_filename() { --config $nu.config-path --env-config $nu.env-path --plugin-config $nu.plugin-path - --commands 'plugin use '{0}'; plugin list | get name | to json --raw' + --commands 'plugin use '{0}'; plugin list --engine | get name | to json --raw' ) "#, example_plugin_path.display()) ); @@ -471,7 +508,7 @@ fn plugin_add_then_use_with_custom_path() { cwd: dirs.test(), r#" plugin use --plugin-config test-plugin-file.msgpackz example - plugin list | get name | to json --raw + plugin list --engine | get name | to json --raw "# );