From 6ff3a4180bfc129ece77578ef2ba1e09deb130b2 Mon Sep 17 00:00:00 2001 From: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Date: Wed, 21 Feb 2024 08:27:13 -0500 Subject: [PATCH] Specify which file not found in error (#11868) # Description Currently, `ShellError::FileNotFound` shows the span where the error occurred but doesn't say which file wasn't found. This PR makes it so the help includes that (like the `DirectoryNotFound` error). # User-Facing Changes No breaking changes, it's just that when a file can't be found, the help will say which file couldn't be found: ![image](https://github.com/nushell/nushell/assets/45539777/e52f1e65-55c1-4cd2-8108-a4ccc334a66f) --- .../nu-cli/src/commands/history/history_.rs | 34 ++++++++++++------- crates/nu-command/src/env/source_env.rs | 1 + crates/nu-command/src/filesystem/mv.rs | 1 + crates/nu-command/src/filesystem/open.rs | 5 ++- crates/nu-command/src/filesystem/ucp.rs | 5 ++- crates/nu-command/src/filesystem/umv.rs | 5 ++- crates/nu-command/src/system/nu_check.rs | 1 + crates/nu-protocol/src/shell_error.rs | 3 +- 8 files changed, 39 insertions(+), 16 deletions(-) diff --git a/crates/nu-cli/src/commands/history/history_.rs b/crates/nu-cli/src/commands/history/history_.rs index b8e7c3ac41..376ea19d7c 100644 --- a/crates/nu-cli/src/commands/history/history_.rs +++ b/crates/nu-cli/src/commands/history/history_.rs @@ -72,7 +72,7 @@ impl Command for History { } else { let history_reader: Option> = match history.file_format { HistoryFileFormat::Sqlite => { - SqliteBackedHistory::with_file(history_path, None, None) + SqliteBackedHistory::with_file(history_path.clone(), None, None) .map(|inner| { let boxed: Box = Box::new(inner); boxed @@ -80,14 +80,15 @@ impl Command for History { .ok() } - HistoryFileFormat::PlainText => { - FileBackedHistory::with_file(history.max_size as usize, history_path) - .map(|inner| { - let boxed: Box = Box::new(inner); - boxed - }) - .ok() - } + HistoryFileFormat::PlainText => FileBackedHistory::with_file( + history.max_size as usize, + history_path.clone(), + ) + .map(|inner| { + let boxed: Box = Box::new(inner); + boxed + }) + .ok(), }; match history.file_format { @@ -107,7 +108,10 @@ impl Command for History { ) }) }) - .ok_or(ShellError::FileNotFound { span: head })? + .ok_or(ShellError::FileNotFound { + file: history_path.display().to_string(), + span: head, + })? .into_pipeline_data(ctrlc)), HistoryFileFormat::Sqlite => Ok(history_reader .and_then(|h| { @@ -119,12 +123,18 @@ impl Command for History { create_history_record(idx, entry, long, head) }) }) - .ok_or(ShellError::FileNotFound { span: head })? + .ok_or(ShellError::FileNotFound { + file: history_path.display().to_string(), + span: head, + })? .into_pipeline_data(ctrlc)), } } } else { - Err(ShellError::FileNotFound { span: head }) + Err(ShellError::FileNotFound { + file: "history file".into(), + span: head, + }) } } diff --git a/crates/nu-command/src/env/source_env.rs b/crates/nu-command/src/env/source_env.rs index 3a7a280f0e..9ce69ca178 100644 --- a/crates/nu-command/src/env/source_env.rs +++ b/crates/nu-command/src/env/source_env.rs @@ -56,6 +56,7 @@ impl Command for SourceEnv { PathBuf::from(&path) } else { return Err(ShellError::FileNotFound { + file: source_filename.item, span: source_filename.span, }); }; diff --git a/crates/nu-command/src/filesystem/mv.rs b/crates/nu-command/src/filesystem/mv.rs index e082bcd95c..9919287119 100644 --- a/crates/nu-command/src/filesystem/mv.rs +++ b/crates/nu-command/src/filesystem/mv.rs @@ -81,6 +81,7 @@ impl Command for Mv { if sources.is_empty() { return Err(ShellError::FileNotFound { + file: spanned_source.item.to_string(), span: spanned_source.span, }); } diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index da77c3c83a..2419abbd1d 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -92,7 +92,10 @@ impl Command for Open { for path in nu_engine::glob_from(&path, &cwd, call_span, None) .map_err(|err| match err { - ShellError::DirectoryNotFound { span, .. } => ShellError::FileNotFound { span }, + ShellError::DirectoryNotFound { span, .. } => ShellError::FileNotFound { + file: path.item.to_string(), + span, + }, _ => err, })? .1 diff --git a/crates/nu-command/src/filesystem/ucp.rs b/crates/nu-command/src/filesystem/ucp.rs index 987de023da..170d1b8a0b 100644 --- a/crates/nu-command/src/filesystem/ucp.rs +++ b/crates/nu-command/src/filesystem/ucp.rs @@ -205,7 +205,10 @@ impl Command for UCp { .map(|f| f.1)? .collect(); if exp_files.is_empty() { - return Err(ShellError::FileNotFound { span: p.span }); + return Err(ShellError::FileNotFound { + file: p.item.to_string(), + span: p.span, + }); }; let mut app_vals: Vec = Vec::new(); for v in exp_files { diff --git a/crates/nu-command/src/filesystem/umv.rs b/crates/nu-command/src/filesystem/umv.rs index beb8304b16..cc4795464c 100644 --- a/crates/nu-command/src/filesystem/umv.rs +++ b/crates/nu-command/src/filesystem/umv.rs @@ -121,7 +121,10 @@ impl Command for UMv { .map(|f| f.1)? .collect(); if exp_files.is_empty() { - return Err(ShellError::FileNotFound { span: p.span }); + return Err(ShellError::FileNotFound { + file: p.item.to_string(), + span: p.span, + }); }; let mut app_vals: Vec = Vec::new(); for v in exp_files { diff --git a/crates/nu-command/src/system/nu_check.rs b/crates/nu-command/src/system/nu_check.rs index 822f0c53fa..009074d9b1 100644 --- a/crates/nu-command/src/system/nu_check.rs +++ b/crates/nu-command/src/system/nu_check.rs @@ -120,6 +120,7 @@ impl Command for NuCheck { path } else { return Err(ShellError::FileNotFound { + file: path_str.item, span: path_str.span, }); } diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index e94b0063a2..d03923d499 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -727,8 +727,9 @@ pub enum ShellError { /// /// Does the file in the error message exist? Is it readable and accessible? Is the casing right? #[error("File not found")] - #[diagnostic(code(nu::shell::file_not_found))] + #[diagnostic(code(nu::shell::file_not_found), help("{file} does not exist"))] FileNotFound { + file: String, #[label("file not found")] span: Span, },