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

Add more Cocoa bindings #74

Merged
merged 2 commits into from
Oct 18, 2021
Merged

Add more Cocoa bindings #74

merged 2 commits into from
Oct 18, 2021

Conversation

fserb
Copy link
Contributor

@fserb fserb commented Oct 1, 2021

This adds two new classes NSViewController and NSPopover. It also adds some new methods on other classes and updates the NSWindow's *WindowLevel to match MacOSX 11.3.

@progrium
Copy link
Owner

progrium commented Oct 9, 2021

I appreciate you submitting this, but we've just merged a different approach to adding types. Can you try it out? https://github.com/progrium/macdriver/wiki/HowToAddTypes

@fserb
Copy link
Contributor Author

fserb commented Oct 9, 2021

ahn, I didn't see this before. That would be perfect. :)
I'll update my branch to use those, to make sure everything that I need is there and update this PR.

Thanks. :)

@fserb fserb force-pushed the main branch 2 times, most recently from 706dc47 to b168067 Compare October 9, 2021 19:04
@fserb
Copy link
Contributor Author

fserb commented Oct 9, 2021

Updated.

  • includes NSControl, NSPopover.
  • adds lookup for int -> int32 and NSPopoverBehavior
  • updates NSWindow const window level
  • Adds LoadHTMLString to WKWebView

@progrium progrium requested a review from mgood October 11, 2021 15:59
Copy link
Collaborator

@mgood mgood left a comment

Choose a reason for hiding this comment

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

Thanks for looking into the new generator. It just landed, so you're the first person to try it out. Seems like you've figured it out, but let me know if you have any feedback.

This looks pretty good overall, but I've added suggestions for a few changes. There's a few comments for things I could also use your feedback on too.

cocoa/NSMenuItem.go Outdated Show resolved Hide resolved
cocoa/NSPopover.go Outdated Show resolved Hide resolved
cocoa/NSStatusItem.go Outdated Show resolved Hide resolved
cocoa/NSView.go Outdated
@@ -25,6 +25,10 @@ func (v NSView) AddSubviewPositionedRelativeTo(subview NSViewRef, positioned int
v.AddSubview_positioned_relativeTo_(subview, core.NSUInteger(positioned), relativeTo)
}

func (v NSView) AddSubview(subview NSViewRef) {
v.Send("addSubview:", subview)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can update this (and others below) to use the generated methods too:

Suggested change
v.Send("addSubview:", subview)
v.AddSubview_(subview)

Copy link
Contributor Author

@fserb fserb Oct 17, 2021

Choose a reason for hiding this comment

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

Done.
Is there a reason why the gen_ classes don't simply define the functions without _ ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, we still need to decide what to do with that trailing _. We could strip it in the generator, or stop wrapping those and just recommend calling AddSubview_ or whatever directly.

I kept the _ as a replacement for : in the selector -> method naming to avoid conflicts in some cases where it would otherwise be ambiguous. E.g. WKWebView has reload and reload: selectors which would conflict if we named them both Reload in Go:

- (WKNavigation *)reload;
- (IBAction)reload:(id)sender;

For now I've updated the wrapper methods that were there before the generator to be aliases to the new methods. The trailing _ does look a little weird, but I don't have another good workaround for the conflicting selectors, so we may just stick with it and stop adding new wrappers to alias those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahn, I see the problem.
I do like the Set*, what I meant by my comment is that maybe there would be a way to automatically figure out those?
I.e., if there's x and x:, then we should have X() and SetX()? But maybe this is not true? And then otherwise we can just drop the :?

cocoa/NSView.go Outdated Show resolved Hide resolved
cocoa/NSView.go Outdated Show resolved Hide resolved
}

func (c NSViewController) SetView(v NSView) {
c.Set("view:", v)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yeah, looks like this property is an IBOutlet affected by progrium/macschema#8
I'm gonna try to fix that soon and then we can go back and update this wrapper.

Copy link
Collaborator

Choose a reason for hiding this comment

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

#79 has a fix to update the generated types here with that IBOutlet fix. I'm going to wait until progrium/macschema#12 is merged though in case there are any changes to the fields.

cocoa/NSWindow_const.go Outdated Show resolved Hide resolved
webkit/WKWebView.go Outdated Show resolved Hide resolved
@fserb
Copy link
Contributor Author

fserb commented Oct 17, 2021

Done all changes above.

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.

3 participants