mirror of
https://github.com/nushell/nushell.git
synced 2024-11-07 17:14:23 +01:00
Restore tilde expansion on external command names (#13001)
# Description Fix a regression introduced by #12921, where tilde expansion was no longer done on the external command name, breaking things like ```nushell > ~/.cargo/bin/exa ``` This properly handles quoted strings, so they don't expand: ```nushell > ^"~/.cargo/bin/exa" Error: nu:🐚:external_command × External command failed ╭─[entry #1:1:2] 1 │ ^"~/.cargo/bin/exa" · ─────────┬──────── · ╰── Command `~/.cargo/bin/exa` not found ╰──── help: `~/.cargo/bin/exa` is neither a Nushell built-in or a known external command ``` This required a change to the parser, so the command name is also parsed in the same way the arguments are - i.e. the quotes on the outside remain in the expression. Hopefully that doesn't break anything else. 🤞 Fixes #13000. Should include in patch release 0.94.1 cc @YizhePKU # User-Facing Changes - Tilde expansion now works again for external commands - The `command` of `run-external` will now have its quotes removed like the other arguments if it is a literal string - The parser is changed to include quotes in the command expression of `ExternalCall` if they were present # Tests + Formatting I would like to add a regression test for this, but it's complicated because we need a well-known binary within the home directory, which just isn't a thing. We could drop one there, but that's kind of a bad behavior for a test to do. I also considered changing the home directory for the test, but that's so platform-specific - potentially could get it working on specific platforms though. Changing `HOME` env on Linux definitely works as far as tilde expansion works. - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib`
This commit is contained in:
parent
6a2996251c
commit
0e1553026e
@ -46,15 +46,36 @@ impl Command for External {
|
||||
) -> Result<PipelineData, ShellError> {
|
||||
let cwd = engine_state.cwd(Some(stack))?;
|
||||
|
||||
// Evaluate the command name in the same way the arguments are evaluated. Since this isn't
|
||||
// a spread, it should return a one-element vec.
|
||||
let name_expr = call
|
||||
.positional_nth(0)
|
||||
.ok_or_else(|| ShellError::MissingParameter {
|
||||
param_name: "command".into(),
|
||||
span: call.head,
|
||||
})?;
|
||||
let name = eval_argument(engine_state, stack, name_expr, false)?
|
||||
.pop()
|
||||
.expect("eval_argument returned zero-element vec")
|
||||
.into_spanned(name_expr.span);
|
||||
|
||||
// Find the absolute path to the executable. On Windows, set the
|
||||
// executable to "cmd.exe" if it's is a CMD internal command. If the
|
||||
// command is not found, display a helpful error message.
|
||||
let name: Spanned<String> = call.req(engine_state, stack, 0)?;
|
||||
let executable = if cfg!(windows) && is_cmd_internal_command(&name.item) {
|
||||
PathBuf::from("cmd.exe")
|
||||
} else {
|
||||
// Expand tilde on the name if it's a bare string (#13000)
|
||||
let expanded_name = if is_bare_string(name_expr) {
|
||||
expand_tilde(&name.item)
|
||||
} else {
|
||||
name.item.clone()
|
||||
};
|
||||
|
||||
// Determine the PATH to be used and then use `which` to find it - though this has no
|
||||
// effect if it's an absolute path already
|
||||
let paths = nu_engine::env::path_str(engine_state, stack, call.head)?;
|
||||
let Some(executable) = which(&name.item, &paths, &cwd) else {
|
||||
let Some(executable) = which(&expanded_name, &paths, &cwd) else {
|
||||
return Err(command_not_found(
|
||||
&name.item,
|
||||
call.head,
|
||||
|
@ -278,13 +278,14 @@ pub fn parse_external_call(working_set: &mut StateWorkingSet, spans: &[Span]) ->
|
||||
let arg = parse_expression(working_set, &[head_span]);
|
||||
Box::new(arg)
|
||||
} else {
|
||||
let (contents, err) = unescape_unquote_string(&head_contents, head_span);
|
||||
// Eval stage will unquote the string, so we don't bother with that here
|
||||
let (contents, err) = unescape_string(&head_contents, head_span);
|
||||
if let Some(err) = err {
|
||||
working_set.error(err)
|
||||
}
|
||||
|
||||
Box::new(Expression {
|
||||
expr: Expr::String(contents),
|
||||
expr: Expr::String(String::from_utf8_lossy(&contents).into_owned()),
|
||||
span: head_span,
|
||||
ty: Type::String,
|
||||
custom_completion: None,
|
||||
|
Loading…
Reference in New Issue
Block a user