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

remove the $nothing variable #10478

Merged
merged 4 commits into from
Sep 26, 2023
Merged

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Sep 23, 2023

related to

thanks to @jntrnr and their super useful tips on this PR, i learned about the parser + evaluation, so 🙏

Description

because we already have null as the value of the type nothing and as a followup to the two other attempts of mine, i propose to remove the redundant $nothing built-in variable 😋

this PR is the first step, deprecating $nothing.
a followup PR will remove it altogether and wait for 0.87 👍

⚙️ details: a new NOTHING_VARIABLE_ID = 3 has been added, parsing $nothing will create it, finally a Value::Nothing will be produced and a warning will be reported.

this PR already fixes the toolkit.nu module so that it does not throw a bunch of warnings each time 👌

User-Facing Changes

$nothing is now deprecated and will be removed in 0.87

> $nothing
Error:   × Deprecated variable
   ╭─[entry #1:1:1]
 1  $nothing
   · ────┬───
   ·     ╰── `$nothing` is deprecated and will be removed in 0.87.
   ╰────
  help: Use `null` instead

Tests + Formatting

tests have been updated, especially

  • nothing_fails_string
  • nothing_fails_int
    which use a variable called nil now to make sure nothing does not support cell paths 👍

After Submitting

classic deprecation mention 👍

`NOTHING_VARIABLE_ID` is now 3.
this changes the tests and the toolkit, i.e. all mentions to `$nothing`
except for those of the deprecation itself.
@amtoine amtoine added pr:release-note-mention Addition/Improvement to be mentioned in the release notes deprecation Related to the deprecation of commands labels Sep 23, 2023
Copy link
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for tackling this with deprecation.

I would say let's mark this as draft until we have made sure the important integrations are up to date as we can update them already to null

crates/nu-protocol/src/engine/engine_state.rs Show resolved Hide resolved
@sholderbach
Copy link
Member

Went through the list of integrations that advertise in https://github.com/nushell/nushell#officially-supported-by with the admittedly imperfect github search.

Only found https://github.com/atuinsh/atuin/blob/fbed2862fda127b747718e4ae6f5f36a56f29a51/atuin/src/shell/atuin.nu to use... null. So I think we should be good, if we have nu_scripts updated.

Copy link
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $nothing deprecation looks good to me.

We could make the changing of the "magic" variable IDs a bit more robust in the future by either doing everything with an enum (a bit more hassle) or having a LAST_MAGIC_VARIABLE_ID const that is used for those checks. But not a blocker for this PR.

sholderbach added a commit to sholderbach/nu_scripts that referenced this pull request Sep 26, 2023
@amtoine
Copy link
Member Author

amtoine commented Sep 26, 2023

let's go with this, bye $nothing, enjoy your last release 😏

@amtoine amtoine merged commit 6c02624 into nushell:main Sep 26, 2023
19 checks passed
@amtoine amtoine deleted the remove-nothing-variable branch September 26, 2023 16:49
sholderbach added a commit to nushell/nu_scripts that referenced this pull request Sep 26, 2023
`$nothing` will be deprecated in nu 0.86

This accompanies
- nushell/nushell#10478
amtoine added a commit that referenced this pull request Oct 19, 2023
related to 
- #10478

# Description
this PR is the followup removal to
#10478.

# User-Facing Changes
`$nothing` is now an undefined variable, unless define by the user.
```nushell
> $nothing
Error: nu::parser::variable_not_found

  × Variable not found.
   ╭─[entry #1:1:1]
 1 │ $nothing
   · ────┬───
   ·     ╰── variable not found.
   ╰────
```

# Tests + Formatting

# After Submitting
mention that in release notes
gaetschwartz pushed a commit to gaetschwartz/nushell that referenced this pull request Oct 20, 2023
related to 
- nushell#10478

# Description
this PR is the followup removal to
nushell#10478.

# User-Facing Changes
`$nothing` is now an undefined variable, unless define by the user.
```nushell
> $nothing
Error: nu::parser::variable_not_found

  × Variable not found.
   ╭─[entry nushell#1:1:1]
 1 │ $nothing
   · ────┬───
   ·     ╰── variable not found.
   ╰────
```

# Tests + Formatting

# After Submitting
mention that in release notes
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
related to 
- nushell#9973
- nushell#9918

thanks to @jntrnr and their super useful tips on this PR, i learned
about the parser + evaluation, so 🙏

# Description
because we already have `null` as the value of the type `nothing` and as
a followup to the two other attempts of mine, i propose to remove the
redundant `$nothing` built-in variable 😋

this PR is the first step, deprecating `$nothing`.
a followup PR will remove it altogether and wait for 0.87 👍 

⚙️ **details**: a new `NOTHING_VARIABLE_ID = 3` has been added,
parsing `$nothing` will create it, finally a `Value::Nothing` will be
produced and a warning will be reported.

this PR already fixes the `toolkit.nu` module so that it does not throw
a bunch of warnings each time 👌

# User-Facing Changes
`$nothing` is now deprecated and will be removed in 0.87
```nushell
> $nothing
Error:   × Deprecated variable
   ╭─[entry #1:1:1]
 1 │ $nothing
   · ────┬───
   ·     ╰── `$nothing` is deprecated and will be removed in 0.87.
   ╰────
  help: Use `null` instead
```

# Tests + Formatting
tests have been updated, especially
- `nothing_fails_string`
- `nothing_fails_int`
which use a variable called `nil` now to make sure `nothing` does not
support cell paths 👍

# After Submitting
classic deprecation mention 👍
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
related to 
- nushell#10478

# Description
this PR is the followup removal to
nushell#10478.

# User-Facing Changes
`$nothing` is now an undefined variable, unless define by the user.
```nushell
> $nothing
Error: nu::parser::variable_not_found

  × Variable not found.
   ╭─[entry #1:1:1]
 1 │ $nothing
   · ────┬───
   ·     ╰── variable not found.
   ╰────
```

# Tests + Formatting

# After Submitting
mention that in release notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Related to the deprecation of commands pr:release-note-mention Addition/Improvement to be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants