Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving output quality #58

Open
pdoane opened this issue Oct 29, 2023 · 3 comments
Open

Improving output quality #58

pdoane opened this issue Oct 29, 2023 · 3 comments

Comments

@pdoane
Copy link

pdoane commented Oct 29, 2023

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.

  • Some comments missing: e.g. ImGuiIO has "Main configuration and I/O between your application and ImGui"
  • Line break before preceding comments. This helps with creating logical groups and I have not found a good heuristic for it
  • When to reset attached comments. Typically this resets across groups, but not always
  • Section information: e.g. "Forward declarations and basic types" is nicer than putting all structs in one place

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

@ShironekoBen
Copy link
Collaborator

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
Copy link
Author

pdoane commented Nov 2, 2023

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants