Skip to content

Commit 0693865

Browse files
authored
Remove required positional arguments from run-external and exec (#14765)
# Description This PR removes the required positional argument from `run-external` and `exec` in favor of the rest arguments, meaning lists of external commands can be spread directly into `run-external` and `exec`. This does have the drawback of making calling `run-external` and `exec` with no arguments a run-time error rather than a parse error, but I don't imagine that is an issue. Before (for both `run-external` and `exec`): ```nushell run-external # => Error: nu::parser::missing_positional # => # => × Missing required positional argument. # => ╭─[entry #9:1:13] # => 1 │ run-external # => ╰──── # => help: Usage: run-external <command> ...(args) . Use `--help` for more # => information. let command = ["cat" "hello.txt"] run-external ...$command # => Error: nu::parser::missing_positional # => # => × Missing required positional argument. # => ╭─[entry #11:1:14] # => 1 │ run-external ...$command # => · ▲ # => · ╰── missing command # => ╰──── # => help: Usage: run-external <command> ...(args) . Use `--help` for more # => information. run-external ($command | first) ...($command | skip 1) # => hello world! ``` After (for both `run-external` and `exec`): ```nushell run-external # => Error: nu:🐚:missing_parameter # => # => × Missing parameter: no command given. # => ╭─[entry #2:1:1] # => 1 │ run-external # => · ──────┬───── # => · ╰── missing parameter: no command given # => ╰──── # => let command = ["cat" "hello.txt"] run-external ...$command # => hello world! ``` # User-Facing Changes Lists can now be spread directly into `run-external` and `exec`: ```nushell let command = [cat hello.txt] run-external ...$command # => hello world! ``` # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting N/A
1 parent 4656629 commit 0693865

File tree

7 files changed

+247
-161
lines changed

7 files changed

+247
-161
lines changed

Cargo.lock

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ rmp = "0.8"
145145
rmp-serde = "1.3"
146146
roxmltree = "0.20"
147147
rstest = { version = "0.23", default-features = false }
148+
rstest_reuse = "0.7"
148149
rusqlite = "0.31"
149150
rust-embed = "8.5.0"
150151
scopeguard = { version = "1.2.0" }

crates/nu-command/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ mockito = { workspace = true, default-features = false }
196196
quickcheck = { workspace = true }
197197
quickcheck_macros = { workspace = true }
198198
rstest = { workspace = true, default-features = false }
199+
rstest_reuse = { workspace = true }
199200
pretty_assertions = { workspace = true }
200201
tempfile = { workspace = true }
201-
rand_chacha = { workspace = true }
202+
rand_chacha = { workspace = true }

crates/nu-command/src/system/exec.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::borrow::Cow;
2+
13
use nu_engine::{command_prelude::*, env_to_strings};
24

35
#[derive(Clone)]
@@ -11,7 +13,11 @@ impl Command for Exec {
1113
fn signature(&self) -> Signature {
1214
Signature::build("exec")
1315
.input_output_types(vec![(Type::Nothing, Type::Any)])
14-
.required("command", SyntaxShape::String, "The command to execute.")
16+
.rest(
17+
"command",
18+
SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::Any]),
19+
"External command to run, with arguments.",
20+
)
1521
.allows_unknown_args()
1622
.category(Category::System)
1723
}
@@ -33,15 +39,29 @@ On Windows based systems, Nushell will wait for the command to finish and then e
3339
_input: PipelineData,
3440
) -> Result<PipelineData, ShellError> {
3541
let cwd = engine_state.cwd(Some(stack))?;
42+
let rest = call.rest::<Value>(engine_state, stack, 0)?;
43+
let name_args = rest.split_first();
44+
45+
let Some((name, call_args)) = name_args else {
46+
return Err(ShellError::MissingParameter {
47+
param_name: "no command given".into(),
48+
span: call.head,
49+
});
50+
};
51+
52+
let name_str: Cow<str> = match &name {
53+
Value::Glob { val, .. } => Cow::Borrowed(val),
54+
Value::String { val, .. } => Cow::Borrowed(val),
55+
_ => Cow::Owned(name.clone().coerce_into_string()?),
56+
};
3657

3758
// Find the absolute path to the executable. If the command is not
3859
// found, display a helpful error message.
39-
let name: Spanned<String> = call.req(engine_state, stack, 0)?;
4060
let executable = {
4161
let paths = nu_engine::env::path_str(engine_state, stack, call.head)?;
42-
let Some(executable) = crate::which(&name.item, &paths, cwd.as_ref()) else {
62+
let Some(executable) = crate::which(name_str.as_ref(), &paths, cwd.as_ref()) else {
4363
return Err(crate::command_not_found(
44-
&name.item,
64+
&name_str,
4565
call.head,
4666
engine_state,
4767
stack,
@@ -73,7 +93,7 @@ On Windows based systems, Nushell will wait for the command to finish and then e
7393
}
7494

7595
// Configure args.
76-
let args = crate::eval_arguments_from_call(engine_state, stack, call)?;
96+
let args = crate::eval_external_arguments(engine_state, stack, call_args.to_vec())?;
7797
command.args(args.into_iter().map(|s| s.item));
7898

7999
// Execute the child process, replacing/terminating the current process

crates/nu-command/src/system/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub use nu_check::NuCheck;
3333
pub use ps::Ps;
3434
#[cfg(windows)]
3535
pub use registry_query::RegistryQuery;
36-
pub use run_external::{command_not_found, eval_arguments_from_call, which, External};
36+
pub use run_external::{command_not_found, eval_external_arguments, which, External};
3737
pub use sys::*;
3838
pub use uname::UName;
3939
pub use which_::Which;

crates/nu-command/src/system/run_external.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,10 @@ impl Command for External {
3434
fn signature(&self) -> nu_protocol::Signature {
3535
Signature::build(self.name())
3636
.input_output_types(vec![(Type::Any, Type::Any)])
37-
.required(
38-
"command",
39-
SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::String]),
40-
"External command to run.",
41-
)
4237
.rest(
43-
"args",
38+
"command",
4439
SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::Any]),
45-
"Arguments for external command.",
40+
"External command to run, with arguments.",
4641
)
4742
.category(Category::System)
4843
}
@@ -55,7 +50,15 @@ impl Command for External {
5550
input: PipelineData,
5651
) -> Result<PipelineData, ShellError> {
5752
let cwd = engine_state.cwd(Some(stack))?;
58-
let name: Value = call.req(engine_state, stack, 0)?;
53+
let rest = call.rest::<Value>(engine_state, stack, 0)?;
54+
let name_args = rest.split_first();
55+
56+
let Some((name, call_args)) = name_args else {
57+
return Err(ShellError::MissingParameter {
58+
param_name: "no command given".into(),
59+
span: call.head,
60+
});
61+
};
5962

6063
let name_str: Cow<str> = match &name {
6164
Value::Glob { val, .. } => Cow::Borrowed(val),
@@ -151,7 +154,7 @@ impl Command for External {
151154
command.envs(envs);
152155

153156
// Configure args.
154-
let args = eval_arguments_from_call(engine_state, stack, call)?;
157+
let args = eval_external_arguments(engine_state, stack, call_args.to_vec())?;
155158
#[cfg(windows)]
156159
if is_cmd_internal_command(&name_str) || potential_nuscript_in_windows {
157160
// The /D flag disables execution of AutoRun commands from registry.
@@ -290,14 +293,13 @@ impl Command for External {
290293
}
291294
}
292295

293-
/// Evaluate all arguments from a call, performing expansions when necessary.
294-
pub fn eval_arguments_from_call(
296+
/// Evaluate all arguments, performing expansions when necessary.
297+
pub fn eval_external_arguments(
295298
engine_state: &EngineState,
296299
stack: &mut Stack,
297-
call: &Call,
300+
call_args: Vec<Value>,
298301
) -> Result<Vec<Spanned<OsString>>, ShellError> {
299302
let cwd = engine_state.cwd(Some(stack))?;
300-
let call_args = call.rest::<Value>(engine_state, stack, 1)?;
301303
let mut args: Vec<Spanned<OsString>> = Vec::with_capacity(call_args.len());
302304

303305
for arg in call_args {

0 commit comments

Comments
 (0)