toml-rs / toml

Rust TOML Parser
https://docs.rs/toml
Apache License 2.0
719 stars 105 forks source link

Allow setting key's representation #753

Open ip1981 opened 3 months ago

ip1981 commented 3 months ago

While working on a library to access TOML data via a "TOML path" like foo[1].bar, similar to toml-cli, we found convenient to reuse the Key struct. At the same time, reusing the key's parser provided by the public interface was not only tricky, but didn't cover some additional feature requirements (like wildcards). Another feature is that keys can come from a TOML file parsed with the toml_edit. That's why reusing Key is desired.

Current public interface does not allow creating a Key and setting its representation (Repr).

I propose making Repr::new_unchecked public and renamed to Repr::new (because Repr is self-contained, there are no invariants inside it, nothing to "check"), and making Key::with_repr_unchecked public as well, renamed to Key::with_repr.

These changes will allow writing custom parsers producing keys like this:

//...
.with_recognized()
    .map(|(o, i)| toml_edit::Key::new(o).with_repr(toml_edit::Repr::new(i)))
    .parse_next(input)

Here is the patch: https://github.com/whamcloud/toml/commit/7f9d96077b2cfd5fc30377304958be081c9e7b7f

epage commented 3 months ago

Note that this was created out of #751

epage commented 3 months ago

I propose making Repr::new_unchecked public and renamed to Repr::new (because Repr is self-contained, there are no invariants inside it, nothing to "check"),

What is "unchecked" is that the string must conform to the TOML specification. We currently allow "unchecked" content with Decor and view that as non-ideal (#267).

As the community seems to feel that unsafe should be reserved for language invariants, and not just library invariants, there also wouldn't be a good way to call out and audit these "unchecked" calls.