From 0872e9c3ae4876db433002a17d4742978dd0fcd9 Mon Sep 17 00:00:00 2001 From: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:41:06 -0500 Subject: [PATCH] Allow both NU_PLUGIN_DIRS const and env at the same time (#14553) # Description Fix #14544 and is also the reciprocal of #14549. Before: If both a const and env `NU_PLUGIN_DIRS` were defined at the same time, the env paths would not be used. After: The directories from `const NU_PLUGIN_DIRS` are searched for a matching filename, and if not found, `$env.NU_PLUGIN_DIRS` directories will be searched. Before: `$env.NU_PLUGIN_DIRS` was unnecessary set both in main() and in default_env.nu After: `$env.NU_PLUGIN_DIRS` is only set in main() Before: `$env.NU_PLUGIN_DIRS` was set to `plugins` in the config directory After: `$env.NU_PLUGIN_DIRS` is set to an empty list and `const NU_PLUGIN_DIRS` is set to the directory above. Also updates `sample_env.nu` to use the `const` # User-Facing Changes Most scenarios should work just fine as there continues to be an `$env.NU_PLUGIN_DIRS` to append to or override. However, there is a small chance of a breaking change if someone was *querying* the old default `$env.NU_PLUGIN_DIRS`. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` Also updated the `env` tests and added one for the `const`. # After Submitting Config doc updates --- crates/nu-cmd-plugin/src/util.rs | 22 ++++++++++++------- .../nu-utils/src/default_files/default_env.nu | 4 ---- .../nu-utils/src/default_files/sample_env.nu | 6 +++-- src/main.rs | 13 +++++++++-- tests/repl/test_env.rs | 12 +++++++++- 5 files changed, 40 insertions(+), 17 deletions(-) diff --git a/crates/nu-cmd-plugin/src/util.rs b/crates/nu-cmd-plugin/src/util.rs index 80d1a766b4..57de225ace 100644 --- a/crates/nu-cmd-plugin/src/util.rs +++ b/crates/nu-cmd-plugin/src/util.rs @@ -135,18 +135,24 @@ pub(crate) fn get_plugin_dirs( engine_state: &EngineState, stack: &Stack, ) -> impl Iterator { - // Get the NU_PLUGIN_DIRS constant or env var + // Get the NU_PLUGIN_DIRS from the constant and/or env var let working_set = StateWorkingSet::new(engine_state); - let value = working_set + let dirs_from_const = working_set .find_variable(b"$NU_PLUGIN_DIRS") .and_then(|var_id| working_set.get_constant(var_id).ok()) - .or_else(|| stack.get_env_var(engine_state, "NU_PLUGIN_DIRS")) - .cloned(); // TODO: avoid this clone - - // Get all of the strings in the list, if possible - value + .cloned() // TODO: avoid this clone .into_iter() .flat_map(|value| value.into_list().ok()) .flatten() - .flat_map(|list_item| list_item.coerce_into_string().ok()) + .flat_map(|list_item| list_item.coerce_into_string().ok()); + + let dirs_from_env = stack + .get_env_var(engine_state, "NU_PLUGIN_DIRS") + .cloned() // TODO: avoid this clone + .into_iter() + .flat_map(|value| value.into_list().ok()) + .flatten() + .flat_map(|list_item| list_item.coerce_into_string().ok()); + + dirs_from_const.chain(dirs_from_env) } diff --git a/crates/nu-utils/src/default_files/default_env.nu b/crates/nu-utils/src/default_files/default_env.nu index f4c3f562ff..2e2d66f0bc 100644 --- a/crates/nu-utils/src/default_files/default_env.nu +++ b/crates/nu-utils/src/default_files/default_env.nu @@ -46,7 +46,3 @@ $env.ENV_CONVERSIONS = { to_string: { |v| $v | path expand --no-symlink | str join (char esep) } } } - -$env.NU_PLUGIN_DIRS = $env.NU_PLUGIN_DIRS | default [ - ($nu.default-config-dir | path join 'plugins') # add /plugins -] diff --git a/crates/nu-utils/src/default_files/sample_env.nu b/crates/nu-utils/src/default_files/sample_env.nu index f44b8e1986..ea323c6c22 100644 --- a/crates/nu-utils/src/default_files/sample_env.nu +++ b/crates/nu-utils/src/default_files/sample_env.nu @@ -113,13 +113,15 @@ const NU_LIB_DIRS = $NU_LIB_DIRS ++ [($nu.default-config-dir | path join 'module # NU_PLUGIN_DIRS # -------------- -# Directories to search for plugin binaries when calling register. +# Directories to search for plugin binaries when calling add. # By default, the `plugins` subdirectory of the default configuration # directory is included: -$env.NU_PLUGIN_DIRS = [ +const NU_PLUGIN_DIRS = [ ($nu.default-config-dir | path join 'plugins') # add /plugins ] +# You can replace (override) or append to this list by shadowing the constant +const NU_PLUGIN_DIRS = $NU_PLUGIN_DIRS ++ [($nu.default-config-dir | path join 'plugins')] # Appending to the OS path is a common configuration task. # Because of the previous ENV_CONVERSIONS (performed internally diff --git a/src/main.rs b/src/main.rs index 7898988844..1c82e58851 100644 --- a/src/main.rs +++ b/src/main.rs @@ -169,12 +169,21 @@ fn main() -> Result<()> { let mut default_nu_plugin_dirs_path = nushell_config_path; default_nu_plugin_dirs_path.push("plugins"); - engine_state.add_env_var( - "NU_PLUGIN_DIRS".to_string(), + engine_state.add_env_var("NU_PLUGIN_DIRS".to_string(), Value::test_list(vec![])); + let mut working_set = nu_protocol::engine::StateWorkingSet::new(&engine_state); + let var_id = working_set.add_variable( + b"$NU_PLUGIN_DIRS".into(), + Span::unknown(), + Type::List(Box::new(Type::String)), + false, + ); + working_set.set_variable_const_val( + var_id, Value::test_list(vec![Value::test_string( default_nu_plugin_dirs_path.to_string_lossy(), )]), ); + engine_state.merge_delta(working_set.render())?; // End: Default NU_LIB_DIRS, NU_PLUGIN_DIRS // This is the real secret sauce to having an in-memory sqlite db. You must diff --git a/tests/repl/test_env.rs b/tests/repl/test_env.rs index f0a2070c30..d540ce2fd4 100644 --- a/tests/repl/test_env.rs +++ b/tests/repl/test_env.rs @@ -33,7 +33,17 @@ fn default_nu_lib_dirs_type() { } #[test] -fn default_nu_plugin_dirs_type() { +fn default_nu_plugin_dirs_env_type() { + // Previously, this was a list + // While we are transitioning to const NU_PLUGIN_DIRS + // the env version will be empty, and thus a + // list let actual = nu!("$env.NU_PLUGIN_DIRS | describe"); + assert_eq!(actual.out, "list"); +} + +#[test] +fn default_nu_plugin_dirs_type() { + let actual = nu!("$NU_PLUGIN_DIRS | describe"); assert_eq!(actual.out, "list"); }