fix(metadata set): return error when both --datasource-filepath and -datasource-ls are used (#16049)

# Description

This PR improves the `metadata set` command by returning a clear error
when both `--datasource-filepath` and `--datasource-ls` flags are used
together. These flags are meant to be mutually exclusive, and previously
this conflicting usage was silently ignored.

# User-Facing Changes

* Users will now see an error message if they use both
`--datasource-filepath` and `--datasource-ls` together in `metadata
set`.

# Tests + Formatting

* [x] Added test at
`crates/nu-command/tests/commands/debug/metadata_set.rs` to verify the
error behavior.
* [x] Ran `cargo fmt --all -- --check`
* [x] Ran `cargo clippy --workspace -- -D warnings -D
clippy::unwrap_used`
* [x] Ran `cargo test --workspace`


# After Submitting

N/A
This commit is contained in:
Kumar Ujjawal
2025-07-02 23:10:34 +05:30
committed by GitHub
parent a340e965e8
commit 118857aedc
4 changed files with 67 additions and 2 deletions

View File

@ -43,6 +43,17 @@ impl Command for Metadata {
let arg = call.positional_nth(stack, 0); let arg = call.positional_nth(stack, 0);
let head = call.head; let head = call.head;
if !matches!(input, PipelineData::Empty) {
if let Some(arg_expr) = arg {
return Err(ShellError::IncompatibleParameters {
left_message: "pipeline input was provided".into(),
left_span: head,
right_message: "but a positional metadata expression was also given".into(),
right_span: arg_expr.span,
});
}
}
match arg { match arg {
Some(Expression { Some(Expression {
expr: Expr::FullCellPath(full_cell_path), expr: Expr::FullCellPath(full_cell_path),
@ -56,7 +67,6 @@ impl Command for Metadata {
.. ..
} => { } => {
let origin = stack.get_var_with_origin(*var_id, *span)?; let origin = stack.get_var_with_origin(*var_id, *span)?;
Ok(build_metadata_record_value( Ok(build_metadata_record_value(
&origin, &origin,
input.metadata().as_ref(), input.metadata().as_ref(),

View File

@ -63,7 +63,18 @@ impl Command for MetadataSet {
match (ds_fp, ds_ls) { match (ds_fp, ds_ls) {
(Some(path), false) => metadata.data_source = DataSource::FilePath(path.into()), (Some(path), false) => metadata.data_source = DataSource::FilePath(path.into()),
(None, true) => metadata.data_source = DataSource::Ls, (None, true) => metadata.data_source = DataSource::Ls,
(Some(_), true) => (), // TODO: error here (Some(_), true) => {
return Err(ShellError::IncompatibleParameters {
left_message: "cannot use `--datasource-filepath`".into(),
left_span: call
.get_flag_span(stack, "datasource-filepath")
.expect("has flag"),
right_message: "with `--datasource-ls`".into(),
right_span: call
.get_flag_span(stack, "datasource-ls")
.expect("has flag"),
});
}
(None, false) => (), (None, false) => (),
} }

View File

@ -0,0 +1,43 @@
use nu_test_support::nu;
use nu_test_support::pipeline;
#[test]
fn errors_on_conflicting_metadata_flags() {
let actual = nu!(
cwd: ".", pipeline(
r#"
echo "foo" | metadata set --datasource-filepath foo.txt --datasource-ls
"#
));
assert!(actual.err.contains("cannot use `--datasource-filepath`"));
assert!(actual.err.contains("with `--datasource-ls`"));
}
#[test]
fn works_with_datasource_filepath() {
let actual = nu!(
cwd: ".", pipeline(
r#"
echo "foo"
| metadata set --datasource-filepath foo.txt
| metadata
"#
));
assert!(actual.out.contains("foo.txt"));
}
#[test]
fn works_with_datasource_ls() {
let actual = nu!(
cwd: ".", pipeline(
r#"
echo "foo"
| metadata set --datasource-ls
| metadata
"#
));
assert!(actual.out.contains("ls"));
}

View File

@ -1 +1,2 @@
mod metadata_set;
mod timeit; mod timeit;