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

Feature/255 keep fields ordered marshal struct #266

Conversation

brendesp
Copy link
Contributor

@brendesp brendesp commented Apr 1, 2019

Here is a fix for #255

Tracking the order of members as they are marshaled so they can be written in order of the structure. Default behavior stays the same.

New func MarshalOrdered() allows a marshalOrder to be passed in. Current options are OrderAlphabetical (default) and OrderPreserve

@codecov
Copy link

codecov bot commented Apr 1, 2019

Codecov Report

Merging #266 into master will increase coverage by 1.25%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
+ Coverage   90.72%   91.97%   +1.25%     
==========================================
  Files           9        9              
  Lines        1789     1845      +56     
==========================================
+ Hits         1623     1697      +74     
+ Misses        120      103      -17     
+ Partials       46       45       -1
Impacted Files Coverage Δ
marshal.go 90.61% <100%> (+0.21%) ⬆️
toml.go 85.22% <85.71%> (+6.84%) ⬆️
tomltree_write.go 85.09% <93.6%> (+4.7%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9070d3...f647046. Read the comment docs.

@pelletier
Copy link
Owner

Thank you so much for tackling that issue! I know it will make a lot of people happy.

Overall the code looks good. I think writeToOrdered will make it easier to fix #216.

However, I don't think introducing a MarshalOrdered function is a good interface. There is already a way to tweak the marshaling operation: the Encoder. I am thinking something like:

var buf bytes.Buffer
err := toml.NewEncoder(&buf).Order(toml.OrderPreserve).Encode(v)

A bit more verbose, but reuses the existing structure and allows for more flexibility, while keeping the API as small as possible. Definitely open to suggestions though!

Thank you again.

@brendesp
Copy link
Contributor Author

brendesp commented Apr 2, 2019

I implemented you suggestions.

@pelletier
Copy link
Owner

Looking great! Thank you!

@pelletier pelletier merged commit 63909f0 into pelletier:master Apr 2, 2019
@brendesp brendesp deleted the feature/255-keep-fields-ordered-marshal-struct branch April 10, 2019 15:41
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

Successfully merging this pull request may close these issues.

2 participants