From 118857aedc72805611f4046da653475c3a482e1c Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Wed, 2 Jul 2025 23:10:34 +0530 Subject: [PATCH] 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 --- crates/nu-command/src/debug/metadata.rs | 12 +++++- crates/nu-command/src/debug/metadata_set.rs | 13 +++++- .../tests/commands/debug/metadata_set.rs | 43 +++++++++++++++++++ crates/nu-command/tests/commands/debug/mod.rs | 1 + 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 crates/nu-command/tests/commands/debug/metadata_set.rs diff --git a/crates/nu-command/src/debug/metadata.rs b/crates/nu-command/src/debug/metadata.rs index 70bd699ba4..9f1705a3f5 100644 --- a/crates/nu-command/src/debug/metadata.rs +++ b/crates/nu-command/src/debug/metadata.rs @@ -43,6 +43,17 @@ impl Command for Metadata { let arg = call.positional_nth(stack, 0); 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 { Some(Expression { expr: Expr::FullCellPath(full_cell_path), @@ -56,7 +67,6 @@ impl Command for Metadata { .. } => { let origin = stack.get_var_with_origin(*var_id, *span)?; - Ok(build_metadata_record_value( &origin, input.metadata().as_ref(), diff --git a/crates/nu-command/src/debug/metadata_set.rs b/crates/nu-command/src/debug/metadata_set.rs index b5dd742ea3..19aeb3f29d 100644 --- a/crates/nu-command/src/debug/metadata_set.rs +++ b/crates/nu-command/src/debug/metadata_set.rs @@ -63,7 +63,18 @@ impl Command for MetadataSet { match (ds_fp, ds_ls) { (Some(path), false) => metadata.data_source = DataSource::FilePath(path.into()), (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) => (), } diff --git a/crates/nu-command/tests/commands/debug/metadata_set.rs b/crates/nu-command/tests/commands/debug/metadata_set.rs new file mode 100644 index 0000000000..746477792e --- /dev/null +++ b/crates/nu-command/tests/commands/debug/metadata_set.rs @@ -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")); +} diff --git a/crates/nu-command/tests/commands/debug/mod.rs b/crates/nu-command/tests/commands/debug/mod.rs index 4d5674a0d9..53f25e51ae 100644 --- a/crates/nu-command/tests/commands/debug/mod.rs +++ b/crates/nu-command/tests/commands/debug/mod.rs @@ -1 +1,2 @@ +mod metadata_set; mod timeit;