Skip to content

show the full directory / file path in "directory not found" error#10430

Merged
WindSoilder merged 6 commits intonushell:mainfrom
amtoine:fix-10406
Sep 26, 2023
Merged

show the full directory / file path in "directory not found" error#10430
WindSoilder merged 6 commits intonushell:mainfrom
amtoine:fix-10406

Conversation

@amtoine
Copy link
Copy Markdown
Member

@amtoine amtoine commented Sep 19, 2023

should close #10406

Description

when writing a script, with variables you try to ls or open, you will get a "directory not found" error but the variable won't be expanded and you won't be able to see which one of the variable was the issue...

this PR adds this information to the error.

User-Facing Changes

let's define a variable

let does_not_exist = "i_do_not_exist_in_the_current_directory"

before

> open $does_not_exist
Error: nu::shell::directory_not_found

  × Directory not found
   ╭─[entry #7:1:1]
 1  open $does_not_exist
   ·      ───────┬───────
   ·             ╰── directory not found
   ╰────
> ls $does_not_exist
Error: nu::shell::directory_not_found

  × Directory not found
   ╭─[entry #8:1:1]
 1  ls $does_not_exist
   ·    ───────┬───────
   ·           ╰── directory not found
   ╰────

after

> open $does_not_exist
Error: nu::shell::directory_not_found

  × Directory not found
   ╭─[entry #3:1:1]
 1  open $does_not_exist
   ·      ───────┬───────
   ·             ╰── directory not found
   ╰────
  help: /home/amtoine/documents/repos/github.com/amtoine/nushell/i_do_not_exist_in_the_current_directory does not exist
> ls $does_not_exist
Error: nu::shell::directory_not_found

  × Directory not found
   ╭─[entry #4:1:1]
 1  ls $does_not_exist
   ·    ───────┬───────
   ·           ╰── directory not found
   ╰────
  help: /home/amtoine/documents/repos/github.com/amtoine/nushell/i_do_not_exist_in_the_current_directory does not exist

Tests + Formatting

shouldn't harm anything 🤞

After Submitting

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Sep 19, 2023

i'm not happy with the failing test, making this a DRAFT for now and i'll review it later, it's on the short-term TODO 👌

@amtoine amtoine marked this pull request as draft September 19, 2023 17:41
code(nu::shell::directory_not_found),
help("the target directory was {0}")
)]
DirectoryNotFound(String, #[label("directory not found")] Span, Option<String>),
Copy link
Copy Markdown
Contributor

@WindSoilder WindSoilder Sep 20, 2023

Choose a reason for hiding this comment

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

What about changing the definition like this:

   #[diagnostic(
        code(nu::shell::directory_not_found),
        help("{1} not found")
    )]
    DirectoryNotFound(#[label("directory not found")] Span, String),

We already defined it as DirectoryNotFound error, I don't think we need to provide underlying IO::Error message to user.
So the error message will be something like this:

 × Directory not found
   ╭─[entry #1:1:1]
 1  ["a.txt"] | each {|$it| open $it}
   ·                              ─┬─
   ·                               ╰── directory not found
   ╰────
  help: /Users/tmp/projects/rust_online_code/nushell/a.txt not found

Which is less verbosing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

should be better in b73b304 😋

just need to fix the tests now i guess 😉

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Sep 23, 2023

yeah, the tests pass and i've updated the PR body, this is ready for review 🥳

@amtoine amtoine marked this pull request as ready for review September 23, 2023 09:55
Copy link
Copy Markdown
Contributor

@WindSoilder WindSoilder left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@WindSoilder WindSoilder merged commit feef612 into nushell:main Sep 26, 2023
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 26, 2023

Having the full path is a great improvement but I don't think we should get rid of the IO Errors. They can be very helpful for diagnosing weird filesystem errors.

@amtoine amtoine deleted the fix-10406 branch September 26, 2023 16:22
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
…ushell#10430)

should close nushell#10406

# Description
when writing a script, with variables you try to `ls` or `open`, you
will get a "directory not found" error but the variable won't be
expanded and you won't be able to see which one of the variable was the
issue...

this PR adds this information to the error.

# User-Facing Changes
let's define a variable
```nushell
let does_not_exist = "i_do_not_exist_in_the_current_directory"
```
### before
```nushell
> open $does_not_exist
Error: nu::shell::directory_not_found

  × Directory not found
   ╭─[entry nushell#7:1:1]
 1 │ open $does_not_exist
   ·      ───────┬───────
   ·             ╰── directory not found
   ╰────
```
```nushell
> ls $does_not_exist
Error: nu:🐚:directory_not_found

  × Directory not found
   ╭─[entry nushell#8:1:1]
 1 │ ls $does_not_exist
   ·    ───────┬───────
   ·           ╰── directory not found
   ╰────
```

### after
```nushell
> open $does_not_exist
Error: nu:🐚:directory_not_found

  × Directory not found
   ╭─[entry nushell#3:1:1]
 1 │ open $does_not_exist
   ·      ───────┬───────
   ·             ╰── directory not found
   ╰────
  help: /home/amtoine/documents/repos/github.com/amtoine/nushell/i_do_not_exist_in_the_current_directory does not exist
```
```nushell
> ls $does_not_exist
Error: nu:🐚:directory_not_found

  × Directory not found
   ╭─[entry nushell#4:1:1]
 1 │ ls $does_not_exist
   ·    ───────┬───────
   ·           ╰── directory not found
   ╰────
  help: /home/amtoine/documents/repos/github.com/amtoine/nushell/i_do_not_exist_in_the_current_directory does not exist
```

# Tests + Formatting
shouldn't harm anything 🤞 

# After Submitting
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.

file_not_found/directory_not_found on evaluated string doesn't say what the path is; makes error hard to debug

3 participants