Reflection-based marshaling / unmarshaling #149
Conversation
|
Good job! That's pretty much what I had in mind. I'd like to stay as close as possible to the behavior of |
tro3
changed the title
|
@pelletier - I've removed the WIP tag. This is ready for review. The only feature it does not have is the handling of @carolynvs, @sdboyer - Would love to hear feedback on how usable this appears in the |
|
Hmm, it occurs I still need documentation... Will add. |
|
@tro3 I think this is pretty close! 🎉 I would just need |
|
I'm on it. |
|
@carolynvs - |
marshal.go
Outdated
| val := tval.Get(key) | ||
| mvalf, err := valueFromToml(mtypef.Type, val) | ||
| if err != nil { | ||
| if err.Error()[0] == '(' { |
There was a problem hiding this comment.
Seems weird how to do some logic based off the first character of the error message. Do you mind elaborating on how this works?
There was a problem hiding this comment.
This is just to prevent every level of hierarchy adding its own location info. I'll take a look to see if there is a cleaner error handling method.
There was a problem hiding this comment.
Got it. If you don't find a cleaner way to do it, I think just adding a comment in places you do that is good enough for now. There is the broader case of how we report errors in toml that needs to be addressed too.
There was a problem hiding this comment.
Agreed. I've already moved to a separate function formatError with a comment describing the "why". Easier to rework later, if needed. Just fighting test coverage now - forcing omitempty onto pointers has resulted in some unreachable(/untestable) code.
marshal.go
Outdated
| func Marshal(v interface{}) ([]byte, error) { | ||
| mtype := reflect.TypeOf(v) | ||
| if mtype.Kind() != reflect.Struct { | ||
| return []byte{}, errors.New("Only a Struct can be marshaled to TOML") |
|
Will work on the above and add test coverage |
|
@pelletier - Incorporated the feedback and did indeed find some holes due to lack of test coverage. Let me know what more is needed. |
Going after #146, @pelletier this initial commit is just to give you a feel for the direction I am heading with this. Still need to add Map and Array handling, error handling (with position info for unmarshaling), and lots of tests. Please let me know your thoughts, so I can shift course early if needed.
One question - do you want to follow the
encoding/jsonpattern of ignoring non-exported fields?