nushell / nushell

A new type of shell
https://www.nushell.sh/
MIT License
32.02k stars 1.65k forks source link

`Error: nu::shell::variable_not_found` in a script with a recursive custom command #14087

Open bartoszwjn opened 4 days ago

bartoszwjn commented 4 days ago

Describe the bug

After upgrading to nushell 0.98.0 a Nu script that was previously working now fails with a nu::shell::variable_not_found error. The error doesn't point to any specific variable, it seems to be pointing at the entire body of a command.

How to reproduce

This is the minimal example that I was able to reduce my original script to.

  1. Create a file foo.nu
    def bar [] {
    let x = 1
    ($x | foo)
    }
    def foo [] {
    foo
    }
  2. Run nu foo.nu

    $ nu foo.nu
    Error: nu::shell::variable_not_found
    
    × Variable not found
    ╭─[/home/user/foo.nu:5:12]
    4 │     }
    5 │ ╭─▶ def foo [] {
    6 │ │       foo
    7 │ ├─▶ }
    · ╰──── variable not found
    ╰────

Some variations of this that I tried that don't result in an error:

Expected behavior

I expected the script to just do nothing and exit without an error.

Configuration

key value
version 0.98.0
major 0
minor 98
patch 0
branch
commit_hash
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.80.1 (3f5fd8dd4 2024-08-06) (built from a source tarball)
cargo_version cargo 1.80.0 (376290515 2024-07-16)
build_time 1980-01-01 00:00:00 +00:00
build_rust_channel release
allocator mimalloc
features default, sqlite, trash
installed_plugins

The same happens with a version of Nu built from the current main branch (639bd4fc2eec0134f70a2bbda771f7a0b558ebc4)

fdncred commented 4 days ago

It should fail like this. not sure why it doesn't.

❯ def foo [] {
::: foo
::: }
❯ foo
Error: nu::shell::recursion_limit_reached

  × Recursion limit (50) reached
   ╭─[entry #57:1:12]
 1 │ ╭─▶ def foo [] {
 2 │ │   foo
 3 │ ├─▶ }
   · ╰──── This called itself too many times
   ╰────
Talia-12 commented 1 day ago

I've also experienced this issue

fdncred commented 1 day ago

@bartoszwjn if you have time, it would be good to test #14114 in your real-world scenario to see if that fix works for you.

bartoszwjn commented 1 day ago

Yup, it works