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

Include comments in enum output #212

Closed
idbrii opened this issue Aug 25, 2022 · 47 comments
Closed

Include comments in enum output #212

idbrii opened this issue Aug 25, 2022 · 47 comments

Comments

@idbrii
Copy link

idbrii commented Aug 25, 2022

I generated lua tables for enums with cimgui and
https://github.com/cimgui/cimgui/blob/master/generator/output/structs_and_enums.lua doesn't include the comments. I did something like this:

local defs = require "cimgui.structs_and_enums"

local comments = {}
for line in io.lines("imgui.h") do
    local name, comment = line:match("^%s+(%u%l%S+).-,.*//%s+(.*)$")
    if name then
        comments[name] = comment
    end
end

for name, vals in pairs(defs.enums) do
    for k, v in pairs(vals) do
        v.comment = comments[v.name]
    end
end

Something similar could probably work for other types too.

@idbrii idbrii changed the title Include comments in enum names Include comments in enum output Aug 25, 2022
@sonoro1234
Copy link
Contributor

I could add an option to parse comments.
Let me think about it.

@sonoro1234
Copy link
Contributor

I have been playing with patterns in cpp2ffi.lua in order to parse the preprocessed file with comments included.
I found it very tricky as for example:

int function(int a) // {} this is a comment
{
 return a + 1;
}

would confuse the parser in thinking that the function definition is

int function(int a) // {}

So I prefer to continue stripping comments in preprocessor.
You could use the code you posted above to get them to fill your needs

@idbrii
Copy link
Author

idbrii commented Aug 29, 2022

I guess the advantage of parsing the preprocessed file is that you get the right branch of ifdef'd code? You could generate two versions of the preprocessed file: one with and one without comments.

I'm not sure how the preprocessing works, but I submitted a PR that parses imgui.h directly and uses those results to populate enum and function comments.

@sonoro1234
Copy link
Contributor

I prefer not using your PR by now and trying to do it with the preprocessed output and the line number information.
I will need some days to take care of it.

@sonoro1234
Copy link
Contributor

Question: what are you using the comments for?

@idbrii
Copy link
Author

idbrii commented Sep 5, 2022

I'm using lua, but not luajit so I can't use ffi to get enum constants. Instead, I generate lua files containing the constant values. I output the comments there for reference. Also makes the comments easily findable for our designers who write lua, but not C++.

@sonoro1234
Copy link
Contributor

How can you use cimgui without LuaJIT?

@sonoro1234
Copy link
Contributor

I can't use ffi to get enum constants.

calc_value field gives you enum constants value

@idbrii
Copy link
Author

idbrii commented Sep 7, 2022

