From cf828146060aa891b66d9f63baad8cc3a59b0a2c Mon Sep 17 00:00:00 2001 From: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> Date: Tue, 10 Dec 2024 07:36:05 -0500 Subject: [PATCH] Use const NU_LIB_DIRS in startup (#14549) # Description A slower, gentler alternative to #14531, in that we're just moving one setting *out* of `default_env.nu` in this PR ;-). All this does is transition from using `$env.NU_LIB_DIRS` in the startup config to `const $NU_LIB_DIRS`. Also updates the `sample_env.nu` to reflect the changes. Details: Before: `$env.NU_LIB_DIRS` was unnecessary set both in `main()` and in `default_env.nu` After: `$env.NU_LIB_DIRS` is only set in `main()` Before: `$env.NU_LIB_DIRS` was set to `config-dir/scripts` and `data-dir/completions` After: `$env.NU_LIB_DIRS` is set to an empty list, and `const NU_LIB_DIRS` is set to the directories above Before: Using `--include-path (-I)` would set the `$env.NU_LIB_DIRS` After: Using `--include-path (-I)` sets the constant `$NU_LIB_DIRS` # User-Facing Changes There shouldn't be any breaking changes here. The `$env.NU_LIBS_DIRS` still works for most cases. There are a few areas we need to clean-up to make sure that the const is usable (`nu-check`, et. al.) but they will still work in the meantime with the older `$env` version. # Tests + Formatting * Changed the Type-check on the `$env` version. * Added a type check for the const version. - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting Doc updates --- .../nu-utils/src/default_files/default_env.nu | 5 ---- .../nu-utils/src/default_files/sample_env.nu | 8 +++--- src/main.rs | 28 ++++++++++++++++--- tests/repl/test_env.rs | 12 +++++++- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/crates/nu-utils/src/default_files/default_env.nu b/crates/nu-utils/src/default_files/default_env.nu index cb85e519ba..f4c3f562ff 100644 --- a/crates/nu-utils/src/default_files/default_env.nu +++ b/crates/nu-utils/src/default_files/default_env.nu @@ -47,11 +47,6 @@ $env.ENV_CONVERSIONS = { } } -$env.NU_LIB_DIRS = $env.NU_LIB_DIRS? | default [ - ($nu.default-config-dir | path join 'scripts') # add /scripts - ($nu.data-dir | path join 'completions') # default home for nushell completions -] - $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 23dc925ccd..f44b8e1986 100644 --- a/crates/nu-utils/src/default_files/sample_env.nu +++ b/crates/nu-utils/src/default_files/sample_env.nu @@ -99,17 +99,17 @@ $env.ENV_CONVERSIONS = $env.ENV_CONVERSIONS | merge { # NU_LIB_DIRS # ----------- -# Directories in this environment variable are searched by the +# Directories in this constant are searched by the # `use` and `source` commands. # # By default, the `scripts` subdirectory of the default configuration # directory is included: -$env.NU_LIB_DIRS = [ +const NU_LIB_DIRS = [ ($nu.default-config-dir | path join 'scripts') # add /scripts ($nu.data-dir | path join 'completions') # default home for nushell completions ] -# You can replace (override) or append to this list: -$env.NU_LIB_DIRS ++= ($nu.default-config-dir | path join 'modules') +# You can replace (override) or append to this list by shadowing the constant +const NU_LIB_DIRS = $NU_LIB_DIRS ++ [($nu.default-config-dir | path join 'modules')] # NU_PLUGIN_DIRS # -------------- diff --git a/src/main.rs b/src/main.rs index bef8d07caf..7898988844 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,7 +25,7 @@ use nu_lsp::LanguageServer; use nu_path::canonicalize_with; use nu_protocol::{ engine::EngineState, report_shell_error, ByteStream, Config, IntoValue, PipelineData, - ShellError, Span, Spanned, Value, + ShellError, Span, Spanned, Type, Value, }; use nu_std::load_standard_library; use nu_utils::perf; @@ -147,13 +147,25 @@ fn main() -> Result<()> { let mut default_nu_lib_dirs_path = nushell_config_path.clone(); default_nu_lib_dirs_path.push("scripts"); - engine_state.add_env_var( - "NU_LIB_DIRS".to_string(), + // env.NU_LIB_DIRS to be replaced by constant (below) - Eventual deprecation + // but an empty list for now to allow older code to work + engine_state.add_env_var("NU_LIB_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_LIB_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_lib_dirs_path.to_string_lossy()), Value::test_string(default_nushell_completions_path.to_string_lossy()), ]), ); + engine_state.merge_delta(working_set.render())?; let mut default_nu_plugin_dirs_path = nushell_config_path; default_nu_plugin_dirs_path.push("plugins"); @@ -273,7 +285,15 @@ fn main() -> Result<()> { .map(|x| Value::string(x.trim().to_string(), span)) .collect(); - engine_state.add_env_var("NU_LIB_DIRS".into(), Value::list(vals, span)); + let mut working_set = nu_protocol::engine::StateWorkingSet::new(&engine_state); + let var_id = working_set.add_variable( + b"$NU_LIB_DIRS".into(), + span, + Type::List(Box::new(Type::String)), + false, + ); + working_set.set_variable_const_val(var_id, Value::list(vals, span)); + engine_state.merge_delta(working_set.render())?; } perf!("NU_LIB_DIRS setup", start_time, use_color); diff --git a/tests/repl/test_env.rs b/tests/repl/test_env.rs index 7963f7580b..f0a2070c30 100644 --- a/tests/repl/test_env.rs +++ b/tests/repl/test_env.rs @@ -17,8 +17,18 @@ fn shorthand_env_3() -> TestResult { } #[test] -fn default_nu_lib_dirs_type() { +fn default_nu_lib_dirs_env_type() { + // Previously, this was a list + // While we are transitioning to const NU_LIB_DIRS + // the env version will be empty, and thus a + // list let actual = nu!("$env.NU_LIB_DIRS | describe"); + assert_eq!(actual.out, "list"); +} + +#[test] +fn default_nu_lib_dirs_type() { + let actual = nu!("$NU_LIB_DIRS | describe"); assert_eq!(actual.out, "list"); }