From feef612388e34238866f3a56ed9583f53958616f Mon Sep 17 00:00:00 2001 From: Antoine Stevan <44101798+amtoine@users.noreply.github.com> Date: Tue, 26 Sep 2023 11:38:58 +0200 Subject: [PATCH] show the full directory / file path in "directory not found" error (#10430) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit should close https://github.com/nushell/nushell/issues/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 #7:1:1] 1 │ open $does_not_exist · ───────┬─────── · ╰── directory not found ╰──── ``` ```nushell > 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 ```nushell > 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 ``` ```nushell > 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 :crossed_fingers: # After Submitting --- crates/nu-cli/src/repl.rs | 5 ++++- crates/nu-command/src/filesystem/cd.rs | 21 ++++++++++++--------- crates/nu-command/src/filesystem/cp.rs | 2 +- crates/nu-command/src/filesystem/mv.rs | 12 ++++++++++-- crates/nu-command/src/filesystem/watch.rs | 4 ++-- crates/nu-command/tests/commands/cp.rs | 2 +- crates/nu-engine/src/glob_from.rs | 7 +++++-- crates/nu-protocol/src/shell_error.rs | 4 ++-- 8 files changed, 37 insertions(+), 20 deletions(-) diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 9341a1b33e..908f589ddd 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -512,7 +512,10 @@ pub fn evaluate_repl( report_error( &working_set, - &ShellError::DirectoryNotFound(tokens.0[0].span, None), + &ShellError::DirectoryNotFound( + tokens.0[0].span, + path.to_string_lossy().to_string(), + ), ); } let path = nu_path::canonicalize_with(path, &cwd) diff --git a/crates/nu-command/src/filesystem/cd.rs b/crates/nu-command/src/filesystem/cd.rs index 18ce5a5dfe..cf8b303357 100644 --- a/crates/nu-command/src/filesystem/cd.rs +++ b/crates/nu-command/src/filesystem/cd.rs @@ -85,21 +85,21 @@ impl Command for Cd { let path = oldpwd.as_path()?; let path = match nu_path::canonicalize_with(path.clone(), &cwd) { Ok(p) => p, - Err(e1) => { + Err(_) => { if use_abbrev { match query(&path, None, v.span) { Ok(p) => p, - Err(e) => { + Err(_) => { return Err(ShellError::DirectoryNotFound( v.span, - Some(format!("IO Error: {e:?}")), + path.to_string_lossy().to_string(), )) } } } else { return Err(ShellError::DirectoryNotFound( v.span, - Some(format!("IO Error: {e1:?}")), + path.to_string_lossy().to_string(), )); } } @@ -119,10 +119,10 @@ impl Command for Cd { // if it's not a dir, let's check to see if it's something abbreviated match query(&p, None, v.span) { Ok(path) => path, - Err(e) => { + Err(_) => { return Err(ShellError::DirectoryNotFound( v.span, - Some(format!("IO Error: {e:?}")), + p.to_string_lossy().to_string(), )) } }; @@ -138,15 +138,18 @@ impl Command for Cd { if use_abbrev { match query(&path_no_whitespace, None, v.span) { Ok(path) => path, - Err(e) => { + Err(_) => { return Err(ShellError::DirectoryNotFound( v.span, - Some(format!("IO Error: {e:?}")), + path_no_whitespace.to_string(), )) } } } else { - return Err(ShellError::DirectoryNotFound(v.span, None)); + return Err(ShellError::DirectoryNotFound( + v.span, + path_no_whitespace.to_string(), + )); } } }; diff --git a/crates/nu-command/src/filesystem/cp.rs b/crates/nu-command/src/filesystem/cp.rs index cb18ec9f33..82fc127f45 100644 --- a/crates/nu-command/src/filesystem/cp.rs +++ b/crates/nu-command/src/filesystem/cp.rs @@ -103,7 +103,7 @@ impl Command for Cp { if is_directory && !destination.exists() { return Err(ShellError::DirectoryNotFound( dst.span, - Some("destination directory does not exist".to_string()), + destination.to_string_lossy().to_string(), )); } let ctrlc = engine_state.ctrlc.clone(); diff --git a/crates/nu-command/src/filesystem/mv.rs b/crates/nu-command/src/filesystem/mv.rs index 43e3b5719a..c8c2771383 100644 --- a/crates/nu-command/src/filesystem/mv.rs +++ b/crates/nu-command/src/filesystem/mv.rs @@ -276,7 +276,10 @@ fn move_file( }; if !destination_dir_exists { - return Err(ShellError::DirectoryNotFound(to_span, None)); + return Err(ShellError::DirectoryNotFound( + to_span, + to.to_string_lossy().to_string(), + )); } // This can happen when changing case on a case-insensitive filesystem (ex: changing foo to Foo on Windows) @@ -287,7 +290,12 @@ fn move_file( if !from_to_are_same_file && to.is_dir() { let from_file_name = match from.file_name() { Some(name) => name, - None => return Err(ShellError::DirectoryNotFound(to_span, None)), + None => { + return Err(ShellError::DirectoryNotFound( + to_span, + from.to_string_lossy().to_string(), + )) + } }; to.push(from_file_name); diff --git a/crates/nu-command/src/filesystem/watch.rs b/crates/nu-command/src/filesystem/watch.rs index 9374a0a226..91fd30912c 100644 --- a/crates/nu-command/src/filesystem/watch.rs +++ b/crates/nu-command/src/filesystem/watch.rs @@ -82,10 +82,10 @@ impl Command for Watch { let path = match nu_path::canonicalize_with(path_no_whitespace, cwd) { Ok(p) => p, - Err(e) => { + Err(_) => { return Err(ShellError::DirectoryNotFound( path_arg.span, - Some(format!("IO Error: {e:?}")), + path_no_whitespace.to_string(), )) } }; diff --git a/crates/nu-command/tests/commands/cp.rs b/crates/nu-command/tests/commands/cp.rs index d1229e727b..53b0934386 100644 --- a/crates/nu-command/tests/commands/cp.rs +++ b/crates/nu-command/tests/commands/cp.rs @@ -391,7 +391,7 @@ fn copy_to_non_existing_dir_impl(progress: bool) { progress_flag ); assert!(actual.err.contains("directory not found")); - assert!(actual.err.contains("destination directory does not exist")); + assert!(actual.err.contains("does not exist")); }); } diff --git a/crates/nu-engine/src/glob_from.rs b/crates/nu-engine/src/glob_from.rs index bd3aebf736..38729544db 100644 --- a/crates/nu-engine/src/glob_from.rs +++ b/crates/nu-engine/src/glob_from.rs @@ -69,10 +69,13 @@ pub fn glob_from( if is_symlink { (path.parent().map(|parent| parent.to_path_buf()), path) } else { - let path = if let Ok(p) = canonicalize_with(path, cwd) { + let path = if let Ok(p) = canonicalize_with(path.clone(), cwd) { p } else { - return Err(ShellError::DirectoryNotFound(pattern.span, None)); + return Err(ShellError::DirectoryNotFound( + pattern.span, + path.to_string_lossy().to_string(), + )); }; (path.parent().map(|parent| parent.to_path_buf()), path) } diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 88cc7f0bc3..d0c84ef4cc 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -801,8 +801,8 @@ pub enum ShellError { /// /// Make sure the directory in the error message actually exists before trying again. #[error("Directory not found")] - #[diagnostic(code(nu::shell::directory_not_found))] - DirectoryNotFound(#[label("directory not found")] Span, #[help] Option), + #[diagnostic(code(nu::shell::directory_not_found), help("{1} does not exist"))] + DirectoryNotFound(#[label("directory not found")] Span, String), /// Attempted to perform an operation on a directory that doesn't exist. ///