first off: this is an awesome code base!
could use some comment on the abstract software design (how the pieces of code relate/interact with each other) but with Go To Definition it's actually quite easy to navigate
(and you said the API is not final, so it might not be a good idea to invest in docs right now anyway)
one thing that surprised me was the
value::ValueType enum
but it could be used to implement nice indexing, serde_json style
(also cf alexcrichton/toml-rs#193)
on a most abstract level, i think the API should try to be close to that of serde_json, because a lot of thought has been put into making that nice
and then, on top of that, let's add custom methods like
insert_at, and sort_entries_by
do you have any plans to make this is general toml crate? i.e., add serde support?
(nice to see you downloading the run-fuzzer.sh script I wrote for rust-fuzz/targets :D)
hey, @killercup! Thanks for sharing your thoughts!
About indexing: yeah serje_json indexing was an inspiration for that struct.
About fuzz: I wasn't able to run fuzzer on my local machine due to some asan error, but I've seen some false positives when running sanitizers.
The current problem with making api as nice as serde_json has is that serde_json and toml-rs have a single enum - Value, while in toml_edit we have Value, ArrayOrTables, Table - and they are different in the way you can own them. E.g. you can only have a reference to Table, while Value permits full ownership semantics.
About indexing: yeah serje_json indexing was an inspiration for that struct.
About fuzz: I wasn't able to run fuzzer on my local machine due to some asan error, but I've seen some false positives when running sanitizers.
The current problem with making api as nice as serde_json has is that serde_json and toml-rs have a single enum - Value, while in toml_edit we have Value, ArrayOrTables, Table - and they are different in the way you can own them. E.g. you can only have a reference to Table, while Value permits full ownership semantics.
if this is not meant to be used to parse 10GB files, it's fine use just add a bunch of Rc/Arc
is it possible for us to use a single
Valueenum?
Well, we have TableChild and TableChildMut, this basically covers all types (except for the document).
if this is not meant to be used to parse 10GB files, it's fine use just add a bunch of Rc/Arc
The problem with Rc is not only performance consideration, but also that it's painful to use without non-lexical lifetimes. Actually early version used Rc as pointer to the document in table and array of tables, but I switched to raw pointers later (it's was before initial public commit).
@ordian I saw you mentioning that the cargo-add command was blocked by the two issues on
I don't have that much Rust experience, but I would try me best :smile:
toml_edit. Would love to help with thatI don't have that much Rust experience, but I would try me best :smile:
@AZanellato hi there, glad to hear that :) I'll add more context to #57 soon. You can post any questions here and I'll try to respond in a timely fashion.
@ironyman hello! Would you be interested in adding toml 0.5 support? ordian/toml_edit#58
This would require changing a parser (https://github.com/ordian/toml_edit/tree/master/src/parser) as well as e.g. adding new Value type "dotted keys".
This would require changing a parser (https://github.com/ordian/toml_edit/tree/master/src/parser) as well as e.g. adding new Value type "dotted keys".
Hi @ordian ! So, I'm checking the codebase and from what I am gathering I would need to change the document parser here: https://github.com/ordian/toml_edit/blob/master/src/parser/document.rs Which uses the macros defined here:
https://github.com/ordian/toml_edit/blob/master/src/parser/macros.rs
https://github.com/ordian/toml_edit/blob/master/src/parser/macros.rs
Is that it? :smile:
@AZanellato I guess that would require changing all the parsers, including the macros. The implementation of
Document::from_str would first convert &str to Token stream and then use document parser on top of tokens. This probably means changing error type to include Token error.