Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toml_edit: value is not written if table is constructed beforehand #762

Open
tingerrr opened this issue Jul 28, 2024 · 5 comments
Open

toml_edit: value is not written if table is constructed beforehand #762

tingerrr opened this issue Jul 28, 2024 · 5 comments
Labels
A-edit Area: TOML editing API C-bug Category: Things not working as expected M-breaking-change Meta: Implementing or merging this will introduce a breaking change.

Comments

@tingerrr
Copy link

The following code simply discards the last write to the tests sub-key.

use toml_edit::{DocumentMut, value};

fn main() {
    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    // doc["tool"] = toml_edit::table();
    // doc["tool"]["typst-test"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());
}

Produces the following output:

tool = { typst-test = { tests = "tests" } }

Uncommenting the second line produces:

tool = {}

Uncommenting the second and first line produces:

[tool]

[tool.typst-test]
tests = "tests"

When inspecting the table tool.typst-test, the value exists, it's simply not written.

@epage
Copy link
Member

epage commented Jul 29, 2024

I've updated the reproduction case to make it easier to follow

#!/usr/bin/env nargo
---
[dependencies]
toml_edit = "0.22"
---

use toml_edit::{DocumentMut, value};

fn main() {
    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());

    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());

    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"]["typst-test"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());

    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"] = toml_edit::table();
    doc["tool"]["typst-test"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());
}
$ ./toml_edit-762.rs
warning: `package.edition` is unspecified, defaulting to `2021`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.05s
     Running `/home/epage/.cargo/target/84/93c6f0d3e9e9a2/debug/toml_edit-762`
doc:
tool = { typst-test = { tests = "tests" } }


doc:
[tool]
typst-test = { tests = "tests" }


doc:
tool = {}


doc:
[tool]

[tool.typst-test]
tests = "tests"

@epage
Copy link
Member

epage commented Jul 29, 2024

To understand whats going on, I'm going to show the debug repr

[/home/epage/.cargo/target/84/93c6f0d3e9e9a2/toml_edit-762.rs:25:5] &doc = DocumentMut {
    root: Table(
        Table {
            decor: Decor {
                prefix: "default",
                suffix: "default",
            },
            implicit: false,
            dotted: false,
            doc_position: None,
            span: None,
            items: {
                "tool": TableKeyValue {
                    key: Key {
                        key: "tool",
                        repr: None,
                        leaf_decor: Decor {
                            prefix: "default",
                            suffix: "default",
                        },
                        dotted_decor: Decor {
                            prefix: "default",
                            suffix: "default",
                        },
                    },
                    value: Value(
                        InlineTable(
                            InlineTable {
                                preamble: empty,
                                implicit: false,
                                decor: Decor {
                                    prefix: "default",
                                    suffix: "default",
                                },
                                span: None,
                                dotted: false,
                                items: {
                                    "typst-test": TableKeyValue {
                                        key: Key {
                                            key: "typst-test",
                                            repr: None,
                                            leaf_decor: Decor {
                                                prefix: "default",
                                                suffix: "default",
                                            },
                                            dotted_decor: Decor {
                                                prefix: "default",
                                                suffix: "default",
                                            },
                                        },
                                        value: Table(
                                            Table {
                                                decor: Decor {
                                                    prefix: "default",
                                                    suffix: "default",
                                                },
                                                implicit: false,
                                                dotted: false,
                                                doc_position: None,
                                                span: None,
                                                items: {
                                                    "tests": TableKeyValue {
                                                        key: Key {
                                                            key: "tests",
                                                            repr: None,
                                                            leaf_decor: Decor {
                                                                prefix: "default",
                                                                suffix: "default",
                                                            },
                                                            dotted_decor: Decor {
                                                                prefix: "default",
                                                                suffix: "default",
                                                            },
                                                        },
                                                        value: Value(
                                                            String(
                                                                Formatted {
                                                                    value: "tests",
                                                                    repr: "default",
                                                                    decor: Decor {
                                                                        prefix: "default",
                                                                        suffix: "default",
                                                                    },
                                                                },
                                                            ),
                                                        ),
                                                    },
                                                },
                                            },
                                        ),
                                    },
                                },
                            },
                        ),
                    ),
                },
            },
        },
    ),
    trailing: empty,
}

InlineTables are implicitly being created. When we go to render the values in an InlineTable but we come across a Table, we ignore it.

@epage
Copy link
Member

epage commented Jul 29, 2024

Unsure how well this is documented (I wanted to do the above before confirming) but

doc["tool"]["typst-test"]["tests"] = value("tests");

always produces inline tables for the intermediate tables and its left to the caller to put in standard tables, correctly, if they want that.

The question is what we should do moving forward

  • Leave it as-is
  • Add an assert when inserting, with [], a standard table into a Value
  • Change the rendering code to implicitly convert standard tables into inline tables / arrays.

@tingerrr
Copy link
Author

always produces inline tables for the intermediate tables and its left to the caller to put in standard tables, correctly, if they want that.

I think that's the part that confused me when I initially used it (it makes somewhat sense in hindsight). An assertion seems like the best option to me, implicit conversion could cause problems elsewhere and be even more confusing, and leaving it as is, just seems wrong. It should definitely not swallow whatever I put in there without warning.

@epage
Copy link
Member

epage commented Aug 1, 2024

Created a test case

#[test]
fn table_under_inline() {
    let mut doc = DocumentMut::new();
    doc["tool"]["typst-test"] = table();
    doc["tool"]["typst-test"]["tests"] = value("tests");

    assert_data_eq!(doc.to_string(), str![[r#"
tool = {}

"#]]);
}

Another approach is we switch from creating inline tables implicitly to we create Tables implicitly unless we are in a Value and then we create an InlineTable. While it doesn't solve all problems, it will make it so there is less of a chance of people using the API incorrectly.

@epage epage added C-bug Category: Things not working as expected M-breaking-change Meta: Implementing or merging this will introduce a breaking change. A-edit Area: TOML editing API labels Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edit Area: TOML editing API C-bug Category: Things not working as expected M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Development

No branches or pull requests

2 participants