From f41b1460aa90c86bfb88c60352bb6cbf28d382fb Mon Sep 17 00:00:00 2001 From: Marco Cunha Date: Thu, 24 Apr 2025 15:09:48 +0100 Subject: [PATCH] Fix #14660: to md breaks on tables with empty values (#15631) Fixes #14660 # Description Fixed an issue where tables with empty values were incorrectly replaced with [table X row] when converted to Markdown using the ```to md``` command. Empty values are now replaced with whitespaces to preserve the original table structure. Additionally, fixed a missing newline (\n) between tables when using --per-element in a list. Removed (\n) from 2 examples for consistency. Example: ``` For the list let list = [ {name: bob, age: 21} {name: jim, age: 20} {name: sarah}] Running "$list | to md --pretty" outputs: | name | age | | ----- | --- | | bob | 21 | | jim | 20 | | sarah | | ------------------------------------------------------------------------------------------------ For the list let list = [ {name: bob, age: 21} {name: jim, age: 20} {name: sarah} {name: timothy, age: 50} {name: paul} ] Running "$list | to md --per-element --pretty" outputs: | name | age | | ------- | --- | | bob | 21 | | jim | 20 | | timothy | 50 | | name | | ----- | | sarah | | paul | ``` # User-Facing Changes The ```to md``` behaves as expected when piping a table that contains empty values showing all rows and the empty items replaced with whitespace. # Tests + Formatting Added 2 test cases to cover both issues. fmt + clippy OK. # After Submitting The command documentation needs to be updated with an example for when you want to "separate list into markdown tables" --- crates/nu-command/src/formats/to/md.rs | 59 +++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/nu-command/src/formats/to/md.rs b/crates/nu-command/src/formats/to/md.rs index 434b65a8d9..def4060257 100644 --- a/crates/nu-command/src/formats/to/md.rs +++ b/crates/nu-command/src/formats/to/md.rs @@ -36,13 +36,13 @@ impl Command for ToMd { Example { description: "Outputs an MD string representing the contents of this table", example: "[[foo bar]; [1 2]] | to md", - result: Some(Value::test_string("|foo|bar|\n|-|-|\n|1|2|\n")), + result: Some(Value::test_string("|foo|bar|\n|-|-|\n|1|2|")), }, Example { description: "Optionally, output a formatted markdown string", example: "[[foo bar]; [1 2]] | to md --pretty", result: Some(Value::test_string( - "| foo | bar |\n| --- | --- |\n| 1 | 2 |\n", + "| foo | bar |\n| --- | --- |\n| 1 | 2 |", )), }, Example { @@ -57,6 +57,13 @@ impl Command for ToMd { example: "[0 1 2] | to md --pretty", result: Some(Value::test_string("0\n1\n2")), }, + Example { + description: "Separate list into markdown tables", + example: "[ {foo: 1, bar: 2} {foo: 3, bar: 4} {foo: 5}] | to md --per-element", + result: Some(Value::test_string( + "|foo|bar|\n|-|-|\n|1|2|\n|3|4|\n|foo|\n|-|\n|5|", + )), + }, ] } @@ -94,11 +101,14 @@ fn to_md( grouped_input .into_iter() .map(move |val| match val { - Value::List { .. } => table(val.into_pipeline_data(), pretty, config), + Value::List { .. } => { + format!("{}\n", table(val.into_pipeline_data(), pretty, config)) + } other => fragment(other, pretty, config), }) .collect::>() - .join(""), + .join("") + .trim(), head, ) .into_pipeline_data_with_metadata(Some(metadata))); @@ -152,7 +162,13 @@ fn collect_headers(headers: &[String]) -> (Vec, Vec) { } fn table(input: PipelineData, pretty: bool, config: &Config) -> String { - let vec_of_values = input.into_iter().collect::>(); + let vec_of_values = input + .into_iter() + .flat_map(|val| match val { + Value::List { vals, .. } => vals, + other => vec![other], + }) + .collect::>(); let mut headers = merge_descriptors(&vec_of_values); let mut empty_header_index = 0; @@ -464,6 +480,39 @@ mod tests { ); } + #[test] + fn test_empty_row_value() { + let value = Value::test_list(vec![ + Value::test_record(record! { + "foo" => Value::test_string("1"), + "bar" => Value::test_string("2"), + }), + Value::test_record(record! { + "foo" => Value::test_string("3"), + "bar" => Value::test_string("4"), + }), + Value::test_record(record! { + "foo" => Value::test_string("5"), + "bar" => Value::test_string(""), + }), + ]); + + assert_eq!( + table( + value.clone().into_pipeline_data(), + false, + &Config::default() + ), + one(r#" + |foo|bar| + |-|-| + |1|2| + |3|4| + |5|| + "#) + ); + } + #[test] fn test_content_type_metadata() { let mut engine_state = Box::new(EngineState::new());