I wrote a lua script that loads cimgui's lua output and generates lua files. It uses calc_value. It's much simpler for a user to look at my generated lua file than to look at structs_and_enums.lua and more accurate than looking at imgui.h (since I'm using tables instead of many globals).

Example:

local ImGuiButtonFlags_None              = 0
local ImGuiButtonFlags_MouseButtonLeft   = 1 -- 1 << 0
local ImGuiButtonFlags_MouseButtonRight  = 2 -- 1 << 1
local ImGuiButtonFlags_MouseButtonMiddle = 4 -- 1 << 2

imgui.ButtonFlags = {
	None              = ImGuiButtonFlags_None,
	MouseButtonLeft   = ImGuiButtonFlags_MouseButtonLeft,   -- React on left mouse button (default)
	MouseButtonRight  = ImGuiButtonFlags_MouseButtonRight,  -- React on right mouse button
	MouseButtonMiddle = ImGuiButtonFlags_MouseButtonMiddle, -- React on center mouse button
}

How can you use cimgui without LuaJIT?

I have my own imgui integration into a game with embedded lua 5.4. I'm only using cimgui to generate my enum files.

@sonoro1234
Copy link
Contributor

Would struct fields comments be desired as well?

@sonoro1234
Copy link
Contributor

If running docking_inter with luajit ./generator.lua gcc "internal comments" glfw opengl3 opengl2 sdl you will get an error caused by the comment // Works on range [n..n2) on imgui_internal. If you delete this comment the parsing should succed (except from a bad generation of cimgui.h and cimgui.cpp which I should investigate further)

Are this comments ok for you?

@sonoro1234
Copy link
Contributor

With last commit nothing fails.

@sonoro1234
Copy link
Contributor

@idbrii ?

@idbrii
Copy link
Author

idbrii commented Sep 13, 2022

Would struct fields comments be desired as well?

I wouldn't use them at the moment.

Are this comments ok for you?

I have a deadline now and can't check this out until sometime next week.

@eliasdaler
Copy link

eliasdaler commented Oct 31, 2022

@sonoro1234, was interested in this as well, so I checked it out - and the comments look pretty great to me!
Checked some enums (e.g. ImGuiWindowFlags) and they look good.

A minor problem I saw is that if the enum value doesn't have a comment e.g. ImGuiNavInput_DpadRight, the generated comment value is equal to "//" - it's better if it was an empty string.

@eliasdaler
Copy link

Another thing - is it possible to generate enum/struct comments as well? \

For example, given this enum:

// Flags for ImGui::IsWindowFocused()
enum ImGuiFocusedFlags_
{
    ImGuiFocusedFlags_None                          = 0,
    ...
};

... extract // Flags for ImGui::IsWindowFocused()

or:

// [Internal] Storage used by IsKeyDown(), IsKeyPressed() etc functions.
// If prior to 1.87 you used io.KeysDownDuration[] (which was marked as internal), you should use GetKeyData(key)->DownDuration and *NOT* io.KeysData[key]->DownDuration.
struct ImGuiKeyData
{
    bool        Down;               // True for if key is down
    float       DownDuration;       // Duration the key has been down (<0.0f: not pressed, 0.0f: just pressed, >0.0f: time held)
    float       DownDurationPrev;   // Last frame duration the key has been down
    float       AnalogValue;        // 0.0f..1.0f for gamepad values
};

extract to // [Internal] Storage used by IsKeyDown(), IsKeyPressed() etc functions...

@sonoro1234
Copy link
Contributor

I will look into it!!

@eliasdaler
Copy link

Great! Looking forward to it.

I also noticed that some comments should don't have whitespace trimmed on the left, e.g.

{
    "calc_value": 1,
    "comment": " // Enable anti-aliased lines/borders (*2 the number of triangles for 1.0f wide line or lines thin enough to be drawn using textures, otherwise *3 the number of triangles)",
    "name": "ImDrawListFlags_AntiAliasedLines",
    "value": "1 << 0"
},

@sonoro1234
Copy link
Contributor

I also noticed that some comments should don't have whitespace trimmed on the left, e.g.

I am not sure to understand. Which is the desired comment?

@sonoro1234
Copy link
Contributor

extract to // [Internal] Storage used by IsKeyDown(), IsKeyPressed() etc functions...

Where should this comment be saved to?

@eliasdaler
Copy link

eliasdaler commented Nov 1, 2022

I am not sure to understand. Which is the desired comment?

The comment starts with a space: " // Enable anti-aliased lines/borders - it would be better if whitespace was trimmed, so it would become "// Enable anti-aliased lines/borders".

And I'm still not 100% sure if "//" is worth keeping or not. It would be easier for bindings in other languages if the commend didn't start with // and then you could needed comment character when generating a wrapper (e.g. -- in Lua, # in Python, etc.)

So maybe the "comment" can be just "Enable anti-aliased lines/borders". If the comment contains several "//", then it's okay to keep the second one, e.g.

"comment": " // [DataType]   // ColorEdit, ColorPicker, ColorButton: _display_ values formatted as 0..255.",

can become

"comment": "[DataType]   // ColorEdit, ColorPicker, ColorButton: _display_ values formatted as 0..255.",

and then in Python, for example, it can become a comment like this:

ImGuiColorEditFlags_Uint8 # [DataType]   // ColorEdit, ColorPicker, ColorButton: _display_ values formatted as 0..255.

extract to // [Internal] Storage used by IsKeyDown(), IsKeyPressed() etc functions...

Where should this comment be saved to?

Hmm, I just noticed that structs and enums are arrays in JSON... that's a bit problematic. Ideally it would be something like this (in structs_and_enums.json):

 "ImGuiKeyData": {
    "comment": "// [Internal] Storage used by IsKeyDown(), IsKeyPressed()...",
    "members": [
          {
            "comment": " // True for if key is down",
            "name": "Down",
            "type": "bool"
      },
  ]
}

... but that would break existing cimgui API. Maybe you can do something like this to not break the API?

{
    "enums": "...",
    "structs": "...",
    "comments": {
        "enums": { },
        "structs": {
             "ImGuiKeyData": "// [Internal] Storage used by IsKeyDown(), IsKeyPressed() etc functions..."
       }
    }
}

@sonoro1234
Copy link
Contributor

Just pushed branch comments4000. It keeps comments in struct_comments and enum_comments.

@eliasdaler
Copy link

eliasdaler commented Nov 1, 2022

Nice. Looks good overall, however, there are some problems.

  1. Whitespace issue is still present, now with newlines appearing in the beginning of some comments
  2. Check out the comment to ImGuiIO.ConfigFlags - now it has this part in the comment to ConfigFlags. Or ImGuiIO.BackendPlatformName - has this (line 1939) as part of a comment. It should be that way - I guess the solution is to only include comments which are on the same line as the defined member/enum values and ignore comments above.
  3. Check out comment to ImGuiInputTextCallbackData - it shouldn't include [SECTION] Misc data structures part - I guess the solution here is to only include comments immediately below structs/enums and stop if you encounter an empty line.
    So, in case of ImGuiInputTextCallbackData see this. Lines 2059-2067 should be a part of comment to ImGuiInputTextCallbackData, while lines 2055-2058 shouldn't be.

@sonoro1234
Copy link
Contributor

  1. Whitespace issue is still present, now with newlines appearing in the beginning of some comments

This will be handled when other issues are finished

2. only include comments which are on the same line as the defined member/enum values and ignore comments above.

Thats the key problem: All comments previous to an item are keept as this is the only way to keep comments before a struct or an enum. Empty line could be used as a mark for not appending previous comments but I am not sure if it will solve issues.

@eliasdaler
Copy link

Empty line could be used as a mark for not appending previous comments but I am not sure if it will solve issues.

I've looked through the whole imgui.h - I think that it will. Of course, I can post-process the comment later when generating a binding manually, though...

As for the enum values and struct fields: the "above" comments are irrelevant, only comments on the same line are relevant,

@sonoro1234
Copy link
Contributor

sonoro1234 commented Nov 2, 2022

I have pushed a change for not keeping coments above empty lines

@eliasdaler
Copy link

Didn't work right for ImDrawListFlags_.
Comment should be

// Flags for ImDrawList instance. Those are set automatically by ImGui:: functions from ImGuiIO settings, and generally not manipulated directly.
// It is however possible to temporarily alter flags between calls to ImDrawList:: functions.

... but it only left the second line - there's no empty lines in the comment above. Same for ImDrawFlags_, ImGuiMouseButton_ and many others...

@sonoro1234
Copy link
Contributor

New try!!!

@eliasdaler
Copy link

Much better!

Only two issues now:

  1. Structs which like ImGuiContext pick up unrelated comments with "[SECTION]" - the pattern is this:
// comment

struct SomeStruct {
    ...
};

so, a blank line immediately above the struct indicates that the comment doesn't belong to SomeStruct - so the comment should be empty in this case

  1. As stated before: the comments above members/enum values should always be ignored. For example, in ImFontAtlas.TexReady the comment should be // Set when texture was built matching current font input, the comment above this member isn't about the member itself, inside structs/enums it's used for grouping things. This could be potentially useful, but only if this comment was separated into two strings in JSON, e.g. "above_comment" and "comment" so that in Go we could generate something like this:
type ImFontAtlas struct {
    ...

    // [Internal]
    // NB: Access texture data via GetTexData*() calls! Which will setup a default font for you.
    TexReady           bool // Set when texture was built matching current font input
    TexPixelsUseColors bool // Tell whether our texture data is known to use colors (rather than just alpha channel), in order to help backend select a format.
    ...
}

by knowing which part was before the member and which part was on the same line as the member.

@sonoro1234
Copy link
Contributor

Now using empty lines as marks and prevcomments separated from comments

@eliasdaler
Copy link

Looks good! Though maybe "comment.above" and "comment.sameline" would be a bit more descriptive?

Also, it's a bit weird that when something doesn't have a comment, the "comment" field is an empty array, not an empty object in JSON.

@sonoro1234
Copy link
Contributor

done?

@eliasdaler
Copy link

The last issue I have it untrimmed whitespace in the beginning of some comments.
Otherwise, looks great. :)

