Refactor to md and Add Padding for Pretty Flag (#2678)

* refactor and cleanup to md

* Add padding around values in each row

* Add padding to test

* Update code to satisfy Clippy and pass other failing tests

crates/nu-cli/src/commands/to_md.rs

@@ -60,44 +60,78 @@ async fn to_md(args: CommandArgs, registry: &CommandRegistry) -> Result<OutputSt
     let (ToMarkdownArgs { pretty }, input) = args.process(&registry).await?;
     let input: Vec<Value> = input.collect().await;
     let headers = nu_protocol::merge_descriptors(&input);
-    let mut output_string = String::new();
 
-    let mut column_length_vector: Vec<usize> = Vec::new();
+    let mut escaped_headers: Vec<String> = Vec::new();
+    let mut column_widths: Vec<usize> = Vec::new();
 
-    if pretty {
+    if !headers.is_empty() && (headers.len() > 1 || headers[0] != "") {
-        if !headers.is_empty() && (headers.len() > 1 || headers[0] != "") {
+        for header in &headers {
-            for header in &headers {
+            let escaped_header_string = htmlescape::encode_minimal(&header);
-                let htmlescape_header_string = &htmlescape::encode_minimal(&header);
+            column_widths.push(escaped_header_string.len());
-                column_length_vector.push(htmlescape_header_string.len());
+            escaped_headers.push(escaped_header_string);
-            }
-        }
-
-        for row in &input {
-            if let UntaggedValue::Row(row) = row.value.clone() {
-                for i in 0..headers.len() {
-                    let data = row.get_data(&headers[i]);
-                    let new_column_length = format_leaf(data.borrow()).plain_string(100_000).len();
-
-                    if column_length_vector[i] < new_column_length {
-                        column_length_vector[i] = new_column_length;
-                    }
-                }
-            }
         }
     }
 
-    if !headers.is_empty() && (headers.len() > 1 || headers[0] != "") {
+    let mut escaped_rows: Vec<Vec<String>> = Vec::new();
+
+    for row in &input {
+        let mut escaped_row: Vec<String> = Vec::new();
+
+        match row.value.clone() {
+            UntaggedValue::Row(row) => {
+                for i in 0..headers.len() {
+                    let data = row.get_data(&headers[i]);
+                    let value_string = format_leaf(data.borrow()).plain_string(100_000);
+                    let new_column_width = value_string.len();
+
+                    escaped_row.push(value_string);
+
+                    if column_widths[i] < new_column_width {
+                        column_widths[i] = new_column_width;
+                    }
+                }
+            }
+            p => {
+                let value_string =
+                    htmlescape::encode_minimal(&format_leaf(&p).plain_string(100_000));
+                escaped_row.push(value_string);
+            }
+        }
+
+        escaped_rows.push(escaped_row);
+    }
+
+    let output_string = get_output_string(&escaped_headers, &escaped_rows, &column_widths, pretty);
+
+    Ok(OutputStream::one(ReturnSuccess::value(
+        UntaggedValue::string(output_string).into_value(name_tag),
+    )))
+}
+
+fn get_output_string(
+    headers: &[String],
+    rows: &[Vec<String>],
+    column_widths: &[usize],
+    pretty: bool,
+) -> String {
+    let mut output_string = String::new();
+
+    if !headers.is_empty() {
         output_string.push_str("|");
 
         for i in 0..headers.len() {
-            let htmlescape_string = htmlescape::encode_minimal(&headers[i]);
+            if pretty {
-            let final_string = if pretty {
+                output_string.push_str(" ");
-                get_padded_string(htmlescape_string, column_length_vector[i], ' ')
+                output_string.push_str(&get_padded_string(
+                    headers[i].clone(),
+                    column_widths[i],
+                    ' ',
+                ));
+                output_string.push_str(" ");
             } else {
-                htmlescape_string
+                output_string.push_str(headers[i].as_str());
-            };
+            }
 
-            output_string.push_str(&final_string);
             output_string.push_str("|");
         }
 
@@ -105,56 +139,57 @@ async fn to_md(args: CommandArgs, registry: &CommandRegistry) -> Result<OutputSt
 
         #[allow(clippy::needless_range_loop)]
         for i in 0..headers.len() {
-            let final_string = if pretty {
+            if pretty {
-                "-".repeat(column_length_vector[i])
+                output_string.push_str(" ");
+                output_string.push_str(&get_padded_string(
+                    String::from("-"),
+                    column_widths[i],
+                    '-',
+                ));
+                output_string.push_str(" ");
             } else {
-                String::from("-")
+                output_string.push_str("-");
-            };
+            }
 
-            output_string.push_str(final_string.as_str());
             output_string.push_str("|");
         }
 
         output_string.push_str("\n");
     }
 
-    for row in input {
+    for row in rows {
-        match row.value {
+        if !headers.is_empty() {
-            UntaggedValue::Row(row) => {
+            output_string.push_str("|");
-                output_string.push_str("|");
+        }
 
-                for i in 0..headers.len() {
+        for i in 0..row.len() {
-                    let data = row.get_data(&headers[i]);
+            if pretty {
-                    let leaf_string = format_leaf(data.borrow()).plain_string(100_000);
+                output_string.push_str(" ");
-                    let final_string = if pretty {
+                output_string.push_str(&get_padded_string(row[i].clone(), column_widths[i], ' '));
-                        get_padded_string(leaf_string, column_length_vector[i], ' ')
+                output_string.push_str(" ");
-                    } else {
+            } else {
-                        leaf_string
+                output_string.push_str(row[i].as_str());
-                    };
-
-                    output_string.push_str(&final_string);
-                    output_string.push_str("|");
-                }
-
-                output_string.push_str("\n");
             }
-            p => {
+
-                output_string.push_str(
+            if !headers.is_empty() {
-                    &(htmlescape::encode_minimal(&format_leaf(&p).plain_string(100_000))),
+                output_string.push_str("|");
-                );
-                output_string.push_str("\n");
             }
         }
+
+        output_string.push_str("\n");
     }
 
-    Ok(OutputStream::one(ReturnSuccess::value(
+    output_string
-        UntaggedValue::string(output_string).into_value(name_tag),
-    )))
 }
 
-fn get_padded_string(text: String, desired_length: usize, character: char) -> String {
+fn get_padded_string(text: String, desired_length: usize, padding_character: char) -> String {
-    let padding_length = desired_length - text.len();
+    format!(
-    return format!("{}{}", text, character.to_string().repeat(padding_length));
+        "{}{}",
+        text,
+        padding_character
+            .to_string()
+            .repeat(desired_length - text.len())
+    )
 }
 
 #[cfg(test)]

crates/nu-cli/tests/format_conversions/markdown.rs

@@ -33,5 +33,5 @@ fn out_md_table_pretty() {
         "#
     ));
 
-    assert_eq!(actual.out, "|name  ||------||joseph|");
+    assert_eq!(actual.out, "| name   || ------ || joseph |");
 }