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

Iterating on toml_edits API #122

Closed
epage opened this issue Aug 23, 2021 · 3 comments
Closed

Iterating on toml_edits API #122

epage opened this issue Aug 23, 2021 · 3 comments

Comments

@epage
Copy link
Member

epage commented Aug 23, 2021

In working on TOML 1.0 compliance, some things have come up (#120)

  • External crates need access to a programmatically meaningful version of each date/time variant
    • Even if cfg(test) tricks work for black box tests, I'm going to be later be using toml_edit in toml_test as the baseline for testing encoders and validating against toml-rs
    • In theory, toml users will want this as we make this crate viable for format-preserving and regular use
  • Nesting all date-time variants into a single Value::DateTime is verbose and we don't do it for integer/fl;oat (feat: Publicly expose DateTime #120, style: Silence clippy #116)
  • As we implement toml-rs on top of toml_edit, where should it live? A separate crate, a toml_edit::simple module?
  • End-users having to distinguish between Table and InlineTable is already a challenging and adding dotted keys is going to make this more challenging
  • Dotted keys opens up additional API questions (Support dotted keys #73)

With additional inputs

@epage
Copy link
Member Author

epage commented Aug 23, 2021

Proposal:

DateTime variants are flattened into Value with newtypes that implement Display and FromStr using a strict, advertised format so people can integrate this with whatever date library they want

toml-rs lives as toml_edit::simple

We take a lesson from tomlkit and consolidate all table types.

  • Table type is now an enum that users can set (with convenience methods)
  • We automatically promote between table types to satisfy toml rules ([feature] Table -> InlineTable #74)
    We best-guess formatting based on other tables. If users want more precise formatting, they can specify the table type and formatting explicitly
  • In rare cases (Invalid ArrayOfTables but can't be InlineTable), we error on serialization and offer a verify function

@ordian
Copy link
Member

ordian commented Aug 23, 2021

End-users having to distinguish between Table and InlineTable is already a challenging and adding dotted keys is going to make this more challenging

for Table vs InlineTable we have AsTableLike trait, but no AsTableLikeMut for now because I didn't know how to make it type-safe (e.g. accidentally inserting a table into an inline table)
dotted keys indeed add more complexity, but it seems inherent rather than accidental

@ordian
Copy link
Member

ordian commented Aug 23, 2021

Your proposal sounds good to me. One thing I'm not sure about is

We best-guess formatting based on other tables

I think we should make as predictable for users as possible with no magic involved.

epage added a commit to epage/toml_edit that referenced this issue Aug 24, 2021
This creates newtypes for each date/time type that enforce their format
and allow conversion to/from it by the user.

This also flattens `DateTime` into `Value`.

This is part of toml-rs#122
ordian pushed a commit that referenced this issue Aug 24, 2021
* fix(repr): Loosen trait bounds

* fix(datetime): Remove chrono from API

This creates newtypes for each date/time type that enforce their format
and allow conversion to/from it by the user.

This also flattens `DateTime` into `Value`.

This is part of #122
@epage epage closed this as completed Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants