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

Let with pipeline #9589

Merged
merged 4 commits into from Jul 3, 2023
Merged

Let with pipeline #9589

merged 4 commits into from Jul 3, 2023

Conversation

sophiajt
Copy link
Member

@sophiajt sophiajt commented Jul 3, 2023

Description

This changes the default behaviour of let to be able to take a pipeline as its initial value.

For example:

> let x = "hello world" | str length

This is a change from the existing behaviour, where the right hand side is assumed to be an expression. Pipelines are more general, and can be more powerful.

My google foo is failing me, but this also fixes this issue:

let x = foo

Currently, this reads foo as a bareword that gets converted to a string rather than running the foo command. In practice, this is really annoying and is a really hard to spot bug in a script.

User-Facing Changes

BREAKING CHANGE BREAKING CHANGE

let gains the power to be assigned via a pipeline. However, this changes the behaviour of let x = foo from assigning the string "foo" to $x to being "run the command foo and give the result to $x"

Tests + Formatting

After Submitting

@sophiajt sophiajt merged commit 5d9e245 into nushell:main Jul 3, 2023
20 checks passed
@sophiajt sophiajt deleted the let_with_pipeline branch July 3, 2023 05:45
@sholderbach sholderbach added the pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes label Jul 3, 2023
amtoine added a commit to amtoine/scripts that referenced this pull request Jul 3, 2023
@hyiltiz
Copy link
Contributor

hyiltiz commented Jul 5, 2023

So we can now let MYPY = /usr/bin/env python3 and then do let result = $MYPY hello.py?

@hyiltiz
Copy link
Contributor

hyiltiz commented Jul 5, 2023

I tried let x = foo and it said it could not find the executable foo in PATH. If we are treating it as a command, shouldn't we only evaluate it at the call site rather than the name assigning site? As in, when I pointed the variable x to foo, the foo may not yet exist but I could call $x later in an environment where foo is well-defined. Maybe my expectation for the behavior is different here. I wonder if I am a special case for getting confused here.

@amtoine
Copy link
Member

amtoine commented Jul 6, 2023

So we can now let MYPY = /usr/bin/env python3 and then do let result = $MYPY hello.py?

you should be able to do that yup 😊

I tried let x = foo and it said it could not find the executable foo in PATH. If we are treating it as a command, shouldn't we only evaluate it at the call site rather than the name assigning site? As in, when I pointed the variable x to foo, the foo may not yet exist but I could call $x later in an environment where foo is well-defined. Maybe my expectation for the behavior is different here. I wonder if I am a special case for getting confused here.

this might require some lazy evaluation of commands, right?
it's something we do not have now i think 🤔

@hyiltiz
Copy link
Contributor

hyiltiz commented Jul 6, 2023

The lazy evaluation semantic is present in closures, i.e. let x = { foo } does not throw an error upon binding, but only during execution, i.e. when do $x is called. I am asking should the engine treat let x = foo with lazy evaluation semantics?

What is a pipeline? I am finding it confusing to think of pipelines as more general than expressions.

sophiajt added a commit that referenced this pull request Jul 11, 2023
# Description

This extends the syntax fix for `let` (#9589) to `mut` as well.

Example: `mut x = "hello world" | str length; print $x`

closes #9634

# User-Facing Changes

`mut` now joins `let` in being able to be assigned from a pipeline

# 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 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.
-->
amtoine added a commit to nushell/nupm that referenced this pull request Jul 28, 2023
related to nushell/nushell#9589.

because the new syntax is imo better, i propose to remove parentheses in
simple `let` variable declarations, e.g.
```nushell
let package_file = ($path | path join "package.nuon")
```
becomes
```nushell
let package_file = $path | path join "package.nuon"
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants