Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
## Bugfixes
- Handle invalid working directories gracefully when using `--full-path`, see #1900 (@Xavrir).
- Fire the "search pattern contains a path separator" diagnostic for any pattern containing `/`, not just patterns that happen to name an existing directory. Preserves the legacy Windows behaviour that also flags native `\` separators when the pattern resolves to a real directory. See #1873.
- Hint that a command is a shell builtin when `-x`/`-X` fails with "command not found", see #1944 (@kimjune01)

# 10.4.2

Expand Down
73 changes: 69 additions & 4 deletions src/exec/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,37 @@ pub fn execute_commands<I: Iterator<Item = io::Result<Command>>>(
ExitCode::Success
}

/// Common shell builtins that typically do not exist as standalone executables.
/// When fd encounters a "command not found" error for one of these, it hints
/// that the user may be trying to use a shell builtin.
const SHELL_BUILTINS: &[&str] = &[

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this should be a HashSet personally

".", "alias", "bg", "bind", "cd", "command", "declare", "dirs", "eval", "exec", "exit",
"export", "fg", "hash", "help", "history", "jobs", "let", "local", "logout", "popd", "pushd",
"read", "readonly", "return", "set", "shift", "shopt", "source", "suspend", "times", "trap",
"type", "typeset", "unalias", "unset", "wait",
];

fn is_shell_builtin(program: &str) -> bool {
SHELL_BUILTINS.iter().any(|&b| b == program)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does contains() work here?

}

fn command_not_found_message(program: &str) -> String {
if is_shell_builtin(program) {
format!(
"Command not found: {program}. Note: {program} is a shell builtin, \
not a standalone program. To run shell builtins, invoke a shell explicitly, \
e.g. fd -x sh -c '{program} ... \"$1\"' sh {{}}",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for getting the quoting/escaping right here :)

)
} else {
format!("Command not found: {program}")
}
}

pub fn handle_cmd_error(cmd: Option<&Command>, err: io::Error) -> ExitCode {
match (cmd, err) {
(Some(cmd), err) if err.kind() == io::ErrorKind::NotFound => {
print_error(format!(
"Command not found: {}",
cmd.get_program().to_string_lossy()
));
let program = cmd.get_program().to_string_lossy();
print_error(command_not_found_message(&program));
ExitCode::GeneralError
}
(_, err) => {
Expand All @@ -113,3 +137,44 @@ pub fn handle_cmd_error(cmd: Option<&Command>, err: io::Error) -> ExitCode {
}
}
}

#[cfg(test)]
mod builtin_tests {
use super::*;

#[test]
fn detects_known_builtins() {
assert!(is_shell_builtin("cd"));
assert!(is_shell_builtin("export"));
assert!(is_shell_builtin("source"));
assert!(is_shell_builtin("eval"));
assert!(is_shell_builtin("."));
}

#[test]
fn rejects_non_builtins() {
assert!(!is_shell_builtin("grep"));
assert!(!is_shell_builtin("ls"));
assert!(!is_shell_builtin(""));
assert!(!is_shell_builtin("CD"));
// These typically exist as standalone executables
assert!(!is_shell_builtin("echo"));
assert!(!is_shell_builtin("printf"));
assert!(!is_shell_builtin("test"));
}

#[test]
fn builtin_message_includes_hint() {
let msg = command_not_found_message("cd");
assert!(msg.starts_with("Command not found: cd."));
assert!(msg.contains("shell builtin"));
assert!(msg.contains("sh -c"));
}

#[test]
fn non_builtin_message_is_plain() {
let msg = command_not_found_message("nonexistent");
assert_eq!(msg, "Command not found: nonexistent");
assert!(!msg.contains("shell builtin"));
}
}
Loading