mirror of
https://github.com/nushell/nushell
synced 2024-12-30 15:03:25 +00:00
4 commits
Author | SHA1 | Message | Date | |
---|---|---|---|---|
Stefan Holderbach
|
86cd387439
|
Refactor and fix Config <->Value mechanism (#10896)
# Description Our config exists both as a `Config` struct for internal consumption and as a `Value`. The latter is exposed through `$env.config` and can be both set and read. Thus we have a complex bug-prone mechanism, that reads a `Value` and then tries to plug anything where the value is unrepresentable in `Config` with the correct state from `Config`. The parsing involves therefore mutation of the `Value` in a nested `Record` structure. Previously this was wholy done manually, with indices. To enable deletion for example, things had to be iterated over from the back. Also things were indexed in a bunch of places. This was hard to read and an invitation for bugs. With #10876 we can now use `Record::retain_mut` to traverse the records, modify anything that needs fixing, and drop invalid fields. # Parts: - Error messages now consistently use the correct spans pointing to the problematic value and the paths displayed in some messages are also aligned with the keys used for lookup. - Reconstruction of values has been fixed for: - `table.padding` - `buffer_editor` - `hooks.command_not_found` - `datetime_format` (partial solution) - Fix validation of `table.padding` input so value is not set (and underflows `usize` causing `table` to run forever with negative values) - New proper types for settings. Fully validated enums instead of strings: - `config.edit_mode` -> `EditMode` - Don't fall back to vi-mode on invalid string - `config.table.mode` -> `TableMode` - there is still a fall back to `rounded` if given an invalid `TableMode` as argument to the `nu` binary - `config.completions.algorithm` -> `CompletionAlgorithm` - `config.error_style` -> `ErrorStyle` - don't implicitly fall back to `fancy` when given an invalid value. - This should also shrink the size of `Config` as instead of 4x24 bytes those fields now need only 4x1 bytes in `Config` - Completely removed macros relying on the scope of `Value::into_config` so we can break it up into smaller parts in the future. - Factored everything into smaller files with the types and helpers for particular topics. - `NuCursorShape` now explicitly expresses the `Inherit` setting. conversion to option only happens at the interface to `reedline` |
||
Stefan Holderbach
|
656f707a0b
|
Clean up tests containing unnecessary cwd: tokens (#9692)
# Description The working directory doesn't have to be set for those tests (or would be the default anyways). When appropriate also remove calls to the `pipeline()` function. In most places kept the diff minimal and only removed the superfluous part to not pollute the blame view. With simpler tests also simplified things to make them more readable overall (this included removal of the raw string literal). Work for #8670 |
||
JT
|
4af24363c2
|
remove let-env, focus on mutating $env (#9574)
# Description For years, Nushell has used `let-env` to set a single environment variable. As our work on scoping continued, we refined what it meant for a variable to be in scope using `let` but never updated how `let-env` would work. Instead, `let-env` confusingly created mutations to the command's copy of `$env`. So, to help fix the mental model and point people to the right way of thinking about what changing the environment means, this PR removes `let-env` to encourage people to think of it as updating the command's environment variable via mutation. Before: ``` let-env FOO = "BAR" ``` Now: ``` $env.FOO = "BAR" ``` It's also a good reminder that the environment owned by the command is in the `$env` variable rather than global like it is in other shells. # User-Facing Changes BREAKING CHANGE BREAKING CHANGE This completely removes `let-env FOO = "BAR"` so that we can focus on `$env.FOO = "BAR"`. # 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 -A clippy::result_large_err` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # After / Before Submitting integration scripts to update: - ✔️ [starship](https://github.com/starship/starship/blob/master/src/init/starship.nu) - ✔️ [virtualenv](https://github.com/pypa/virtualenv/blob/main/src/virtualenv/activation/nushell/activate.nu) - ✔️ [atuin](https://github.com/ellie/atuin/blob/main/atuin/src/shell/atuin.nu) (PR: https://github.com/ellie/atuin/pull/1080) - ❌ [zoxide](https://github.com/ajeetdsouza/zoxide/blob/main/templates/nushell.txt) (PR: https://github.com/ajeetdsouza/zoxide/pull/587) - ✔️ [oh-my-posh](https://github.com/JanDeDobbeleer/oh-my-posh/blob/main/src/shell/scripts/omp.nu) (pr: https://github.com/JanDeDobbeleer/oh-my-posh/pull/4011) |
||
Leon
|
ce78817f41
|
$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. |