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

Can I add TOML support? #40

Closed
benfaerber opened this issue Nov 11, 2023 · 5 comments
Closed

Can I add TOML support? #40

benfaerber opened this issue Nov 11, 2023 · 5 comments
Labels
on ice Not relevant now, pending some external blocker.

Comments

@benfaerber
Copy link

benfaerber commented Nov 11, 2023

Just watched your talk, love it! I work with PHP and Rust professionally so I'd love to convince my team to use this. I would like to add TOML support if you are open to it.

@Crell
Copy link
Owner

Crell commented Nov 12, 2023

TOML is definitely on the todo list. I looked into it a while back, and found one existing PHP library that is... rather dated but probably still works?

https://github.com/leonelquinteros/php-toml

(If you know a better one, I'm all ears.)

For the most part, TOML should be reasonably straightforward; its Formatter would probably look an awful lot like the YAML formatter. The catch is that TOML has variable depth structure; its section headers can be either section headers or inline, at any level. That's no issue on deserializing, but when serializing, how do you know what level it should switch from headers to inline key names? I haven't figured out the answer to that yet, as I don't know what's conventional in the TOML-using community. (That said, I've not spent much time trying to answer it. It may be that just defaulting to "2 levels in header, rest inline" or something would cover the 95% use case, with a constructor override on the formatter like YAML does. If that is the case, I think that's a reasonable solution.)

@benfaerber
Copy link
Author

benfaerber commented Nov 12, 2023

I haven't tested php-toml personally but looking at the TOML website, it is only compliant with v0.4.0 compliant of TOML where most of the other implementations work with version 1.0.0.

These 2 exist as well but same issue, only compliant with v0.4.0:
https://github.com/yosymfony/toml
https://github.com/betrixed/Toml-Pun8

So a good first step might be to either upgrade those to 1.0.0, write a new library (probably out of scope), or just say "only 0.4.0 is currently supported".

I looked around at some popular Rust crates Cargo.toml file. From what I can tell they nest the headers as deeply as possible and then have all the fields below (except for specifying package path and version). Maybe it could be all inline, except for the last 1-2 headers depending on property count. It would probably be good to look at some other larger files to see if they do things differently. We could also just look at the most popular serializers and copy what they do (to please the largest amount of users)

I just threw a bunch of examples into this:

[workspace]
resolver = "2"
members = [
    "actix-files",
]

[workspace.package]
license = "MIT OR Apache-2.0"
edition = "2021"
rust-version = "1.68"

[profile.dev]
debug = 0

[profile.release]
lto = true
opt-level = 3
codegen-units = 1

[patch.crates-io]
actix-files = { path = "actix-files" }
actix-http = { path = "actix-http" }

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
rustdoc-args = ["--generate-link-to-definition"]

I looked at these files:
https://github.com/serde-rs/serde/blob/master/Cargo.toml
https://github.com/actix/actix-web/blob/master/Cargo.toml
https://github.com/dtolnay/quote/blob/master/Cargo.toml
https://github.com/SergioBenitez/Rocket/blob/master/core/lib/Cargo.toml

It could be different in other ecosystems but to my knowledge Rust is the most popular user of TOML.

@Crell
Copy link
Owner

Crell commented Nov 13, 2023

Hm. Yeah, I'm not finding any TOML 1.0-compatible libraries for PHP so far. I'm not sure adding 0.4 support to Serde directly is wise, given how out of date it is. Building a new TOML library would definitely be out of scope; that should be its own library that Serde (and others) can leverage. Or upgrading one of the existing libraries, which is then a matter for those libraries.

A Serde-TOML 0.4 bridge should be feasible, but I'd rather not include that directly. Adding additional formatters is trivial, though, so easily done outside of Serde itself.

I'll leave this ticket open for now, but I consider it on ice until there's a TOML 1.0 library we can leverage. At that point, I'm full on board with adding it.

@Crell Crell added the on ice Not relevant now, pending some external blocker. label Nov 13, 2023
@Crell
Copy link
Owner

Crell commented Aug 9, 2024

This may have resolved itself: https://github.com/vanodevium/toml

@Crell
Copy link
Owner

Crell commented Sep 20, 2024

Now available in Serde 1.3.

@Crell Crell closed this as completed Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on ice Not relevant now, pending some external blocker.
Projects
None yet
Development

No branches or pull requests

2 participants