@sonoro1234
Copy link
Contributor

solved?

@eliasdaler
Copy link

Not solved here, for example.

@sonoro1234
Copy link
Contributor

Just solved

@eliasdaler
Copy link

Everything looks great now. I'll try to generate Go binding with comments later and see if things work as I expect and there are no useful comments missing.

@idbrii
Copy link
Author

idbrii commented Nov 17, 2022

Sorry for not checking this out sooner. Output generally looks really good! Capturing the above comments is awesome and your generator captures a bunch that mine missed (ImGuiWindowFlags_AlwaysVerticalScrollbar, ImGuiSortDirection_Descending).

Would be nice for the initial // to be stripped from each line and only include the comment text:

defs["enums"]["ImGuiMouseCursor_"][3]["comment"] = "When hovering over InputText, etc."

That way languages with a different comment leader can add their own and use multiline comments without messing with the text.

In the generated lua, could use [[]] quotes for multiline strings instead of escaping. Looks like lua newlines are already handled differently from json.

Are there any guarantees which comments may be multiline? above may be multiline but comment are never multiline?

ImGuiColorEditFlags_'s comments are a bit weird with the double comment, but that's how it is in imgui.h's so 🤷🏽 . Seems like the indent is to make [category] stand out.

When you generate without comments, it still generates a defs["enum_comments"] field. What do you think about generating a comment/value there that mentions how to turn on comment generation?

