search

radareorg / r2pipe.rs

Rust crate for r2pipe
Other
45 stars 19 forks source link

fix json mapping for multiple structs #22

Closed r-jenish closed 7 years ago

radare commented 7 years ago

Cant this be done with a Derive for Serde? Looks to me like there mist be a better way to do that

On 28 Jun 2017, at 20:53, Rakholiya Jenish notifications@github.com wrote:

You can view, comment on, or merge this pull request online at:

https://github.com/radare/r2pipe.rs/pull/22

Commit Summary

fix json mapping for multiple structs File Changes

M Cargo.toml (6) M src/structs.rs (31) Patch Links:

https://github.com/radare/r2pipe.rs/pull/22.patch https://github.com/radare/r2pipe.rs/pull/22.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

r-jenish commented 7 years ago

Not sure. I will have to look further into this tomorrow. Though the attribute for struct fields that remains same in the json format too (e.g. name), can be removed. It might make the code somewhat better.

chinmaydd commented 7 years ago

If we are to change field names when converting to higher level structs giving more importance to API semantics, I think @P4N74 's approach is simpler and more easier to maintain in the long run.

chinmaydd commented 7 years ago

@P4N74 I have committed these changes under your authorship in the radare2-r2pipe-api repository. I hope that's fine :) I think this PR can be closed now.