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

Support deserializing to a specific TOML type, rather than only toml::Value #363

Closed
bstrie opened this issue Oct 26, 2022 · 3 comments
Closed
Labels
A-parse Area: Parsing TOML C-enhancement Category: Raise on the bar on expectations

Comments

@bstrie
Copy link

bstrie commented Oct 26, 2022

I'm writing some code that ingests and manipulates TOML, and I have a use case where I expect a user to pass in a TOML array, e.g. ["a", "b"]. The following code doesn't work:

fn main() {
    let x: toml::Value = toml::from_str(r#"["a", "b"]"#).unwrap(); // error
    dbg!(x);
}

With a slight tweak, it's easy to see why this code can't ever work:

fn main() {
    let x: toml::Value = toml::from_str(r#"["a"]"#).unwrap();
    dbg!(x);
}

Output:

[src/main.rs:3] x = Table(
    {
        "a": Table(
            {},
        ),
    },
)

The ["a"] is interpreted as being the identifier of a top-level table, and without context ["a"] is simply ambiguous in TOML syntax.

The toml crate does expose a type toml::value::Array, but sadly the following doesn't work:

fn main() {
    let x: toml::value::Array = toml::from_str(r#"["a"]"#).unwrap(); // error
    dbg!(x);
}

Ideally I would see the following output:

[src/main.rs:3] y = Array(
    [
        String(
            "a",
        ),
    ],
)

(Obviously the API design needs some consideration, as I'm asking for a toml::value::Array but what I really want is a toml::Value::Array, but obviously we don't have first-class enum variants like that.)

Instead, the only supported workaround appears to be the following, which is somewhat gruesome:

fn main() {
    let x = r#"["a"]"#;
    let y: toml::Value = toml::from_str(&format!("dummy = {x}")).unwrap();
    let z = match y {
        toml::Value::Table(table) => table["dummy"].clone(),
        _ => unreachable!(),
    };
    dbg!(z);
}
@bstrie
Copy link
Author

bstrie commented Nov 1, 2022

Here's our use case, if you're interested in understand why we want to do this:

Our CLI application supports loading a TOML configuration file. However, while we encourage the use of the configuration file, in many contexts it's cumbersome to create files (e.g. CI, or cargo test), and it may be overkill to create an entire file if you only want to configure a single value, and even if you have a file you may want to temporarily override a single value specified in the file. Thus, our CLI supports overriding individual configuration values. Of course, some of our configuration values are rather elaborate, e.g. an array of custom objects, and rather than invent some CLI-native way of representing all of these we'd like to merely let people use the TOML syntax they're already using in the config file, so e.g. --files '[{kind = "stdin"}, {name = "bar", kind = "connect", host = "localhost", port = "23456" }]'. Using TOML rather than, say, JSON here is useful because it lets people easily copy/paste between the CLI and the config file.

@epage
Copy link
Member

epage commented Nov 3, 2022

The main challenge with supporting that for toml::from_str is that that isn't toml. Most likely, this would be exposed as FromStr impls for the individual types and then convert those to the application types. As this would be FromStr for specific forms of toml syntax, it seems like doing this within toml_edit would be best.

Note that cargo does something similar though its made simpler by the fact that --config accepts key = value which can be parsed as a TOML Document. We still used toml_edit to ensure no other syntax was used. See rust-lang/cargo#10176

@epage epage added A-parse Area: Parsing TOML C-enhancement Category: Raise on the bar on expectations labels Nov 3, 2022
@epage
Copy link
Member

epage commented Jan 18, 2023

Previously, toml::Deserializer would deserialize a document. If a type was incompatible with a document, it would deserialize it as a value

#457, this is more explicit with toml::Deserializer only deserializing a document and toml::ValueDeserializer that only deserializes values. This should remove the ambiguity that this issue is running into.

Closing as this seems to be resolved in main. If I missed something, let me know and we can re-open.

@epage epage closed this as completed Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parse Area: Parsing TOML C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants