diff --git a/crates/nu-cmd-plugin/src/commands/plugin/add.rs b/crates/nu-cmd-plugin/src/commands/plugin/add.rs index 150b007eeb..99d72b23ab 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/add.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/add.rs @@ -4,7 +4,7 @@ use nu_engine::{command_prelude::*, current_dir}; use nu_plugin::{GetPlugin, PersistentPlugin}; use nu_protocol::{PluginCacheItem, PluginGcConfig, PluginIdentity, RegisteredPlugin}; -use crate::util::modify_plugin_file; +use crate::util::{get_plugin_dirs, modify_plugin_file}; #[derive(Clone)] pub struct PluginAdd; @@ -85,24 +85,10 @@ apparent the next time `nu` is next launched with that plugin cache file. let cwd = current_dir(engine_state, stack)?; // Check the current directory, or fall back to NU_PLUGIN_DIRS - let filename_expanded = match nu_path::canonicalize_with(&filename.item, &cwd) { - Ok(path) => path, - Err(err) => { - // Try to find it in NU_PLUGIN_DIRS first, before giving up - let mut found = None; - if let Some(nu_plugin_dirs) = stack.get_env_var(engine_state, "NU_PLUGIN_DIRS") { - for dir in nu_plugin_dirs.into_list().unwrap_or(vec![]) { - if let Ok(path) = nu_path::canonicalize_with(dir.as_str()?, &cwd) - .and_then(|dir| nu_path::canonicalize_with(&filename.item, dir)) - { - found = Some(path); - break; - } - } - } - found.ok_or(err.into_spanned(filename.span))? - } - }; + let filename_expanded = nu_path::locate_in_dirs(&filename.item, &cwd, || { + get_plugin_dirs(engine_state, stack) + }) + .err_span(filename.span)?; let shell_expanded = shell .as_ref() diff --git a/crates/nu-cmd-plugin/src/commands/plugin/rm.rs b/crates/nu-cmd-plugin/src/commands/plugin/rm.rs index f2ed58bd9b..6c3147e52b 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/rm.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/rm.rs @@ -1,6 +1,6 @@ use nu_engine::command_prelude::*; -use crate::util::modify_plugin_file; +use crate::util::{canonicalize_possible_filename_arg, modify_plugin_file}; #[derive(Clone)] pub struct PluginRm; @@ -28,7 +28,7 @@ impl Command for PluginRm { .required( "name", SyntaxShape::String, - "The name of the plugin to remove (not the filename)", + "The name, or filename, of the plugin to remove", ) .category(Category::Plugin) } @@ -61,6 +61,11 @@ fixed with `plugin add`. description: "Remove the installed signatures for the `inc` plugin.", result: None, }, + Example { + example: "plugin rm ~/.cargo/bin/nu_plugin_inc", + description: "Remove the installed signatures for the plugin with the filename `~/.cargo/bin/nu_plugin_inc`.", + result: None, + }, Example { example: "plugin rm --plugin-config polars.msgpackz polars", description: "Remove the installed signatures for the `polars` plugin from the \"polars.msgpackz\" plugin cache file.", @@ -80,8 +85,19 @@ fixed with `plugin add`. let custom_path = call.get_flag(engine_state, stack, "plugin-config")?; let force = call.has_flag(engine_state, stack, "force")?; + let filename = canonicalize_possible_filename_arg(engine_state, stack, &name.item); + modify_plugin_file(engine_state, stack, call.head, custom_path, |contents| { - if !force && !contents.plugins.iter().any(|p| p.name == name.item) { + if let Some(index) = contents + .plugins + .iter() + .position(|p| p.name == name.item || p.filename == filename) + { + contents.plugins.remove(index); + Ok(()) + } else if force { + Ok(()) + } else { Err(ShellError::GenericError { error: format!("Failed to remove the `{}` plugin", name.item), msg: "couldn't find a plugin with this name in the cache file".into(), @@ -89,9 +105,6 @@ fixed with `plugin add`. help: None, inner: vec![], }) - } else { - contents.remove_plugin(&name.item); - Ok(()) } })?; diff --git a/crates/nu-cmd-plugin/src/commands/plugin/stop.rs b/crates/nu-cmd-plugin/src/commands/plugin/stop.rs index 54f4cdf90e..d74ee59a3f 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/stop.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/stop.rs @@ -1,5 +1,7 @@ use nu_engine::command_prelude::*; +use crate::util::canonicalize_possible_filename_arg; + #[derive(Clone)] pub struct PluginStop; @@ -14,7 +16,7 @@ impl Command for PluginStop { .required( "name", SyntaxShape::String, - "The name of the plugin to stop.", + "The name, or filename, of the plugin to stop", ) .category(Category::Plugin) } @@ -30,6 +32,11 @@ impl Command for PluginStop { description: "Stop the plugin named `inc`.", result: None, }, + Example { + example: "plugin stop ~/.cargo/bin/nu_plugin_inc", + description: "Stop the plugin with the filename `~/.cargo/bin/nu_plugin_inc`.", + result: None, + }, Example { example: "plugin list | each { |p| plugin stop $p.name }", description: "Stop all plugins.", @@ -47,9 +54,12 @@ impl Command for PluginStop { ) -> Result { let name: Spanned = call.req(engine_state, stack, 0)?; + let filename = canonicalize_possible_filename_arg(engine_state, stack, &name.item); + let mut found = false; for plugin in engine_state.plugins() { - if plugin.identity().name() == name.item { + let id = &plugin.identity(); + if id.name() == name.item || id.filename() == filename { plugin.stop()?; found = true; } diff --git a/crates/nu-cmd-plugin/src/commands/plugin/use_.rs b/crates/nu-cmd-plugin/src/commands/plugin/use_.rs index 9959b326ce..a5ff8ae043 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/use_.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/use_.rs @@ -24,7 +24,7 @@ impl Command for PluginUse { .required( "name", SyntaxShape::String, - "The name of the plugin to load (not the filename)", + "The name, or filename, of the plugin to load", ) .category(Category::Plugin) } @@ -41,6 +41,9 @@ preparing a plugin cache file and passing `--plugin-config`, or using the If the plugin was already loaded, this will reload the latest definition from the cache file into scope. + +Note that even if the plugin filename is specified, it will only be loaded if +it was already previously registered with `plugin add`. "# .trim() } @@ -70,6 +73,11 @@ the cache file into scope. example: r#"plugin use query"#, result: None, }, + Example { + description: "Load the commands for the plugin with the filename `~/.cargo/bin/nu_plugin_query` from $nu.plugin-path", + example: r#"plugin use ~/.cargo/bin/nu_plugin_query"#, + result: None, + }, Example { description: "Load the commands for the `query` plugin from a custom plugin cache file", diff --git a/crates/nu-cmd-plugin/src/util.rs b/crates/nu-cmd-plugin/src/util.rs index e24e72eab8..bfbcd5309e 100644 --- a/crates/nu-cmd-plugin/src/util.rs +++ b/crates/nu-cmd-plugin/src/util.rs @@ -1,7 +1,10 @@ -use std::fs::{self, File}; +use std::{ + fs::{self, File}, + path::PathBuf, +}; use nu_engine::{command_prelude::*, current_dir}; -use nu_protocol::PluginCacheFile; +use nu_protocol::{engine::StateWorkingSet, PluginCacheFile}; pub(crate) fn modify_plugin_file( engine_state: &EngineState, @@ -48,3 +51,39 @@ pub(crate) fn modify_plugin_file( Ok(()) } + +pub(crate) fn canonicalize_possible_filename_arg( + engine_state: &EngineState, + stack: &Stack, + arg: &str, +) -> PathBuf { + // This results in the best possible chance of a match with the plugin item + if let Ok(cwd) = nu_engine::current_dir(engine_state, stack) { + let path = nu_path::expand_path_with(arg, &cwd, true); + // Try to canonicalize + nu_path::locate_in_dirs(&path, &cwd, || get_plugin_dirs(engine_state, stack)) + // If we couldn't locate it, return the expanded path alone + .unwrap_or(path) + } else { + arg.into() + } +} + +pub(crate) fn get_plugin_dirs( + engine_state: &EngineState, + stack: &Stack, +) -> impl Iterator { + // Get the NU_PLUGIN_DIRS constant or env var + let working_set = StateWorkingSet::new(engine_state); + let value = working_set + .find_variable(b"$NU_PLUGIN_DIRS") + .and_then(|var_id| working_set.get_constant(var_id).ok().cloned()) + .or_else(|| stack.get_env_var(engine_state, "NU_PLUGIN_DIRS")); + + // Get all of the strings in the list, if possible + value + .into_iter() + .flat_map(|value| value.into_list().ok()) + .flatten() + .flat_map(|list_item| list_item.coerce_into_string().ok()) +} diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 300012807c..314192ef89 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -3803,6 +3803,17 @@ pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box) -> P }) .transpose()?; + // The name could also be a filename, so try our best to expand it for that match. + let filename_query = { + let path = nu_path::expand_path_with(&name.item, &cwd, true); + path.to_str() + .and_then(|path_str| { + find_in_dirs(path_str, working_set, &cwd, Some("NU_PLUGIN_DIRS")) + }) + .map(|parser_path| parser_path.path_buf()) + .unwrap_or(path) + }; + // Find the actual plugin config path location. We don't have a const/env variable for this, // it either lives in the current working directory or in the script's directory let plugin_config_path = if let Some(custom_path) = &plugin_config { @@ -3842,7 +3853,7 @@ pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box) -> P let plugin_item = contents .plugins .iter() - .find(|plugin| plugin.name == name.item) + .find(|plugin| plugin.name == name.item || plugin.filename == filename_query) .ok_or_else(|| ParseError::PluginNotFound { name: name.item.clone(), name_span: name.span, diff --git a/crates/nu-path/src/expansions.rs b/crates/nu-path/src/expansions.rs index f20b149fb8..8e99b84092 100644 --- a/crates/nu-path/src/expansions.rs +++ b/crates/nu-path/src/expansions.rs @@ -92,3 +92,35 @@ where let path = expand_tilde(path); expand_ndots(path) } + +/// Attempts to canonicalize the path against the current directory. Failing that, if +/// the path is relative, it attempts all of the dirs in `dirs`. If that fails, it returns +/// the original error. +pub fn locate_in_dirs( + filename: impl AsRef, + cwd: impl AsRef, + dirs: impl FnOnce() -> I, +) -> std::io::Result +where + I: IntoIterator, + P: AsRef, +{ + let filename = filename.as_ref(); + let cwd = cwd.as_ref(); + match canonicalize_with(filename, cwd) { + Ok(path) => Ok(path), + Err(err) => { + // Try to find it in `dirs` first, before giving up + let mut found = None; + for dir in dirs() { + if let Ok(path) = + canonicalize_with(dir, cwd).and_then(|dir| canonicalize_with(filename, dir)) + { + found = Some(path); + break; + } + } + found.ok_or(err) + } + } +} diff --git a/crates/nu-path/src/lib.rs b/crates/nu-path/src/lib.rs index 63c0091892..93179e42a5 100644 --- a/crates/nu-path/src/lib.rs +++ b/crates/nu-path/src/lib.rs @@ -4,7 +4,7 @@ mod helpers; mod tilde; mod util; -pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path}; +pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path, locate_in_dirs}; pub use helpers::{config_dir, config_dir_old, home_dir}; pub use tilde::expand_tilde; pub use util::trim_trailing_slash; diff --git a/crates/nu-protocol/src/plugin/cache_file/mod.rs b/crates/nu-protocol/src/plugin/cache_file/mod.rs index 58d16f342c..c77b3e5c23 100644 --- a/crates/nu-protocol/src/plugin/cache_file/mod.rs +++ b/crates/nu-protocol/src/plugin/cache_file/mod.rs @@ -96,11 +96,6 @@ impl PluginCacheFile { .sort_by(|item1, item2| item1.name.cmp(&item2.name)); } } - - /// Remove a plugin from the plugin cache file by name. - pub fn remove_plugin(&mut self, name: &str) { - self.plugins.retain_mut(|item| item.name != name) - } } /// A single plugin definition from a [`PluginCacheFile`]. diff --git a/tests/plugin_persistence/mod.rs b/tests/plugin_persistence/mod.rs index 41fb917de1..68939cab19 100644 --- a/tests/plugin_persistence/mod.rs +++ b/tests/plugin_persistence/mod.rs @@ -72,6 +72,17 @@ fn plugin_process_exits_after_stop() { ); } +#[test] +fn plugin_stop_can_find_by_filename() { + let result = nu_with_plugins!( + cwd: ".", + plugin: ("nu_plugin_inc"), + r#"plugin stop (plugin list | where name == inc).0.filename"# + ); + assert!(result.status.success()); + assert!(result.err.is_empty()); +} + #[test] fn plugin_process_exits_when_nushell_exits() { let out = nu_with_plugins!( diff --git a/tests/plugins/cache_file.rs b/tests/plugins/cache_file.rs index a5365fd3f0..d8132955ce 100644 --- a/tests/plugins/cache_file.rs +++ b/tests/plugins/cache_file.rs @@ -38,6 +38,75 @@ fn plugin_add_then_restart_nu() { assert_eq!(r#"["example"]"#, result.out); } +#[test] +fn plugin_add_in_nu_plugin_dirs_const() { + let example_plugin_path = example_plugin_path(); + + let dirname = example_plugin_path.parent().expect("no parent"); + let filename = example_plugin_path + .file_name() + .expect("no file_name") + .to_str() + .expect("not utf-8"); + + let result = nu_with_plugins!( + cwd: ".", + plugins: [], + &format!( + r#" + $env.NU_PLUGIN_DIRS = null + const NU_PLUGIN_DIRS = ['{0}'] + plugin add '{1}' + ( + ^$nu.current-exe + --config $nu.config-path + --env-config $nu.env-path + --plugin-config $nu.plugin-path + --commands 'plugin list | get name | to json --raw' + ) + "#, + dirname.display(), + filename + ) + ); + assert!(result.status.success()); + assert_eq!(r#"["example"]"#, result.out); +} + +#[test] +fn plugin_add_in_nu_plugin_dirs_env() { + let example_plugin_path = example_plugin_path(); + + let dirname = example_plugin_path.parent().expect("no parent"); + let filename = example_plugin_path + .file_name() + .expect("no file_name") + .to_str() + .expect("not utf-8"); + + let result = nu_with_plugins!( + cwd: ".", + plugins: [], + &format!( + r#" + $env.NU_PLUGIN_DIRS = ['{0}'] + plugin add '{1}' + ( + ^$nu.current-exe + --config $nu.config-path + --env-config $nu.env-path + --plugin-config $nu.plugin-path + --commands 'plugin list | get name | to json --raw' + ) + "#, + dirname.display(), + filename + ) + ); + assert!(result.status.success()); + assert_eq!(r#"["example"]"#, result.out); +} + #[test] fn plugin_add_to_custom_path() { let example_plugin_path = example_plugin_path(); @@ -192,6 +261,57 @@ fn plugin_rm_from_custom_path() { }) } +#[test] +fn plugin_rm_using_filename() { + let example_plugin_path = example_plugin_path(); + Playground::setup("plugin rm using filename", |dirs, _playground| { + let file = File::create(dirs.test().join("test-plugin-file.msgpackz")) + .expect("failed to create file"); + let mut contents = PluginCacheFile::new(); + + contents.upsert_plugin(PluginCacheItem { + name: "example".into(), + filename: example_plugin_path.clone(), + shell: None, + data: PluginCacheItemData::Valid { commands: vec![] }, + }); + + contents.upsert_plugin(PluginCacheItem { + name: "foo".into(), + // this doesn't exist, but it should be ok + filename: dirs.test().join("nu_plugin_foo"), + shell: None, + data: PluginCacheItemData::Valid { commands: vec![] }, + }); + + contents + .write_to(file, None) + .expect("failed to write plugin file"); + + let result = nu!( + cwd: dirs.test(), + &format!( + "plugin rm --plugin-config test-plugin-file.msgpackz '{}'", + example_plugin_path.display() + ) + ); + assert!(result.status.success()); + assert!(result.err.trim().is_empty()); + + // Check the contents after running + let contents = PluginCacheFile::read_from( + File::open(dirs.test().join("test-plugin-file.msgpackz")).expect("failed to open file"), + None, + ) + .expect("failed to read file"); + + assert!(!contents.plugins.iter().any(|p| p.name == "example")); + + // Shouldn't remove anything else + assert!(contents.plugins.iter().any(|p| p.name == "foo")); + }) +} + /// Running nu with a test plugin file that fails to parse on one plugin should just cause a warning /// but the others should be loaded #[test] @@ -306,6 +426,27 @@ fn plugin_add_and_then_use() { assert_eq!(r#"["example"]"#, result.out); } +#[test] +fn plugin_add_and_then_use_by_filename() { + let example_plugin_path = example_plugin_path(); + let result = nu_with_plugins!( + cwd: ".", + plugins: [], + &format!(r#" + plugin add '{0}' + ( + ^$nu.current-exe + --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' + ) + "#, example_plugin_path.display()) + ); + assert!(result.status.success()); + assert_eq!(r#"["example"]"#, result.out); +} + #[test] fn plugin_add_then_use_with_custom_path() { let example_plugin_path = example_plugin_path();