dearimgui / dear_bindings

C header (and language binding metadata) generator for Dear ImGui
MIT License
221 stars 12 forks source link

Defines not included in metadata #8

Closed emoon closed 2 years ago

emoon commented 2 years ago

Defines such as IM_UNICODE_CODEPOINT_MAX isn't included in the metadata. It would be good to include this as some of them will affect code-generation of the bindings in important ways (such as ImWchar that can be either 16 or 32 bit)

ShironekoBen commented 2 years ago

Oops! Yep, well spotted - I'd completed blocked out the fact that some of the #defines are effectively part of the API description. I've added support for emitting them in the metadata file. Thanks!

emoon commented 2 years ago

Great! Thanks :)

emoon commented 2 years ago

Seems to work great!

emoon commented 2 years ago

Hey, I ran into a small issue with this

    "defines": [
        {
            "name": "IMGUI_VERSION",
            "content": "1.81 WIP"
        },
        {
            "name": "IMGUI_VERSION_NUM",
            "content": "18002"
        },
        {
            "name": "IMGUI_HAS_IMSTR",
            "content": "1"
        },
        {
            "name": "IMGUI_PAYLOAD_TYPE_COLOR_3F",
            "content": "_COL3F",
            "comments": {
                "attached": "// float[3]: Standard type for colors, without alpha. User code may use this type."
            }
        },

The problem here is that there is no type, for the first entry content will be a string. for the IMGUI_VERSION_NUM it will be an integer. Now, most of them it's possible to just try to convert to a number to see if they are a integer or not, but when you parse you will know if it's a string or not (as it will be inside quotes) so I think if possible to include if it's a string or number would help the consumer of the data a bit.

ocornut commented 2 years ago

I guess to be lossless the strings should be stored with the “ escaped inside the value, eg:

"content": "\”1.81 WIP\”"

emoon commented 2 years ago

yes, that would be fine also

ShironekoBen commented 2 years ago

LOL, ironically I added code specifically to not do that - initially the string stored in the metadata included the quotes, but then whilst pondering it I realised that since in C++ #define foo bar is a completely valid thing to do, the consumer of the metadata kinda needs to be aware of the possibility that the value there may be a string (from their perspective) regardless of the presence/absence of quotes, so I added some code that strips out the quotes (and brackets around integers if they exist) to make it a bit "neater".

That said, though, it may make more sense to include them as even if there's the possibility of an unexpected string it would make life simpler, and it would also allow consumers of the file to distinguish between #define foo bar and #define foo "bar", which are obviously different semantically now I consider it.

So... hm... I think on second thoughts you're right and the output should include the quotes. I may keep the bracket stripping though as unless anyone can think of a counterexample I think it should be semantically equivalent (the brackets only being there as a mechanism to prevent the preprocessor generating valid-but-unintended code), and for anyone expecting an integer value they probably will come across as a nasty surprise.

emoon commented 2 years ago

I think including the quotes is the correct thing to do. Otherwise it's hard to know if for example _COL3F is referring to another define or a string.

ShironekoBen commented 2 years ago

OK, having considered it a bit more I'm pretty sure you're right and this is the correct thing to do, so I've pushed up a new version that removes the quote stripping. Thanks!

emoon commented 2 years ago

Great! Thanks :)