$env.config now always holds a record with only valid values (#7309)

# Description
Closes #7059. Rather than generate a new Record each time $env.config is
accessed (as described in that issue), instead `$env.config = ` now A)
parses the input record, then B) un-parses it into a clean Record with
only the valid values, and stores that as an env-var. The reasoning for
this is that I believe `config_to_nu_record()` (the method that performs
step B) will be useful in later PRs. (See below)

As a result, this also "fixes" the following "bug":
```
〉$env.config = 'butts'
$env.config is not a record
〉$env.config
butts
```
~~Instead, `$env.config = 'butts'` now turns `$env.config` into the
default (not the default config.nu, but `Config::default()`, which
notably has empty keybindings, color_config, menus and hooks vecs).~~

This doesn't attempt to fix #7110. cc @Kangaxx-0

# Example of new behaviour

OLD:
```
〉$env.config = ($env.config | merge { foo: 1 })
$env.config.foo is an unknown config setting
〉$env.config.foo
1
```
NEW:
```
〉$env.config = ($env.config | merge { foo: 1 })
Error:
  × Config record contains invalid values or unknown settings

Error:
  × Error while applying config changes
   ╭─[entry #1:1:1]
 1 │ $env.config = ($env.config | merge { foo: 1 })
   ·                                           ┬
   ·                                           ╰── $env.config.foo is an unknown config setting
   ╰────
  help: This value has been removed from your $env.config record.

〉$env.config.foo
Error: nu:🐚:column_not_found (link)

  × Cannot find column
   ╭─[entry #1:1:1]
 1 │ $env.config = ($env.config | merge { foo: 1 })
   ·                              ──┬──
   ·                                ╰── value originates here
   ╰────
   ╭─[entry #2:1:1]
 1 │ $env.config.foo
   ·             ─┬─
   ·              ╰── cannot find column 'foo'
   ╰────
```
# Example of new errors

OLD:
```
$env.config.cd.baz is an unknown config setting
$env.config.foo is an unknown config setting
$env.config.bar is an unknown config setting
$env.config.table.qux is an unknown config setting
$env.config.history.qux is an unknown config setting
```
NEW:
```
Error: 
  × Config record contains invalid values or unknown settings

Error:
  × Error while applying config changes
     ╭─[C:\Users\Leon\AppData\Roaming\nushell\config.nu:267:1]
 267 │     abbreviations: true # allows `cd s/o/f` to expand to `cd some/other/folder`
 268 │     baz: 3,
     ·          ┬
     ·          ╰── $env.config.cd.baz is an unknown config setting
 269 │   }
     ╰────
  help: This value has been removed from your $env.config record.

Error:
  × Error while applying config changes
     ╭─[C:\Users\Leon\AppData\Roaming\nushell\config.nu:269:1]
 269 │   }
 270 │   foo: 1,
     ·        ┬
     ·        ╰── $env.config.foo is an unknown config setting
 271 │   bar: 2,
     ╰────
  help: This value has been removed from your $env.config record.

Error:
  × Error while applying config changes
     ╭─[C:\Users\Leon\AppData\Roaming\nushell\config.nu:270:1]
 270 │   foo: 1,
 271 │   bar: 2,
     ·        ┬
     ·        ╰── $env.config.bar is an unknown config setting
     ╰────
  help: This value has been removed from your $env.config record.

Error:
  × Error while applying config changes
     ╭─[C:\Users\Leon\AppData\Roaming\nushell\config.nu:279:1]
 279 │     }
 280 │     qux: 4,
     ·          ┬
     ·          ╰── $env.config.table.qux is an unknown config setting
 281 │   }
     ╰────
  help: This value has been removed from your $env.config record.

Error:
  × Error while applying config changes
     ╭─[C:\Users\Leon\AppData\Roaming\nushell\config.nu:285:1]
 285 │     file_format: "plaintext" # "sqlite" or "plaintext"
 286 │  qux: 2
     ·       ┬
     ·       ╰── $env.config.history.qux is an unknown config setting
 287 │   }
     ╰────
  help: This value has been removed from your $env.config record.


```

# User-Facing Changes

See above.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
This commit is contained in:
Leon 2022-12-10 23:34:46 +10:00 committed by GitHub
parent f0e93c2fa9
commit ce78817f41
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 827 additions and 313 deletions

2
Cargo.lock generated
View file

@ -2776,6 +2776,8 @@ dependencies = [
"indexmap",
"miette",
"nu-json",
"nu-path",
"nu-test-support",
"nu-utils",
"num-format",
"serde",

View file

@ -12,6 +12,8 @@ version = "0.72.2"
[dependencies]
nu-utils = { path = "../nu-utils", version = "0.72.2" }
nu-json = { path = "../nu-json", version = "0.72.2" }
nu-path = { path = "../nu-path", version = "0.72.2" } # Used for test support
nu-test-support = { path = "../nu-test-support", version = "0.72.2" }
byte-unit = "4.0.9"
chrono = { version="0.4.23", features= ["serde", "std"], default-features = false }

File diff suppressed because it is too large Load diff

View file

@ -236,10 +236,18 @@ impl EngineState {
// Updating existing overlay
for (k, v) in env.drain() {
if k == "config" {
self.config = v.clone().into_config().unwrap_or_default();
// Don't insert the record as the "config" env var as-is.
// Instead, mutate a clone of it with into_config(), and put THAT in env_vars.
let mut new_record = v.clone();
let (config, error) = new_record.into_config(&self.config);
self.config = config;
env_vars.insert(k, new_record);
if let Some(e) = error {
return Err(e);
}
} else {
env_vars.insert(k, v);
}
env_vars.insert(k, v);
}
} else {
// Pushing a new overlay

View file

@ -1315,6 +1315,24 @@ impl Value {
Value::Bool { val, span }
}
pub fn record(cols: Vec<String>, vals: Vec<Value>, span: Span) -> Value {
Value::Record { cols, vals, span }
}
pub fn record_from_hashmap(map: &HashMap<String, Value>, span: Span) -> Value {
let mut cols = vec![];
let mut vals = vec![];
for (key, val) in map.iter() {
cols.push(key.clone());
vals.push(val.clone());
}
Value::record(cols, vals, span)
}
pub fn list(vals: Vec<Value>, span: Span) -> Value {
Value::List { vals, span }
}
/// Note: Only use this for test data, *not* live data, as it will point into unknown source
/// when used in errors.
pub fn test_string(s: impl Into<String>) -> Value {

View file

@ -0,0 +1,101 @@
use nu_test_support::{nu, nu_repl_code};
#[test]
fn config_is_mutable() {
let actual = nu!(cwd: ".", nu_repl_code(&[r"let-env config = { ls: { clickable_links: true } }",
"$env.config.ls.clickable_links = false;",
"$env.config.ls.clickable_links"]));
assert_eq!(actual.out, "false");
}
#[test]
fn config_preserved_after_do() {
let actual = nu!(cwd: ".", nu_repl_code(&[r"let-env config = { ls: { clickable_links: true } }",
"do -i { $env.config.ls.clickable_links = false }",
"$env.config.ls.clickable_links"]));
assert_eq!(actual.out, "true");
}
#[test]
fn config_affected_when_mutated() {
let actual = nu!(cwd: ".", nu_repl_code(&[r#"let-env config = { filesize: { metric: false, format:"auto" } }"#,
r#"$env.config = { filesize: { metric: true, format:"auto" } }"#,
"20mib | into string"]));
assert_eq!(actual.out, "21.0 MB");
}
#[test]
fn config_affected_when_deep_mutated() {
let actual = nu!(cwd: "crates/nu-utils/src/sample_config", nu_repl_code(&[
r#"source default_config.nu"#,
r#"$env.config.filesize.metric = true"#,
r#"20mib | into string"#]));
assert_eq!(actual.out, "21.0 MB");
}
#[test]
fn config_add_unsupported_key() {
let actual = nu!(cwd: "crates/nu-utils/src/sample_config", nu_repl_code(&[
r#"source default_config.nu"#,
r#"$env.config.foo = 2"#,
r#";"#]));
assert!(actual
.err
.contains("$env.config.foo is an unknown config setting"));
}
#[test]
fn config_add_unsupported_type() {
let actual = nu!(cwd: "crates/nu-utils/src/sample_config", nu_repl_code(&[r#"source default_config.nu"#,
r#"$env.config.ls = '' "#,
r#";"#]));
assert!(actual.err.contains("should be a record"));
}
#[test]
fn config_add_unsupported_value() {
let actual = nu!(cwd: "crates/nu-utils/src/sample_config", nu_repl_code(&[r#"source default_config.nu"#,
r#"$env.config.history.file_format = ''"#,
r#";"#]));
assert!(actual.err.contains(
"unrecognized $env.config.history.file_format ''; expected either 'sqlite' or 'plaintext'"
));
}
#[test]
#[ignore = "Figure out how to make test_bins::nu_repl() continue execution after shell errors"]
fn config_unsupported_key_reverted() {
let actual = nu!(cwd: "crates/nu-utils/src/sample_config", nu_repl_code(&[r#"source default_config.nu"#,
r#"$env.config.foo = 1"#,
r#"'foo' in $env.config"#]));
assert_eq!(actual.out, "false");
}
#[test]
#[ignore = "Figure out how to make test_bins::nu_repl() continue execution after shell errors"]
fn config_unsupported_type_reverted() {
let actual = nu!(cwd: "crates/nu-utils/src/sample_config", nu_repl_code(&[r#" source default_config.nu"#,
r#"$env.config.ls = ''"#,
r#"$env.config.ls | describe"#]));
assert_eq!(actual.out, "record");
}
#[test]
#[ignore = "Figure out how to make test_bins::nu_repl() continue execution after errors"]
fn config_unsupported_value_reverted() {
let actual = nu!(cwd: "crates/nu-utils/src/sample_config", nu_repl_code(&[r#" source default_config.nu"#,
r#"$env.config.history.file_format = 'plaintext'"#,
r#"$env.config.history.file_format = ''"#,
r#"$env.config.history.file_format | to json"#]));
assert_eq!(actual.out, "\"plaintext\"");
}