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

Allow setting key's representation #753

Open
ip1981 opened this issue Jul 15, 2024 · 2 comments
Open

Allow setting key's representation #753

ip1981 opened this issue Jul 15, 2024 · 2 comments
Labels
A-output Area: Outputting TOML C-enhancement Category: Raise on the bar on expectations

Comments

@ip1981
Copy link

ip1981 commented Jul 15, 2024

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: whamcloud@7f9d960

@epage
Copy link
Member

epage commented Jul 15, 2024

Note that this was created out of #751

@epage
Copy link
Member

epage commented Jul 15, 2024

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.

@epage epage added C-enhancement Category: Raise on the bar on expectations A-output Area: Outputting TOML labels Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-output Area: Outputting TOML C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants