Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Nushell env update syntax #587

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

sophiajt
Copy link
Contributor

Greetings!

The Nushell team is planning to move away from let-env FOO = ... to the more straightforward (and more honest) $env.FOO = ....

This PR should update zoxide to use the supported environment update syntax.

@ajeetdsouza
Copy link
Owner

Thank you!

@ajeetdsouza ajeetdsouza merged commit 786519a into ajeetdsouza:main Jun 30, 2023
4 checks passed
sophiajt added a commit to nushell/nushell that referenced this pull request Jun 30, 2023
# 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: atuinsh/atuin#1080)
- ❌
[zoxide](https://github.com/ajeetdsouza/zoxide/blob/main/templates/nushell.txt)
(PR: ajeetdsouza/zoxide#587)
- ✔️
[oh-my-posh](https://github.com/JanDeDobbeleer/oh-my-posh/blob/main/src/shell/scripts/omp.nu)
(pr: JanDeDobbeleer/oh-my-posh#4011)
@happysalada
Copy link

@ajeetdsouza would it be possible to make a release with this fix applied ?
for package maintainers, we either have to manually apply this patch or wait for a release.
Both are not a problem, I just thought I might ask anyway.

@sophiajt sophiajt deleted the remove_let_env branch August 1, 2023 16:11
@ajeetdsouza
Copy link
Owner

Just saw that this was officially removed in 0.83.1. I've been swamped with work, but I'll try to cut a release this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants