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

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `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

@@ -135,18 +135,24 @@ pub(crate) fn get_plugin_dirs(
     engine_state: &EngineState,
     stack: &Stack,
 ) -> impl Iterator<Item = String> {
-    // 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)
 }

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 <nushell-config-dir>/plugins
-]

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 <nushell-config-dir>/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

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

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<string>
+    // While we are transitioning to const NU_PLUGIN_DIRS
+    // the env version will be empty, and thus a
+    // list<any>
     let actual = nu!("$env.NU_PLUGIN_DIRS | describe");
+    assert_eq!(actual.out, "list<any>");
+}
+
+#[test]
+fn default_nu_plugin_dirs_type() {
+    let actual = nu!("$NU_PLUGIN_DIRS | describe");
     assert_eq!(actual.out, "list<string>");
 }