Crell / Serde

Robust Serde (serialization/deserialization) library for PHP 8.
Other
299 stars 14 forks source link

Can I add TOML support? #40

Closed benfaerber closed 2 months ago

benfaerber commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 3 months ago

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

Crell commented 2 months ago

Now available in Serde 1.3.