RUST-2019 Respect pretty printing flag for Document and Bson

[jadalilleboe]

RUST-2019

Didn't add a unit test because Display outputs to the console, let me know if this should be tested though
I did test manually with this code

let my_hashmap = HashMap::from([
        ("hello", "world!"), ("world", "hello"), ("key", "val")
      ]);
      
    let bson_document = to_bson(&my_hashmap).unwrap();
    println!("{bson_document}");
    println!("{bson_document:#}");

    let hm: HashMap<&str, &str> = HashMap::from([]);
    let md = to_document(&hm).unwrap();
    println!("{md}");
    println!("{md:#}");

which outputted

{ "hello": "world!", "world": "hello", "key": "val" }
{
 "hello": "world!", 
 "world": "hello", 
 "key": "val"
}
{}
{}

did the same test switching to_document instead of to_bson and got the same output.

[abr-egn]

You'll need some additional logic in there to handle nested documents, those should have the flag passed in and then be indented for the nesting.

Didn't add a unit test because Display outputs to the console, let me know if this should be tested though

I do think these need some tests. Happily, Display and Debug are actually agnostic to where they're being written to :) You can use the format! macro to get a String output. Also, you can construct a BSON Document more conveniently with the doc! macro.

As a side note, we usually do development on our own forks of the repository so the main repo doesn't accumulate dev branches (and to make accidental main-branch-clobbering less likely, not that yours truly would have done that on his first day). No need to switch for this PR, just to know for the future.

[jadalilleboe]

@abr-egn Added some logic for nested documents and added some tests, let me know what you think. also thought it would be good to add some logics for arrays as well:

{
 "hello": [
  1, 
  2, 
  3
 ]
}

noted about the forks, will adhere to that in the future

[abr-egn]

Good thought on the array formatting!

It looks like there's some extra spacing on initial indented lines - I tried this test:

#[test]
fn test_pretty_printing() {
    let d = doc! { "hello": "world!", "world": "hello", "key": "val" };
    let expected = r#"{ "hello": "world!", "world": "hello", "key": "val" }"#;
    let formatted = format!("{d}");
    assert_eq!(
        expected, formatted,
        "expected:\n{expected}\ngot:\n{formatted}"
    );

    let d = doc! { "hello": "world!", "nested": { "key": "val", "double": { "a": "thing" } } };
    let expected = r#"{
 "hello": "world",
 "nested": {
  "key": "val",
  "double": {
   "a": "thing"
  }
 }
}"#;
    let formatted = format!("{d:#}");
    assert_eq!(
        expected, formatted,
        "expected:\n{expected}\ngot:\n{formatted}"
    );
}

and the output was

assertion `left == right` failed: expected:
{
 "hello": "world",
 "nested": {
  "key": "val",
  "double": {
   "a": "thing"
  }
 }
}
got:
{
 "hello": "world!", 
 "nested": {
   "key": "val", 
  "double": {
     "a": "thing"
  }
 }
}
  left: "{\n \"hello\": \"world\",\n \"nested\": {\n  \"key\": \"val\",\n  \"double\": {\n   \"a\": \"thing\"\n  }\n }\n}"
 right: "{\n \"hello\": \"world!\", \n \"nested\": {\n   \"key\": \"val\", \n  \"double\": {\n     \"a\": \"thing\"\n  }\n }\n}"

[jadalilleboe]

@abr-egn thanks for pointing that out, I fixed that error and added some more tests which cover that case

[abr-egn]

LGTM! Tagging in Isabel.

[isabelatkinson]

just a few style suggestions and a question!

[isabelatkinson]

this can be made a bit more concise with:

let indent_str = if let Some(width) = fmt.width() {
    " ".repeat(width)
} else {
    "".to_string()
};

(ditto elsewhere with a similar pattern)

[jadalilleboe]

I updated this to use unwrap_or to be more concise

[isabelatkinson]

suggest using unwrap_or here:

let mut indent = fmt.width().unwrap_or(0);

[jadalilleboe]

Thanks for bringing this method to my attention!

[isabelatkinson]

any reason to indent by 1 space? I think 2 spaces would be more readable, but not sure if there's a convention we want to follow here

[jadalilleboe]

no real reason, I agree that 2 spaces would be more readable and I updated this

[isabelatkinson]

lgtm!