Thanks for implementing this!

@eliasdaler
Copy link

ImGuiColorEditFlags_'s comments are a bit weird with the double comment, but that's how it is in imgui.h's so 🤷🏽 . Seems like the indent is to make [category] stand out.

That's how most of struct members are documented. If we trim the leading "//", the remaining comment should stay as-is, I think

Are there any guarantees which comments may be multiline? above may be multiline but comment are never multiline?

From what I've seen, there are no "same line" comments which are multiline.

@gucio321
Copy link
Contributor

@sonoro1234 is there any progress here? comments400 branch seems to b dead for a while...

@sonoro1234
Copy link
Contributor

All progress is done in docking_inter branch. To generate with comments just add "comments" option to generator.sh (.bat) and generate. To try another imgui branch checkout from imgui folder and generate.

@gucio321
Copy link
Contributor

oh ok, thank you!

@gucio321
Copy link
Contributor

hmm, I think I hit #232 again 😿

@sonoro1234
Copy link
Contributor

Just add '-DIMGUI_USE_WCHAR32' as an option

@gucio321
Copy link
Contributor

@sonoro1234 I meant that I can't generate with comments without editing generator.sh - see #238

@sonoro1234
Copy link
Contributor

#232 (comment)

@gucio321
Copy link
Contributor

#232 (comment)

the best would be for you to have your own generator.bat (sh) outside cimgui repo edited as:

cd cimgui/generator
luajit ./generator.lua gcc "internal noimstrv" glfw opengl3 opengl2 sdl2 -DIMGUI_USE_WCHAR32
That way you are able to update cimgui but keeping your generator options

I see, but why not introduce some simple way to make this script more customizable with command-line flags? (just like I did in #238 )

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

Successfully merging a pull request may close this issue.

4 participants