Open gibbz00 opened 3 months ago
Unsure how much else there's to split up.
I suggest the following to make reviewing easier:
code_generator.rs
to separate modules, however the diff this PR is still a big red file and big green files. So I can't compare the two versions. Maybe a rebase will solve that.Hi, thank you for starting to look into this pr 😊
I'll see what I can do during the weekend.
Apil 13
Throwing this out here ASAP as a draft so that others can become aware of it. Will most definitely clash with PRs such as https://github.com/tokio-rs/prost/pull/1019.
Built upon #1020.(rebased)Will create a proper write-up once things become presentable.
Update (14 April)
Post
quote
+syn
POC made separation of concerns a lot clearer, which lead to a bunch of module splitting. Might be worth trying to send for these as separate PRs, which this draft then builds upon. Ex. placingunescape_c_escape_string
and its unit tests in a separate module.Update (21 April)
Think this is ready to be reviewed now.
Goals:
Assessment:
Still plenty left to be done. Refrained from making larger API changes before input from others, in addition to the experience gained from trying to add JSON mapping.
Similar to outcome of 1, but here an example of what has changed:
Example:
Before
After
Other examples
One takeaway it that this PR removes manual indentation handling, delegating it to the
format
feature.Possible future "improvements" which lead to breaking changes.
ServiceGenerator
API to pass a tokenTokenStream
/syn::Item
rather thanbuf: &mut String
. See tonic's usage: https://github.com/hyperium/tonic/blob/eeb3268f71ae5d1107c937392389db63d8f721fb/tonic-build/src/prost.rs#L247-L276config.field_attribute("in", "#[serde(rename = \"in\")]");
PR splitting
Waiting for #1029 and #1030 to get merged before rebasing this one on top of them. Unsure how much else there's to split up.