dearimgui / dear_bindings

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

Improving output quality #58

Open pdoane opened 8 months ago

pdoane commented 8 months ago

I'm using cimgui.h as a reference and there are things it can do better than what I can do with the metadata. E.g.

None of this is critical, but if it's on your roadmap I could make use of it.

ShironekoBen commented 8 months ago

Interesting... it's not on the roadmap as-such (at least not until this moment), but I can definitely see the need.

Some comments missing: e.g. ImGuiIO has "Main configuration and I/O between your application and ImGui"

Yeah, that's a consequence of the metadata only including comments that are attached to something it is actually emitting - "loose" comments get ignored.

Line break before preceding comments. This helps with creating logical groups and I have not found a good heuristic for it

That's probably relatively easy to add data for since Dear Bindings already knows quite a lot of that information already.

When to reset attached comments. Typically this resets across groups, but not always

I'm not entirely sure what you mean here - attached comments are logically related to the thing that they are attached to, but don't really have any context beyond that... unless I'm missing something there isn't really any grouping?

Section information: e.g. "Forward declarations and basic types" is nicer than putting all structs in one place

Yeah... that would be nice. Section detection would be beneficial elsewhere too, come to think of it (mainly when adding generated definitions - at the moment it's kinda ad-hoc in how it tries to place those).

One thought that sprang to mind that I'm not sure on the viability of but seemed interesting - do you think there would be an value in providing a "template" that definitions could be slotted into?

I'm thinking something along the lines of "take the cimgui.h file, strip out everything that isn't a comment or whitespace, but then add markers that indicate where struct/function/etc definitions originally lived"... that would mean that a binding generator could (at least for languages that are somewhat flexible about how definitions are laid out) construct a cimgui.xyz file that structurally matches cimgui.h but has language-appropriate definitions slotted into the right places.

Does that sound like something that would be at all useful?

pdoane commented 8 months ago

Yeah, that's a consequence of the metadata only including comments that are attached to something it is actually emitting - "loose" comments get ignored.

What's confusing to me here is that other comments in that section for forward declarations are in the metadata, e.g.

typedef struct ImDrawListSharedData_t ImDrawListSharedData;              // Data shared among multiple draw lists (typically owned by parent ImGui context, but you may create one yourself)

Has its comment but ImGuiIO does not. Maybe because the underlying structure isn't emitted?

I'm not entirely sure what you mean here - attached comments are logically related to the thing that they are attached to, but don't really have any context beyond that... unless I'm missing something there isn't really any grouping?

cimgui.h does a lot of column alignment around types, variable names, values, and comments. I can ignore alignment for most things but the comments need to be aligned for their content, e.g. ImGuiPlatformIO_t comments for what calls which platform function.

It's not exactly clear when/how to reset these "tab stops". Generally a single-line preceding comment preserves the alignment, and preceding comments with an extra line in groups seem to preserve them as well? Maybe there's a heuristic/algorithm here that works well?

Or I could just not include the comments :)

Does that sound like something that would be at all useful?

I think I would just focus on things that are easy and useful to your primary goals. I had originally thought cimgui.h was built from the metadata, so was curious what I could do to improve my output. Then I looked at the scripts and saw that's not how things are structured. The comments for the issue are the biggest things that stand out, but it's already coming along pretty well!

I still have a few more things to finish up before I'm comfortable releasing this (particularly around flags that I want to revisit now that bug has fixed). I've added both you and Omar to my prototype repo in case you want to take a look early.

ShironekoBen commented 8 months ago

Has its comment but ImGuiIO does not. Maybe because the underlying structure isn't emitted?

In a roundabout way, yes. Basically what's happening here is that ImGuiIO has both a forward-declaration (with a comment) and a real declaration (which doesn't have one). In that situation Dear Bindings prefers to emit the full version in the metadata, which results in the comment getting lost.

However ImDrawListSharedData only has a forward declaration, so that is what gets emitted into the metadata (with the comment intact).

It might make sense for the metadata to merge comments from forward-declarations and actual declarations, which would avoid this problem - I think that would work in most situations, but there might be some in which it causes problems...

It's not exactly clear when/how to reset these "tab stops". Generally a single-line preceding comment preserves the alignment, and preceding comments with an extra line in groups seem to preserve them as well? Maybe there's a heuristic/algorithm here that works well?

That's what Dear Bindings does internally - mod_align_comments.py breaks up the file into blocks by looking for blank lines, and then aligns all comments within those blocks. You could replicate that logic yourself, or if you think it would be useful I could emit the "comment group" that gets generated internally as part of the metadata?

And thanks for the invite to the prototype repo! I don't have much of a reference point but on a casual look the output seems nice and readable.