search

Pelagicore / gdbus-codegen-glibmm

Code generator for C++ D-Bus stubs and proxies using Giomm/Glibmm
GNU Lesser General Public License v2.1
23 stars 25 forks source link

Use jinja2 instead of inlined strings for *_common.h #11

Closed jonte closed 6 years ago

jonte commented 6 years ago

Submitting PR to show this work in progress - don't merge this, but please have a look and come with feedback!

mardy commented 6 years ago

This looks soo much better! I'd propose we first create some unit tests which simply compare the output files with the expected output, before merging this. This would help us be confident that we are not introducing regressions.

jonte commented 6 years ago

Yep. I agree!

mardy commented 6 years ago

Hi @jonte, please have a look at this branch: https://github.com/mardy/gdbus-codegen-glibmm/tree/jonte-feature-jinja

It contains you commit here (rebased on top of latest master), plus some minor changes to reduce the differences in the generated files. I'd try to keep as much of the logic as it was, and try to have the generated files as close as possible to the original ones.

In particular, I find the implementation of unique_return_types very hard to read. I think that we should try to break it into multiple lines and make it clear what it's doing. Preserving the order of the generated methods would be a big plus.

mardy commented 6 years ago

OK, I've cleaned up my branch and made it generate nearly identical code as the current one. We'll beautify it later, so that the git history will be easier to follow.

Let me know, if you are going to merge my branch into yours, or if you prefer me to create a new MP and close this one.

jonte commented 6 years ago

@mardy I'll merge your branch into this one, and we can merge this MR.

jonte commented 6 years ago

@mardy I squashed your stuff into mine, I hope that's OK with you.

mardy commented 6 years ago

Looks good to me!