From 059167ac96e3864a9e47aa95ad60245fa5df2a8a Mon Sep 17 00:00:00 2001 From: playdohface <110300169+playdohface@users.noreply.github.com> Date: Mon, 12 Aug 2024 14:21:08 +0200 Subject: [PATCH] Make error-message more helpful when user invokes a non-executable file (#13589) # Description Fixes Issue #13477 This adds a check to see if a user is trying to invoke a (non-executable) file as a command and returns a helpful error if so. EDIT: this will not work on Windows, and is arguably not relevant there, because of the different semantics of executables. I think the equivalent on Windows would be if a user tries to invoke `./foo`, we should look for `foo.exe` or `foo.bat` in the directory and recommend that if it exists. # User-Facing Changes When a user invokes an unrecognized command that is the path to an existing file, the error used to say: `{name} is neither a Nushell built-in or a known external command` This PR proposes to change the message to: `{name} refers to a file that is not executable. Did you forget to to set execute permissions?` # Tests + Formatting Ran cargo fmt, clippy and test on the workspace. EDIT: added test asserting the new behavior --- crates/nu-command/src/system/run_external.rs | 9 +++++++++ tests/shell/pipeline/commands/external.rs | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index 0b9f1950ea..c812280b62 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -473,6 +473,15 @@ pub fn command_not_found( }; } + // If we find a file, it's likely that the user forgot to set permissions + if Path::new(name).is_file() { + return ShellError::ExternalCommand { + label: format!("Command `{name}` not found"), + help: format!("`{name}` refers to a file that is not executable. Did you forget to to set execute permissions?"), + span, + }; + } + // We found nothing useful. Give up and return a generic error message. ShellError::ExternalCommand { label: format!("Command `{name}` not found"), diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index ff851a3293..5b9f116626 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -127,6 +127,15 @@ fn command_not_found_error_suggests_typo_fix() { assert!(actual.err.contains("timeit")); } +#[cfg(not(windows))] +#[test] +fn command_not_found_error_recognizes_non_executable_file() { + let actual = nu!("./Cargo.toml"); + assert!(actual.err.contains( + "refers to a file that is not executable. Did you forget to to set execute permissions?" + )); +} + #[test] fn command_not_found_error_shows_not_found_1() { let actual = nu!(r#"