Skip to content

Conversation

@gucio321
Copy link
Collaborator

@gucio321 gucio321 commented Jan 7, 2023

Hi there!
First of all, I'm sorry for this being so large PR. The situation got a bit out of control. 😄
I'll try to briefly point what was changed here but forgive if I've missed something
In general I go rid of this large generateGoFuncs function - it is now of human-readable size

  • split gengo.go into a several files (one for each "datatype") - the further steps are for gengo_funcs.go - it is the most complex one
  • create an internal-state-type for function generator. It holds shared variable such as StringBuilder
  • split generateGoFuncs function into several smaller steps
  • simplify function generation (redesign code; remove "duplicated" code)
  • add some comments

Let me know what do you think about these changes as well as feel free to suggest another one!
IMO the code is easier to read now but let me know if it isn't ;-)

@AllenDang
Copy link
Owner

@gucio321
First of all, big thanks for your effort! Really appreciate your contribute for this repo.
I have a suggestion, do the business first, optimize later. The purpose for this repo and giu is to empower the ability of GUI creation for Golang, so let's spend our effort on make giu more usable, stable and powerful, and drive the optimization from giu to cimgui-go.
What's your opinion?

Copy link
Owner

@AllenDang AllenDang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gucio321
Copy link
Collaborator Author

gucio321 commented Jan 9, 2023

Sure! I'm going to focus on giu as soon as cimgui-go shares all the necessary functions.
To do so however, we need to have this codegen script easy to fix and extend.

@gucio321 gucio321 merged commit feffd3a into AllenDang:main Jan 9, 2023
@gucio321 gucio321 deleted the codegen-cleanup2 branch January 9, 2023 07:31
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 this pull request may close these issues.

2 participants