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

gen: Improve identifier naming, quality of generated code #122

Merged
merged 13 commits into from
Jul 6, 2023

Conversation

tmc
Copy link
Collaborator

@tmc tmc commented Jul 5, 2023

Fixes #119, this omits trailing underscores from generated identifiers and adopts new more Go-idiomatic naming.

@tmc tmc changed the title gen: Improve names gen: Improve identifier naming Jul 5, 2023
@tmc
Copy link
Collaborator Author

tmc commented Jul 5, 2023

This greatly improves the ergonomicsd of the generated code and godoc.

compare

Screenshot 2023-07-05 at 2 35 34 PM

with

Screenshot 2023-07-05 at 2 35 37 PM

@tmc tmc requested review from mgood and progrium July 5, 2023 21:37
@progrium
Copy link
Owner

progrium commented Jul 5, 2023

Looks good so far. Much nicer. Don't know if people actually want lowercase class methods after underscore, but given we convert method names everywhere else to capitalized it seems like they should be capitalized too, even if they technically don't have to be. Thoughts?

@tmc
Copy link
Collaborator Author

tmc commented Jul 5, 2023

Looks good so far. Much nicer. Don't know if people actually want lowercase class methods after underscore, but given we convert method names everywhere else to capitalized it seems like they should be capitalized too, even if they technically don't have to be. Thoughts?

Open to that, trying to do this iteratively and I think this merits inclusion as-is while we iterate on naming scheme.

@mgood
Copy link
Collaborator

mgood commented Jul 5, 2023

IIRC the trailing underscores is something I saw done in PyObjC. I know it looks a little weird, but without it I think there can be selector conflicts where you have one with and one without an argument.

@tmc
Copy link
Collaborator Author

tmc commented Jul 5, 2023

IIRC the trailing underscores is something I saw done in PyObjC. I know it looks a little weird, but without it I think there can be selector conflicts where you have one with and one without an argument.

Yeah we have an example in/with the webkit reload method

@tmc tmc changed the title gen: Improve identifier naming gen: Improve identifier naming, quality of generated code Jul 6, 2023
@progrium
Copy link
Owner

progrium commented Jul 6, 2023

Looks good. Fairly breaking change though. Should we cut a release before merging?

@progrium progrium merged commit 7e5bd0a into progrium:main Jul 6, 2023
@programmingkidx
Copy link
Contributor

@progrium As for the question of whether to capitalize the first letter of a method's name when dealing with a class method. Objective-C doesn't do it. Go does like camel case. But Go functions don't usually have an underscore to them. If I had to vote between these options:

  • NSEvent_alloc()
  • NSEvent_Alloc()
  • NSEventAlloc()

I would probably pick NSEvent_alloc(). It seems like less work to have to write it out like this.

As for cutting a release, please do so. The more release numbers there are the easier it to tell what changes have been made between releases.

@programmingkidx
Copy link
Contributor

@tmc Thank you for the changes. I really like how the return type of a function is on the same line as the func statement. This is so much easier to read than how it was before.

@tmc tmc deleted the improve-names branch July 6, 2023 19:50
@tmc
Copy link
Collaborator Author

tmc commented Jul 6, 2023

we're doing NSEvent_Alloc for consistency

@programmingkidx
Copy link
Contributor

@tmc I can learn to live with that.

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.

Change generator to stop translating colon to underscore
4 participants