Skip to content

Commit

Permalink
fix(ser): Render empty tables when using to_string_pretty
Browse files Browse the repository at this point in the history
I considered making it so we always showed empty implicit tables but I
figured that in the editing case, we could show tables that were
originally hidden just by deleting entries.  This seems surprising for a
user and they shouldn't have to account for it.

So I limited the fix to `to_string_pretty`.

This was found by rust-lang/cargo#10349.
  • Loading branch information
epage committed Jan 31, 2022
1 parent e869ac6 commit 217ccd4
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
12 changes: 11 additions & 1 deletion src/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,16 @@ fn visit_table(
is_array_of_tables: bool,
) -> Result {
let children = table.get_values();
// We are intentionally hiding implicit tables without any tables nested under them (ie
// `table.is_empty()` which is in contrast to `table.get_values().is_empty()`). We are
// trusting the user that an empty implicit table is not semantically meaningful
//
// This allows a user to delete all tables under this implicit table and the implicit table
// will disappear.
//
// However, this means that users need to take care in deciding what tables get marked as
// implicit.
let is_visible_std_table = !(table.implicit && children.is_empty());

if path.is_empty() {
// don't print header for the root node
Expand All @@ -216,7 +226,7 @@ fn visit_table(
"]]{}",
table.decor.suffix().unwrap_or(DEFAULT_TABLE_DECOR.1)
)?;
} else if !(table.implicit && children.is_empty()) {
} else if is_visible_std_table {
write!(
buf,
"{}[",
Expand Down
6 changes: 5 additions & 1 deletion src/ser/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ impl crate::visit_mut::VisitMut for Pretty {

fn visit_table_mut(&mut self, node: &mut crate::Table) {
node.decor_mut().clear();
node.set_implicit(true);

// Empty tables could be semantically meaningful, so make sure they are not implicit
if !node.is_empty() {
node.set_implicit(true);
}

crate::visit_mut::visit_table_mut(self, node);
}
Expand Down
13 changes: 13 additions & 0 deletions tests/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,19 @@ fn table_array() {
assert_eq!(toml, &result);
}

const PRETTY_EMPTY_TABLE: &str = r#"[example]
"#;

#[test]
fn pretty_empty_table() {
let toml = PRETTY_EMPTY_TABLE;
let value: toml_edit::easy::Value = toml_edit::easy::from_str(toml).unwrap();
let result = toml_edit::easy::to_string_pretty(&value).unwrap();
println!("EXPECTED:\n{}", toml);
println!("\nRESULT:\n{}", result);
assert_eq!(toml, &result);
}

#[test]
fn error_includes_key() {
#[derive(Debug, serde::Serialize, serde::Deserialize)]
Expand Down

0 comments on commit 217ccd4

Please sign in to comment.